Merge branch 'perform-sql-matching-of-tags' into 'master'

Perform SQL matching of Build&Runner tags to greatly speed-up job picking

Closes #24975

See merge request gitlab-org/gitlab-ce!15674

app/models/ci/build.rb

@@ -47,6 +47,25 @@ module Ci
     scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) }
     scope :ref_protected, -> { where(protected: true) }
 
+    scope :matches_tag_ids, -> (tag_ids) do
+      matcher = ::ActsAsTaggableOn::Tagging
+        .where(taggable_type: CommitStatus)
+        .where(context: 'tags')
+        .where('taggable_id = ci_builds.id')
+        .where.not(tag_id: tag_ids).select('1')
+
+      where("NOT EXISTS (?)", matcher)
+    end
+
+    scope :with_any_tags, -> do
+      matcher = ::ActsAsTaggableOn::Tagging
+        .where(taggable_type: CommitStatus)
+        .where(context: 'tags')
+        .where('taggable_id = ci_builds.id').select('1')
+
+      where("EXISTS (?)", matcher)
+    end
+
     mount_uploader :legacy_artifacts_file, LegacyArtifactUploader, mount_on: :artifacts_file
     mount_uploader :legacy_artifacts_metadata, LegacyArtifactUploader, mount_on: :artifacts_metadata
 

app/models/ci/runner.rb

@@ -112,7 +112,7 @@ module Ci
     def can_pick?(build)
       return false if self.ref_protected? && !build.protected?
 
-      assignable_for?(build.project) && accepting_tags?(build)
+      assignable_for?(build.project_id) && accepting_tags?(build)
     end
 
     def only_for?(project)
@@ -171,8 +171,8 @@ module Ci
       end
     end
 
-    def assignable_for?(project)
-      is_shared? || projects.exists?(id: project.id)
+    def assignable_for?(project_id)
+      is_shared? || projects.exists?(id: project_id)
     end
 
     def accepting_tags?(build)

app/services/ci/register_job_service.rb

@@ -22,6 +22,16 @@ module Ci
 
       valid = true
 
+      if Feature.enabled?('ci_job_request_with_tags_matcher')
+        # pick builds that does not have other tags than runner's one
+        builds = builds.matches_tag_ids(runner.tags.ids)
+
+        # pick builds that have at least one tag
+        unless runner.run_untagged?
+          builds = builds.with_any_tags
+        end
+      end
+
       builds.find do |build|
         next unless runner.can_pick?(build)
 

changelogs/unreleased/perform-sql-matching-of-tags.yml

+---
+title: Perform SQL matching of Build&Runner tags to greatly speed-up job picking
+merge_request:
+author:
+type: performance

spec/models/ci/build_spec.rb

@@ -1921,4 +1921,77 @@ describe Ci::Build do
       end
     end
   end
+
+  describe '.matches_tag_ids' do
+    set(:build) { create(:ci_build, project: project, user: user) }
+    let(:tag_ids) { ::ActsAsTaggableOn::Tag.named_any(tag_list).ids }
+
+    subject { described_class.where(id: build).matches_tag_ids(tag_ids) }
+
+    before do
+      build.update(tag_list: build_tag_list)
+    end
+
+    context 'when have different tags' do
+      let(:build_tag_list) { %w(A B) }
+      let(:tag_list) { %w(C D) }
+
+      it "does not match a build" do
+        is_expected.not_to contain_exactly(build)
+      end
+    end
+
+    context 'when have a subset of tags' do
+      let(:build_tag_list) { %w(A B) }
+      let(:tag_list) { %w(A B C D) }
+
+      it "does match a build" do
+        is_expected.to contain_exactly(build)
+      end
+    end
+
+    context 'when build does not have tags' do
+      let(:build_tag_list) { [] }
+      let(:tag_list) { %w(C D) }
+
+      it "does match a build" do
+        is_expected.to contain_exactly(build)
+      end
+    end
+
+    context 'when does not have a subset of tags' do
+      let(:build_tag_list) { %w(A B C) }
+      let(:tag_list) { %w(C D) }
+
+      it "does not match a build" do
+        is_expected.not_to contain_exactly(build)
+      end
+    end
+  end
+
+  describe '.matches_tags' do
+    set(:build) { create(:ci_build, project: project, user: user) }
+
+    subject { described_class.where(id: build).with_any_tags }
+
+    before do
+      build.update(tag_list: tag_list)
+    end
+
+    context 'when does have tags' do
+      let(:tag_list) { %w(A B) }
+
+      it "does match a build" do
+        is_expected.to contain_exactly(build)
+      end
+    end
+
+    context 'when does not have tags' do
+      let(:tag_list) { [] }
+
+      it "does not match a build" do
+        is_expected.not_to contain_exactly(build)
+      end
+    end
+  end
 end

spec/services/ci/register_job_service_spec.rb

@@ -15,16 +15,14 @@ module Ci
     describe '#execute' do
       context 'runner follow tag list' do
         it "picks build with the same tag" do
-          pending_job.tag_list = ["linux"]
-          pending_job.save
-          specific_runner.tag_list = ["linux"]
+          pending_job.update(tag_list: ["linux"])
+          specific_runner.update(tag_list: ["linux"])
           expect(execute(specific_runner)).to eq(pending_job)
         end
 
         it "does not pick build with different tag" do
-          pending_job.tag_list = ["linux"]
-          pending_job.save
-          specific_runner.tag_list = ["win32"]
+          pending_job.update(tag_list: ["linux"])
+          specific_runner.update(tag_list: ["win32"])
           expect(execute(specific_runner)).to be_falsey
         end
 
@@ -33,13 +31,12 @@ module Ci
         end
 
         it "does not pick build with tag" do
-          pending_job.tag_list = ["linux"]
-          pending_job.save
+          pending_job.update(tag_list: ["linux"])
           expect(execute(specific_runner)).to be_falsey
         end
 
         it "pick build without tag" do
-          specific_runner.tag_list = ["win32"]
+          specific_runner.update(tag_list: ["win32"])
           expect(execute(specific_runner)).to eq(pending_job)
         end
       end
@@ -172,7 +169,7 @@ module Ci
 
       context 'when first build is stalled' do
         before do
-          pending_job.lock_version = 10
+          pending_job.update(lock_version: 0)
         end
 
         subject { described_class.new(specific_runner).execute }
@@ -182,7 +179,7 @@ module Ci
 
           before do
             allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner)
-              .and_return([pending_job, other_build])
+              .and_return(Ci::Build.where(id: [pending_job, other_build]))
           end
 
           it "receives second build from the queue" do
@@ -194,7 +191,7 @@ module Ci
         context 'when single build is in queue' do
           before do
             allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner)
-              .and_return([pending_job])
+              .and_return(Ci::Build.where(id: pending_job))
           end
 
           it "does not receive any valid result" do
@@ -205,7 +202,7 @@ module Ci
         context 'when there is no build in queue' do
           before do
             allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner)
-              .and_return([])
+              .and_return(Ci::Build.none)
           end
 
           it "does not receive builds but result is valid" do