summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2015-10-07 17:37:39 +0200
committerYorick Peterse <yorickpeterse@gmail.com>2015-10-08 14:35:32 +0200
commit03417456f0b7db408bfefd28e5b9342889b7f711 (patch)
tree77da54c8c65a594672f0d5e3416caff05619a275
parent1190d0ab3dc7a3025bf55b666f34d1a0b51a8d89 (diff)
downloadgitlab-ce-03417456f0b7db408bfefd28e5b9342889b7f711.tar.gz
Revamp finding projects by namespaces
By using a JOIN we can remove the need for using 2 separate queries to find a project by its namespace. Combined with an index (only needed for PostgreSQL) this reduces the query time from ~245 ms (~520 ms for the first call) down to roughly 10 ms (~15 ms for the first call).
-rw-r--r--app/models/concerns/case_sensitivity.rb2
-rw-r--r--app/models/project.rb20
-rw-r--r--db/migrate/20151007120511_namespaces_projects_path_lower_indexes.rb17
-rw-r--r--db/schema.rb2
-rw-r--r--spec/benchmarks/models/project_spec.rb20
-rw-r--r--spec/models/concerns/case_sensitivity_spec.rb43
6 files changed, 80 insertions, 24 deletions
diff --git a/app/models/concerns/case_sensitivity.rb b/app/models/concerns/case_sensitivity.rb
index 49d350e092b..fe0cea8465f 100644
--- a/app/models/concerns/case_sensitivity.rb
+++ b/app/models/concerns/case_sensitivity.rb
@@ -6,7 +6,7 @@ module CaseSensitivity
# Queries the given columns regardless of the casing used.
#
# Unlike other ActiveRecord methods this method only operates on a Hash.
- def case_insensitive_where(params)
+ def iwhere(params)
criteria = self
cast_lower = Gitlab::Database.postgresql?
diff --git a/app/models/project.rb b/app/models/project.rb
index bb47b9abb03..f75082a35d0 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -40,6 +40,7 @@ class Project < ActiveRecord::Base
include Referable
include Sortable
include AfterCommitQueue
+ include CaseSensitivity
extend Gitlab::ConfigHelper
extend Enumerize
@@ -235,13 +236,18 @@ class Project < ActiveRecord::Base
end
def find_with_namespace(id)
- return nil unless id.include?('/')
-
- id = id.split('/')
- namespace = Namespace.by_path(id.first)
- return nil unless namespace
-
- where(namespace_id: namespace.id).where("LOWER(projects.path) = :path", path: id.second.downcase).first
+ namespace_path, project_path = id.split('/')
+
+ return nil if !namespace_path || !project_path
+
+ # Use of unscoped ensures we're not secretly adding any ORDER BYs, which
+ # have a negative impact on performance (and aren't needed for this
+ # query).
+ unscoped.
+ joins(:namespace).
+ iwhere('namespaces.path' => namespace_path).
+ iwhere('projects.path' => project_path).
+ take
end
def visibility_levels
diff --git a/db/migrate/20151007120511_namespaces_projects_path_lower_indexes.rb b/db/migrate/20151007120511_namespaces_projects_path_lower_indexes.rb
new file mode 100644
index 00000000000..7f6cd6d5a78
--- /dev/null
+++ b/db/migrate/20151007120511_namespaces_projects_path_lower_indexes.rb
@@ -0,0 +1,17 @@
+class NamespacesProjectsPathLowerIndexes < ActiveRecord::Migration
+ disable_ddl_transaction!
+
+ def up
+ return unless Gitlab::Database.postgresql?
+
+ execute 'CREATE INDEX CONCURRENTLY index_on_namespaces_lower_path ON namespaces (LOWER(path));'
+ execute 'CREATE INDEX CONCURRENTLY index_on_projects_lower_path ON projects (LOWER(path));'
+ end
+
+ def down
+ return unless Gitlab::Database.postgresql?
+
+ remove_index :namespaces, name: :index_on_namespaces_lower_path
+ remove_index :projects, name: :index_on_projects_lower_path
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 93202f16111..c5c462c2e57 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20151005162154) do
+ActiveRecord::Schema.define(version: 20151007120511) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
diff --git a/spec/benchmarks/models/project_spec.rb b/spec/benchmarks/models/project_spec.rb
new file mode 100644
index 00000000000..0c6b533ac2b
--- /dev/null
+++ b/spec/benchmarks/models/project_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Project, benchmark: true do
+ describe '.find_with_namespace' do
+ let(:group) { create(:group, name: 'sisinmaru') }
+ let(:project) { create(:project, name: 'maru', namespace: group) }
+
+ describe 'using a capitalized namespace' do
+ benchmark_subject { described_class.find_with_namespace('sisinmaru/MARU') }
+
+ it { is_expected.to iterate_per_second(600) }
+ end
+
+ describe 'using a lowercased namespace' do
+ benchmark_subject { described_class.find_with_namespace('sisinmaru/maru') }
+
+ it { is_expected.to iterate_per_second(600) }
+ end
+ end
+end
diff --git a/spec/models/concerns/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb
index 8b9f50aada7..f7ed30f8198 100644
--- a/spec/models/concerns/case_sensitivity_spec.rb
+++ b/spec/models/concerns/case_sensitivity_spec.rb
@@ -1,11 +1,16 @@
require 'spec_helper'
describe CaseSensitivity do
- describe '.case_insensitive_where' do
+ describe '.iwhere' do
let(:connection) { ActiveRecord::Base.connection }
let(:model) { Class.new { include CaseSensitivity } }
describe 'using PostgreSQL' do
+ before do
+ allow(Gitlab::Database).to receive(:postgresql?).and_return(true)
+ allow(Gitlab::Database).to receive(:mysql?).and_return(false)
+ end
+
describe 'with a single column/value pair' do
it 'returns the criteria for a column and a value' do
criteria = double(:criteria)
@@ -18,7 +23,7 @@ describe CaseSensitivity do
with(%q{LOWER("foo") = LOWER(:value)}, value: 'bar').
and_return(criteria)
- expect(model.case_insensitive_where(foo: 'bar')).to eq(criteria)
+ expect(model.iwhere(foo: 'bar')).to eq(criteria)
end
it 'returns the criteria for a column with a table, and a value' do
@@ -32,7 +37,7 @@ describe CaseSensitivity do
with(%q{LOWER("foo"."bar") = LOWER(:value)}, value: 'bar').
and_return(criteria)
- expect(model.case_insensitive_where('foo.bar': 'bar')).to eq(criteria)
+ expect(model.iwhere(:'foo.bar' => 'bar')).to eq(criteria)
end
end
@@ -57,7 +62,7 @@ describe CaseSensitivity do
with(%q{LOWER("bar") = LOWER(:value)}, value: 'baz').
and_return(final)
- got = model.case_insensitive_where(foo: 'bar', bar: 'baz')
+ got = model.iwhere(foo: 'bar', bar: 'baz')
expect(got).to eq(final)
end
@@ -82,7 +87,8 @@ describe CaseSensitivity do
with(%q{LOWER("foo"."baz") = LOWER(:value)}, value: 'baz').
and_return(final)
- got = model.case_insensitive_where('foo.bar': 'bar', 'foo.baz': 'baz')
+ got = model.iwhere(:'foo.bar' => 'bar',
+ :'foo.baz' => 'baz')
expect(got).to eq(final)
end
@@ -90,6 +96,11 @@ describe CaseSensitivity do
end
describe 'using MySQL' do
+ before do
+ allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
+ allow(Gitlab::Database).to receive(:mysql?).and_return(true)
+ end
+
describe 'with a single column/value pair' do
it 'returns the criteria for a column and a value' do
criteria = double(:criteria)
@@ -99,10 +110,10 @@ describe CaseSensitivity do
and_return('`foo`')
expect(model).to receive(:where).
- with(%q{LOWER(`foo`) = LOWER(:value)}, value: 'bar').
+ with(%q{`foo` = :value}, value: 'bar').
and_return(criteria)
- expect(model.case_insensitive_where(foo: 'bar')).to eq(criteria)
+ expect(model.iwhere(foo: 'bar')).to eq(criteria)
end
it 'returns the criteria for a column with a table, and a value' do
@@ -113,10 +124,11 @@ describe CaseSensitivity do
and_return('`foo`.`bar`')
expect(model).to receive(:where).
- with(%q{LOWER(`foo`.`bar`) = LOWER(:value)}, value: 'bar').
+ with(%q{`foo`.`bar` = :value}, value: 'bar').
and_return(criteria)
- expect(model.case_insensitive_where('foo.bar': 'bar')).to eq(criteria)
+ expect(model.iwhere(:'foo.bar' => 'bar')).
+ to eq(criteria)
end
end
@@ -134,14 +146,14 @@ describe CaseSensitivity do
and_return('`bar`')
expect(model).to receive(:where).
- with(%q{LOWER(`foo`) = LOWER(:value)}, value: 'bar').
+ with(%q{`foo` = :value}, value: 'bar').
and_return(initial)
expect(initial).to receive(:where).
- with(%q{LOWER(`bar`) = LOWER(:value)}, value: 'baz').
+ with(%q{`bar` = :value}, value: 'baz').
and_return(final)
- got = model.case_insensitive_where(foo: 'bar', bar: 'baz')
+ got = model.iwhere(foo: 'bar', bar: 'baz')
expect(got).to eq(final)
end
@@ -159,14 +171,15 @@ describe CaseSensitivity do
and_return('`foo`.`baz`')
expect(model).to receive(:where).
- with(%q{LOWER(`foo`.`bar`) = LOWER(:value)}, value: 'bar').
+ with(%q{`foo`.`bar` = :value}, value: 'bar').
and_return(initial)
expect(initial).to receive(:where).
- with(%q{LOWER(`foo`.`baz`) = LOWER(:value)}, value: 'baz').
+ with(%q{`foo`.`baz` = :value}, value: 'baz').
and_return(final)
- got = model.case_insensitive_where('foo.bar': 'bar', 'foo.baz': 'baz')
+ got = model.iwhere(:'foo.bar' => 'bar',
+ :'foo.baz' => 'baz')
expect(got).to eq(final)
end