Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-05-07 19:22:07 +0300
committerYorick Peterse <yorickpeterse@gmail.com>2018-05-17 14:53:00 +0300
commit19428e800895ba20eacb3357285acef8d69f6d8c (patch)
tree0f16e630b6a808b6013d463146c32a134d6ee9c0 /spec/models
parent70985aa19b389c2ee8234edfbb516b5403a7bfcf (diff)
Preload pipeline data for project pipelines
When displaying the pipelines of a project we now preload the following data: 1. Authors of the commits that belong to these pipelines 2. The number of warnings per pipeline, which is used by Ci::Pipeline#has_warnings? == Commit Authors Previously this data was queried for every Commit separately, leading to 20 SQL queries being executed in the worst case. With an average of 3 to 5 milliseconds per SQL query this could result in 100 milliseconds being spent in _just_ getting Commit authors. To preload this data Commit#author now uses BatchLoader (through Commit#lazy_author), and a separate module Gitlab::Ci::Pipeline::Preloader is used to ensure all authors are loaded before they are used. == Number of warnings This changes Ci::Pipeline#has_warnings? so it supports preloading of the number of warnings per pipeline. This removes the need for executing a COUNT(*) query for every pipeline just to see if it has any warnings or not.
Diffstat (limited to 'spec/models')
-rw-r--r--spec/models/ci/pipeline_spec.rb27
-rw-r--r--spec/models/commit_spec.rb84
2 files changed, 107 insertions, 4 deletions
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index ddd66a6be87..e7845b693a1 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -774,6 +774,33 @@ describe Ci::Pipeline, :mailer do
end
end
+ describe '#number_of_warnings' do
+ it 'returns the number of warnings' do
+ create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop')
+
+ expect(pipeline.number_of_warnings).to eq(1)
+ end
+
+ it 'supports eager loading of the number of warnings' do
+ pipeline2 = create(:ci_empty_pipeline, status: :created, project: project)
+
+ create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop')
+ create(:ci_build, :allowed_to_fail, :failed, pipeline: pipeline2, name: 'rubocop')
+
+ pipelines = project.pipelines.to_a
+
+ pipelines.each(&:number_of_warnings)
+
+ # To run the queries we need to actually use the lazy objects, which we do
+ # by just sending "to_i" to them.
+ amount = ActiveRecord::QueryRecorder
+ .new { pipelines.each { |p| p.number_of_warnings.to_i } }
+ .count
+
+ expect(amount).to eq(1)
+ end
+ end
+
shared_context 'with some outdated pipelines' do
before do
create_pipeline(:canceled, 'ref', 'A', project)
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 448b813c2e1..090f91168ad 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -52,22 +52,98 @@ describe Commit do
end
end
- describe '#author' do
+ describe '#author', :request_store do
it 'looks up the author in a case-insensitive way' do
user = create(:user, email: commit.author_email.upcase)
expect(commit.author).to eq(user)
end
- it 'caches the author', :request_store do
+ it 'caches the author' do
user = create(:user, email: commit.author_email)
- expect(User).to receive(:find_by_any_email).and_call_original
expect(commit.author).to eq(user)
+
key = "Commit:author:#{commit.author_email.downcase}"
- expect(RequestStore.store[key]).to eq(user)
+ expect(RequestStore.store[key]).to eq(user)
expect(commit.author).to eq(user)
end
+
+ context 'using eager loading' do
+ let!(:alice) { create(:user, email: 'alice@example.com') }
+ let!(:bob) { create(:user, email: 'hunter2@example.com') }
+
+ let(:alice_commit) do
+ described_class.new(RepoHelpers.sample_commit, project).tap do |c|
+ c.author_email = 'alice@example.com'
+ end
+ end
+
+ let(:bob_commit) do
+ # The commit for Bob uses one of his alternative Emails, instead of the
+ # primary one.
+ described_class.new(RepoHelpers.sample_commit, project).tap do |c|
+ c.author_email = 'bob@example.com'
+ end
+ end
+
+ let(:eve_commit) do
+ described_class.new(RepoHelpers.sample_commit, project).tap do |c|
+ c.author_email = 'eve@example.com'
+ end
+ end
+
+ let!(:commits) { [alice_commit, bob_commit, eve_commit] }
+
+ before do
+ create(:email, user: bob, email: 'bob@example.com')
+ end
+
+ it 'executes only two SQL queries' do
+ recorder = ActiveRecord::QueryRecorder.new do
+ # Running this first ensures we don't run one query for every
+ # commit.
+ commits.each(&:lazy_author)
+
+ # This forces the execution of the SQL queries necessary to load the
+ # data.
+ commits.each { |c| c.author.try(:id) }
+ end
+
+ expect(recorder.count).to eq(2)
+ end
+
+ it "preloads the authors for Commits matching a user's primary Email" do
+ commits.each(&:lazy_author)
+
+ expect(alice_commit.author).to eq(alice)
+ end
+
+ it "preloads the authors for Commits using a User's alternative Email" do
+ commits.each(&:lazy_author)
+
+ expect(bob_commit.author).to eq(bob)
+ end
+
+ it 'sets the author to Nil if an author could not be found for a Commit' do
+ commits.each(&:lazy_author)
+
+ expect(eve_commit.author).to be_nil
+ end
+
+ it 'does not execute SQL queries once the authors are preloaded' do
+ commits.each(&:lazy_author)
+ commits.each { |c| c.author.try(:id) }
+
+ recorder = ActiveRecord::QueryRecorder.new do
+ alice_commit.author
+ bob_commit.author
+ eve_commit.author
+ end
+
+ expect(recorder.count).to be_zero
+ end
+ end
end
describe '#to_reference' do