Improve performance of User.by_login

Performance is improved in two steps:

1. On PostgreSQL an expression index is used for checking lower(email)
   and lower(username).
2. The check to determine if we're searching for a username or Email is
   moved to Ruby. Thanks to @haynes for suggesting and writing the
   initial implementation of this.

Moving the check to Ruby makes this method an additional 1.5 times
faster compared to doing the check in the SQL query.

With performance being improved I've now also tweaked the amount of
iterations required by the User.by_login benchmark. This method now runs
between 900 and 1000 iterations per second.
This commit is contained in:
Yorick Peterse 2015-10-08 13:28:26 +02:00
parent fb7785628a
commit 72f428c7d2
5 changed files with 31 additions and 3 deletions

View file

@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.1.0 (unreleased)
- 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)

View file

@ -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)

View file

@ -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

View file

@ -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

View file

@ -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') }