Optimise builds matching to making this in one placeoptimise-runner-builds-matching

5 files changed, 88 insertions, 73 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 41446946a5e..c1224761d76 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -401,7 +401,7 @@ module Ci
     end
 
     def any_runners_online?
-      project.any_runners? { |runner| runner.active? && runner.online? && runner.can_pick?(self) }
+      Ci::RunnerBuildsMatcherService.new.valid_runners_for_pending_build(self).any?
     end
 
     def stuck?
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 8c9aacca8de..81a759cb20d 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -27,6 +27,7 @@ module Ci
     scope :paused, -> { where(active: false) }
     scope :online, -> { where('contacted_at > ?', contact_time_deadline) }
     scope :ordered, -> { order(id: :desc) }
+    scope :run_untagged, -> { where(run_untagged: true) }
 
     scope :belonging_to_project, -> (project_id) {
       joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
@@ -55,6 +56,16 @@ module Ci
         .project_type
     end
 
+    scope :contains_all_tag_ids, -> (tag_ids) do
+      matcher = ::ActsAsTaggableOn::Tagging
+        .where(taggable_type: Ci::Runner)
+        .where(context: 'tags')
+        .where('taggable_id = ci_runners.id')
+        .where(tag_id: tag_ids).select('count(*)')
+
+      where("(?)=(?)", matcher, tag_ids.count)
+    end
+
     validate :tag_constraints
     validates :access_level, presence: true
     validates :runner_type, presence: true
@@ -171,12 +182,6 @@ module Ci
       runner_projects.any?
     end
 
-    def can_pick?(build)
-      return false if self.ref_protected? && !build.protected?
-
-      assignable_for?(build.project_id) && accepting_tags?(build)
-    end
-
     def only_for?(project)
       projects == [project]
     end
@@ -223,9 +228,33 @@ module Ci
       self.update_columns(values) if persist_cached_data?
     end
 
-    def pick_build!(build)
-      if can_pick?(build)
-        tick_runner_queue
+    def pick_build!
+      tick_runner_queue
+    end
+
+    def all_projects
+      case runner_type
+      when 'instance_type'
+        Project.all
+          .with_shared_runners_enabled
+          .without_deleted
+          .with_builds_enabled
+
+      when 'group_type'
+        groups = ::Group.joins(:runner_namespaces).merge(runner_namespaces)
+        hierarchy_groups = Gitlab::GroupHierarchy.new(groups).base_and_descendants
+        Project.where(namespace_id: hierarchy_groups)
+          .with_group_runners_enabled
+          .without_deleted
+          .with_builds_enabled
+
+      when 'project_type'
+        projects
+          .without_deleted
+          .with_builds_enabled
+
+      else
+        raise ArgumentError, 'invalid runner_type'
       end
     end
 
@@ -259,10 +288,6 @@ module Ci
       end
     end
 
-    def assignable_for?(project_id)
-      self.class.owned_or_shared(project_id).where(id: self.id).any?
-    end
-
     def no_projects
       if projects.any?
         errors.add(:runner, 'cannot have projects assigned')
@@ -292,9 +317,5 @@ module Ci
         errors.add(:is_shared, 'is not equal to instance_type?')
       end
     end
-
-    def accepting_tags?(build)
-      (run_untagged? || build.has_tags?) && (build.tag_list - tag_list).empty?
-    end
   end
 end
diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb
index 9bdbb2c0d99..8841f0dd43b 100644
--- a/app/services/ci/register_job_service.rb
+++ b/app/services/ci/register_job_service.rb
@@ -14,28 +14,12 @@ module Ci
     end
 
     def execute
-      builds =
-        if runner.shared?
-          builds_for_shared_runner
-        elsif runner.group_type?
-          builds_for_group_runner
-        else
-          builds_for_project_runner
-        end
+      builds = Ci::RunnerBuildsMatcherService.new.pending_builds_for_runner(runner)
+      builds = builds_for_shared_runner(builds) if runner.shared?
 
       valid = true
 
-      # 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
-
       builds.find do |build|
-        next unless runner.can_pick?(build)
-
         begin
           # In case when 2 runners try to assign the same build, second runner will be declined
           # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method.
@@ -68,47 +52,20 @@ module Ci
 
     private
 
-    def builds_for_shared_runner
-      new_builds.
-        # don't run projects which have not enabled shared runners and builds
-        joins(:project).where(projects: { shared_runners_enabled: true, pending_delete: false })
-        .joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id')
-        .where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0').
-
+    def builds_for_shared_runner(new_builds)
       # Implement fair scheduling
       # this returns builds that are ordered by number of running builds
       # we prefer projects that don't use shared runners at all
-      joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.project_id=project_builds.project_id")
+      new_builds
+        .joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.project_id=project_builds.project_id")
         .order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC')
     end
 
-    def builds_for_project_runner
-      new_builds.where(project: runner.projects.without_deleted.with_builds_enabled).order('id ASC')
-    end
-
-    def builds_for_group_runner
-      # Workaround for weird Rails bug, that makes `runner.groups.to_sql` to return `runner_id = NULL`
-      groups = ::Group.joins(:runner_namespaces).merge(runner.runner_namespaces)
-
-      hierarchy_groups = Gitlab::GroupHierarchy.new(groups).base_and_descendants
-      projects = Project.where(namespace_id: hierarchy_groups)
-        .with_group_runners_enabled
-        .with_builds_enabled
-        .without_deleted
-      new_builds.where(project: projects).order('id ASC')
-    end
-
     def running_builds_for_shared_runners
       Ci::Build.running.where(runner: Ci::Runner.shared)
         .group(:project_id).select(:project_id, 'count(*) AS running_builds')
     end
 
-    def new_builds
-      builds = Ci::Build.pending.unstarted
-      builds = builds.ref_protected if runner.ref_protected?
-      builds
-    end
-
     def register_failure
       failed_attempt_counter.increment
       attempt_counter.increment
diff --git a/app/services/ci/runner_builds_matcher_service.rb b/app/services/ci/runner_builds_matcher_service.rb
new file mode 100644
index 00000000000..f67d2890497
--- /dev/null
+++ b/app/services/ci/runner_builds_matcher_service.rb
@@ -0,0 +1,43 @@
+# This class requires that valid_runners_for_pending_build and pending_builds_for_runner are complementary
+# One focues on getting runners, the other one on getting builds
+# They are highly optimised to be a single SQL query
+
+module Ci
+  class RunnerBuildsMatcherService
+    def valid_runners_for_pending_build(build)
+      runners = build.project.all_runners
+
+      # select only protected runners
+      runners = runners.ref_protected if build.ref_protected?
+
+      # pick only untagged runners, or ones containing all our tags
+      if build.tag_ids.any?
+        runners = runners.contains_all_tag_ids(build.tag_ids)
+      else
+        runners = runners.run_untagged
+      end
+
+      runners
+    end
+
+    def pending_builds_for_runner(runner)
+      builds = Ci::Build.pending.unstarted
+
+      # get builds only for matching projects
+      builds = builds.joins(:project).merge(runner.all_projects)
+
+      # pick only protected builds
+      builds = builds.ref_protected if runner.ref_protected?
+
+      # pick builds that does not have other tags than runner's one
+      builds = builds.matches_tag_ids(runner.tag_ids)
+
+      # pick builds that have at least one tag
+      unless runner.run_untagged?
+        builds = builds.with_any_tags
+      end
+
+      builds
+    end
+  end
+end
diff --git a/app/services/ci/update_build_queue_service.rb b/app/services/ci/update_build_queue_service.rb
index 41b1c144c3e..a0358e5a02c 100644
--- a/app/services/ci/update_build_queue_service.rb
+++ b/app/services/ci/update_build_queue_service.rb
@@ -1,13 +1,7 @@
 module Ci
   class UpdateBuildQueueService
     def execute(build)
-      tick_for(build, build.project.all_runners)
-    end
-
-    private
-
-    def tick_for(build, runners)
-      runners.each do |runner|
+      Ci::RunnerBuildsMatcherService.new.execute(build) do |runner|
         runner.pick_build!(build)
       end
     end