Commit 569ee102 authored by Fabio Pitino's avatar Fabio Pitino Committed by Kamil Trzciński

Reduce queries when ticking runner queue

When UpdateBuildQueueService runs it iterates already
through the available runners for a given project.
In order to `tick_runner_queue` for each runner we
checked whether the `can_pick?` the build which
was checking again whether the runner was in the list
of available runner for the given project.

This change removes this redundant query which is very
expensive given that it's a UNION of 3 expensive
subqueries.
parent b3b1f810
...@@ -253,9 +253,10 @@ module Ci ...@@ -253,9 +253,10 @@ module Ci
end end
def can_pick?(build) def can_pick?(build)
return false if self.ref_protected? && !build.protected? # Run `matches_build?` checks before, since they are cheaper than
# `assignable_for?`.
assignable_for?(build.project_id) && accepting_tags?(build) #
matches_build?(build) && assignable_for?(build.project_id)
end end
def only_for?(project) def only_for?(project)
...@@ -305,8 +306,10 @@ module Ci ...@@ -305,8 +306,10 @@ module Ci
end end
def pick_build!(build) def pick_build!(build)
if can_pick?(build) if Feature.enabled?(:ci_reduce_queries_when_ticking_runner_queue, self, default_enabled: :yaml)
tick_runner_queue tick_runner_queue if matches_build?(build)
else
tick_runner_queue if can_pick?(build)
end end
end end
...@@ -370,6 +373,13 @@ module Ci ...@@ -370,6 +373,13 @@ module Ci
end end
end end
# TODO: choose a better name and consider splitting this method into two
def matches_build?(build)
return false if self.ref_protected? && !build.protected?
accepting_tags?(build)
end
def accepting_tags?(build) def accepting_tags?(build)
(run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty? (run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty?
end end
......
---
title: Reduce queries when ticking runner queue
merge_request: 55496
author:
type: performance
---
name: ci_reduce_queries_when_ticking_runner_queue
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55496
rollout_issue_url:
milestone: '13.10'
type: development
group: group::continuous integration
default_enabled: false
...@@ -842,27 +842,50 @@ RSpec.describe Ci::Runner do ...@@ -842,27 +842,50 @@ RSpec.describe Ci::Runner do
end end
describe '#pick_build!' do describe '#pick_build!' do
let(:build) { create(:ci_build) }
let(:runner) { create(:ci_runner) }
context 'runner can pick the build' do context 'runner can pick the build' do
it 'calls #tick_runner_queue' do it 'calls #tick_runner_queue' do
ci_build = build(:ci_build)
runner = build(:ci_runner)
allow(runner).to receive(:can_pick?).with(ci_build).and_return(true)
expect(runner).to receive(:tick_runner_queue) expect(runner).to receive(:tick_runner_queue)
runner.pick_build!(ci_build) runner.pick_build!(build)
end end
end end
context 'runner cannot pick the build' do context 'runner cannot pick the build' do
it 'does not call #tick_runner_queue' do before do
ci_build = build(:ci_build) build.tag_list = [:docker]
runner = build(:ci_runner) end
allow(runner).to receive(:can_pick?).with(ci_build).and_return(false)
it 'does not call #tick_runner_queue' do
expect(runner).not_to receive(:tick_runner_queue) expect(runner).not_to receive(:tick_runner_queue)
runner.pick_build!(ci_build) runner.pick_build!(build)
end
end
context 'build picking improvement enabled' do
before do
stub_feature_flags(ci_reduce_queries_when_ticking_runner_queue: true)
end
it 'does not check if the build is assignable to a runner' do
expect(runner).not_to receive(:can_pick?)
runner.pick_build!(build)
end
end
context 'build picking improvement disabled' do
before do
stub_feature_flags(ci_reduce_queries_when_ticking_runner_queue: false)
end
it 'checks if the build is assignable to a runner' do
expect(runner).to receive(:can_pick?).and_call_original
runner.pick_build!(build)
end end
end end
end end
......
...@@ -26,6 +26,24 @@ RSpec.describe Ci::UpdateBuildQueueService do ...@@ -26,6 +26,24 @@ RSpec.describe Ci::UpdateBuildQueueService do
end end
it_behaves_like 'refreshes runner' it_behaves_like 'refreshes runner'
it 'avoids running redundant queries' do
expect(Ci::Runner).not_to receive(:owned_or_instance_wide)
subject.execute(build)
end
context 'when feature flag ci_reduce_queries_when_ticking_runner_queue is disabled' do
before do
stub_feature_flags(ci_reduce_queries_when_ticking_runner_queue: false)
end
it 'runs redundant queries using `owned_or_instance_wide` scope' do
expect(Ci::Runner).to receive(:owned_or_instance_wide).and_call_original
subject.execute(build)
end
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment