diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2015-10-15 16:41:17 +0300 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2015-10-15 16:41:17 +0300 |
commit | 5ce933599c1c1407620a340de4947497576ad12a (patch) | |
tree | c12c26b807c314cd004d8ddebda0aa1421aa5ec5 | |
parent | c0a6836be48bdf9a32ac2c9e610f62aef1e2e3f7 (diff) | |
parent | 72f428c7d217a5c40ed87d68ab9100e4c8754633 (diff) |
Merge branch 'user-by-login-performance' into 'master'
Improve User.by_login performance
This greatly speeds up the performance of `User.by_login`. I adopted some changes from @haynes in this patch, the credits go to him for coming up with those originally.
Fixes #2341
See merge request !1545
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/user.rb | 10 | ||||
-rw-r--r-- | db/migrate/20151008110232_add_users_lower_username_email_indexes.rb | 17 | ||||
-rw-r--r-- | lib/tasks/migrate/setup_postgresql.rake | 2 | ||||
-rw-r--r-- | spec/benchmarks/models/user_spec.rb | 4 |
5 files changed, 31 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index d2704ed204e..e9ca5ddee5f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.1.0 (unreleased) - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu) - Speed up load times of issue detail pages by roughly 1.5x - Make diff file view easier to use on mobile screens (Stan Hu) + - Improved performance of finding users by username or Email address - Add support for creating directories from Files page (Stan Hu) - Allow removing of project without confirmation when JavaScript is disabled (Stan Hu) - Support filtering by "Any" milestone or issue and fix "No Milestone" and "No Label" filters (Stan Hu) diff --git a/app/models/user.rb b/app/models/user.rb index 889d2d3b867..17ccb3b8788 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -68,6 +68,7 @@ class User < ActiveRecord::Base include Referable include Sortable include TokenAuthenticatable + include CaseSensitivity default_value_for :admin, false default_value_for :can_create_group, gitlab_config.default_can_create_group @@ -273,8 +274,13 @@ class User < ActiveRecord::Base end def by_login(login) - where('lower(username) = :value OR lower(email) = :value', - value: login.to_s.downcase).first + return nil unless login + + if login.include?('@'.freeze) + unscoped.iwhere(email: login).take + else + unscoped.iwhere(username: login).take + end end def find_by_username!(username) diff --git a/db/migrate/20151008110232_add_users_lower_username_email_indexes.rb b/db/migrate/20151008110232_add_users_lower_username_email_indexes.rb new file mode 100644 index 00000000000..2f2dc776785 --- /dev/null +++ b/db/migrate/20151008110232_add_users_lower_username_email_indexes.rb @@ -0,0 +1,17 @@ +class AddUsersLowerUsernameEmailIndexes < ActiveRecord::Migration + disable_ddl_transaction! + + def up + return unless Gitlab::Database.postgresql? + + execute 'CREATE INDEX CONCURRENTLY index_on_users_lower_username ON users (LOWER(username));' + execute 'CREATE INDEX CONCURRENTLY index_on_users_lower_email ON users (LOWER(email));' + end + + def down + return unless Gitlab::Database.postgresql? + + remove_index :users, :index_on_users_lower_username + remove_index :users, :index_on_users_lower_email + end +end diff --git a/lib/tasks/migrate/setup_postgresql.rake b/lib/tasks/migrate/setup_postgresql.rake index bf6894a8351..141a0b74ec0 100644 --- a/lib/tasks/migrate/setup_postgresql.rake +++ b/lib/tasks/migrate/setup_postgresql.rake @@ -1,6 +1,8 @@ require Rails.root.join('db/migrate/20151007120511_namespaces_projects_path_lower_indexes') +require Rails.root.join('db/migrate/20151008110232_add_users_lower_username_email_indexes') desc 'GitLab | Sets up PostgreSQL' task setup_postgresql: :environment do NamespacesProjectsPathLowerIndexes.new.up + AddUsersLowerUsernameEmailIndexes.new.up end diff --git a/spec/benchmarks/models/user_spec.rb b/spec/benchmarks/models/user_spec.rb index 168be20b7a5..cc5c3904193 100644 --- a/spec/benchmarks/models/user_spec.rb +++ b/spec/benchmarks/models/user_spec.rb @@ -11,7 +11,9 @@ describe User, benchmark: true do end end - let(:iterations) { 1000 } + # The iteration count is based on the query taking little over 1 ms when + # using PostgreSQL. + let(:iterations) { 900 } describe 'using a capitalized username' do benchmark_subject { User.by_login('Alice') } |