From 72f428c7d217a5c40ed87d68ab9100e4c8754633 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 8 Oct 2015 13:28:26 +0200
Subject: [PATCH] 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.
---
CHANGELOG | 1 +
app/models/user.rb | 10 ++++++++--
...32_add_users_lower_username_email_indexes.rb | 17 +++++++++++++++++
lib/tasks/migrate/setup_postgresql.rake | 2 ++
spec/benchmarks/models/user_spec.rb | 4 +++-
5 files changed, 31 insertions(+), 3 deletions(-)
create mode 100644 db/migrate/20151008110232_add_users_lower_username_email_indexes.rb
diff --git a/CHANGELOG b/CHANGELOG
index 07ff3d6de42..12561d6ceec 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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)
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') }
--
GitLab