Merge branch '325333_optimize_query' into 'master'

Apply optimizations to JobsController#show.json [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!57367

app/models/ci/build.rb

@@ -33,6 +33,7 @@ module Ci
     }.freeze
 
     DEGRADATION_THRESHOLD_VARIABLE_NAME = 'DEGRADATION_THRESHOLD'
+    RUNNERS_STATUS_CACHE_EXPIRATION = 1.minute
 
     has_one :deployment, as: :deployable, class_name: 'Deployment'
     has_one :pending_state, class_name: 'Ci::BuildPendingState', inverse_of: :build
@@ -702,7 +703,23 @@ module Ci
     end
 
     def any_runners_online?
-      project.any_active_runners? { |runner| runner.match_build_if_online?(self) }
+      if Feature.enabled?(:runners_cached_states, project, default_enabled: :yaml)
+        cache_for_online_runners do
+          project.any_online_runners? { |runner| runner.match_build_if_online?(self) }
+        end
+      else
+        project.any_active_runners? { |runner| runner.match_build_if_online?(self) }
+      end
+    end
+
+    def any_runners_available?
+      if Feature.enabled?(:runners_cached_states, project, default_enabled: :yaml)
+        cache_for_available_runners do
+          project.active_runners.exists?
+        end
+      else
+        project.any_active_runners?
+      end
     end
 
     def stuck?
@@ -1107,6 +1124,20 @@ module Ci
         .to_a
         .include?(exit_code)
     end
+
+    def cache_for_online_runners(&block)
+      Rails.cache.fetch(
+        ['has-online-runners', id],
+        expires_in: RUNNERS_STATUS_CACHE_EXPIRATION
+      ) { yield }
+    end
+
+    def cache_for_available_runners(&block)
+      Rails.cache.fetch(
+        ['has-available-runners', project.id],
+        expires_in: RUNNERS_STATUS_CACHE_EXPIRATION
+      ) { yield }
+    end
   end
 end
 

app/models/project.rb

@@ -1715,10 +1715,15 @@ class Project < ApplicationRecord
     end
   end
 
+  # Deprecated: https://gitlab.com/gitlab-org/gitlab/-/issues/326989
   def any_active_runners?(&block)
     active_runners_with_tags.any?(&block)
   end
 
+  def any_online_runners?(&block)
+    online_runners_with_tags.any?(&block)
+  end
+
   def valid_runners_token?(token)
     self.runners_token && ActiveSupport::SecurityUtils.secure_compare(token, self.runners_token)
   end
@@ -2741,9 +2746,11 @@ class Project < ApplicationRecord
   end
 
   def active_runners_with_tags
-    strong_memoize(:active_runners_with_tags) do
-      active_runners.with_tags
-    end
+    @active_runners_with_tags ||= active_runners.with_tags
+  end
+
+  def online_runners_with_tags
+    @online_runners_with_tags ||= active_runners_with_tags.online
   end
 end
 

app/serializers/build_details_entity.rb

@@ -99,7 +99,7 @@ class BuildDetailsEntity < JobEntity
     end
 
     expose :available do |build|
-      project.any_active_runners?
+      build.any_runners_available?
     end
 
     expose :settings_path, if: -> (*) { can_admin_build? } do |build|

changelogs/unreleased/325333_optimize_query.yml

+---
+title: Apply optimizations to JobsController#show.json
+merge_request: 57367
+author:
+type: performance

config/feature_flags/development/runners_cached_states.yml

+---
+name: runners_cached_states
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57367
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326989
+milestone: '13.11'
+type: development
+group: group::source code
+default_enabled: false

ee/spec/models/project_spec.rb

@@ -1086,37 +1086,40 @@ RSpec.describe Project do
     end
   end
 
-  describe '#any_runners_limit' do
-    let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) }
-    let(:specific_runner) { create(:ci_runner, :project) }
-    let(:shared_runner) { create(:ci_runner, :instance) }
+  describe '#any_active_runners?' do
+    let!(:shared_runner) { create(:ci_runner, :instance) }
 
-    context 'for shared runners enabled' do
-      let(:shared_runners_enabled) { true }
+    it { expect(project.any_active_runners?).to be_truthy }
 
-      before do
-        shared_runner
+    context 'with used pipeline minutes' do
+      let(:namespace) { create(:namespace, :with_used_build_minutes_limit) }
+      let(:project) do
+        create(:project,
+               namespace: namespace,
+               shared_runners_enabled: true)
       end
 
-      it 'has a shared runner' do
-        expect(project.any_active_runners?).to be_truthy
+      it 'does not have any active runners' do
+        expect(project.any_active_runners?).to be_falsey
       end
+    end
+  end
 
-      it 'checks the presence of shared runner' do
-        expect(project.any_active_runners? { |runner| runner == shared_runner }).to be_truthy
-      end
+  describe '#any_online_runners?' do
+    let!(:shared_runner) { create(:ci_runner, :instance, :online) }
 
-      context 'with used pipeline minutes' do
-        let(:namespace) { create(:namespace, :with_used_build_minutes_limit) }
-        let(:project) do
-          create(:project,
-            namespace: namespace,
-            shared_runners_enabled: shared_runners_enabled)
-        end
+    it { expect(project.any_online_runners?).to be_truthy }
 
-        it 'does not have a shared runner' do
-          expect(project.any_active_runners?).to be_falsey
-        end
+    context 'with used pipeline minutes' do
+      let(:namespace) { create(:namespace, :with_used_build_minutes_limit) }
+      let(:project) do
+        create(:project,
+               namespace: namespace,
+               shared_runners_enabled: true)
+      end
+
+      it 'does not have any online runners' do
+        expect(project.any_online_runners?).to be_falsey
       end
     end
   end

spec/models/ci/build_spec.rb

@@ -585,6 +585,68 @@ RSpec.describe Ci::Build do
         is_expected.to be_falsey
       end
     end
+
+    context 'with runners_cached_states feature flag enabled' do
+      before do
+        stub_feature_flags(runners_cached_states: true)
+      end
+
+      it 'caches the result in Redis' do
+        expect(Rails.cache).to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute)
+
+        build.any_runners_online?
+      end
+    end
+
+    context 'with runners_cached_states feature flag disabled' do
+      before do
+        stub_feature_flags(runners_cached_states: false)
+      end
+
+      it 'does not cache' do
+        expect(Rails.cache).not_to receive(:fetch).with(['has-online-runners', build.id], expires_in: 1.minute)
+
+        build.any_runners_online?
+      end
+    end
+  end
+
+  describe '#any_runners_available?' do
+    subject { build.any_runners_available? }
+
+    context 'when no runners' do
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when there are runners' do
+      let!(:runner) { create(:ci_runner, :project, projects: [build.project]) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'with runners_cached_states feature flag enabled' do
+      before do
+        stub_feature_flags(runners_cached_states: true)
+      end
+
+      it 'caches the result in Redis' do
+        expect(Rails.cache).to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute)
+
+        build.any_runners_available?
+      end
+    end
+
+    context 'with runners_cached_states feature flag disabled' do
+      before do
+        stub_feature_flags(runners_cached_states: false)
+      end
+
+      it 'does not cache' do
+        expect(Rails.cache).not_to receive(:fetch).with(['has-available-runners', build.project.id], expires_in: 1.minute)
+
+        build.any_runners_available?
+      end
+    end
   end
 
   describe '#artifacts?' do

spec/models/project_spec.rb

@@ -1629,6 +1629,8 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe '#any_active_runners?' do
+    subject { project.any_active_runners? }
+
     context 'shared runners' do
       let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) }
       let(:specific_runner) { create(:ci_runner, :project, projects: [project]) }
@@ -1638,19 +1640,19 @@ RSpec.describe Project, factory_default: :keep do
         let(:shared_runners_enabled) { false }
 
         it 'has no runners available' do
-          expect(project.any_active_runners?).to be_falsey
+          is_expected.to be_falsey
         end
 
         it 'has a specific runner' do
           specific_runner
 
-          expect(project.any_active_runners?).to be_truthy
+          is_expected.to be_truthy
         end
 
         it 'has a shared runner, but they are prohibited to use' do
           shared_runner
 
-          expect(project.any_active_runners?).to be_falsey
+          is_expected.to be_falsey
         end
 
         it 'checks the presence of specific runner' do
@@ -1672,7 +1674,7 @@ RSpec.describe Project, factory_default: :keep do
         it 'has a shared runner' do
           shared_runner
 
-          expect(project.any_active_runners?).to be_truthy
+          is_expected.to be_truthy
         end
 
         it 'checks the presence of shared runner' do
@@ -1698,13 +1700,13 @@ RSpec.describe Project, factory_default: :keep do
         let(:group_runners_enabled) { false }
 
         it 'has no runners available' do
-          expect(project.any_active_runners?).to be_falsey
+          is_expected.to be_falsey
         end
 
         it 'has a group runner, but they are prohibited to use' do
           group_runner
 
-          expect(project.any_active_runners?).to be_falsey
+          is_expected.to be_falsey
         end
       end
 
@@ -1714,7 +1716,7 @@ RSpec.describe Project, factory_default: :keep do
         it 'has a group runner' do
           group_runner
 
-          expect(project.any_active_runners?).to be_truthy
+          is_expected.to be_truthy
         end
 
         it 'checks the presence of group runner' do
@@ -1732,6 +1734,126 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#any_online_runners?' do
+    subject { project.any_online_runners? }
+
+    context 'shared runners' do
+      let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) }
+      let(:specific_runner) { create(:ci_runner, :project, :online, projects: [project]) }
+      let(:shared_runner) { create(:ci_runner, :instance, :online) }
+      let(:offline_runner) { create(:ci_runner, :instance) }
+
+      context 'for shared runners disabled' do
+        let(:shared_runners_enabled) { false }
+
+        it 'has no runners available' do
+          is_expected.to be_falsey
+        end
+
+        it 'has a specific runner' do
+          specific_runner
+
+          is_expected.to be_truthy
+        end
+
+        it 'has a shared runner, but they are prohibited to use' do
+          shared_runner
+
+          is_expected.to be_falsey
+        end
+
+        it 'checks the presence of specific runner' do
+          specific_runner
+
+          expect(project.any_online_runners? { |runner| runner == specific_runner }).to be_truthy
+        end
+
+        it 'returns false if match cannot be found' do
+          specific_runner
+
+          expect(project.any_online_runners? { false }).to be_falsey
+        end
+
+        it 'returns false if runner is offline' do
+          offline_runner
+
+          is_expected.to be_falsey
+        end
+      end
+
+      context 'for shared runners enabled' do
+        let(:shared_runners_enabled) { true }
+
+        it 'has a shared runner' do
+          shared_runner
+
+          is_expected.to be_truthy
+        end
+
+        it 'checks the presence of shared runner' do
+          shared_runner
+
+          expect(project.any_online_runners? { |runner| runner == shared_runner }).to be_truthy
+        end
+
+        it 'returns false if match cannot be found' do
+          shared_runner
+
+          expect(project.any_online_runners? { false }).to be_falsey
+        end
+      end
+    end
+
+    context 'group runners' do
+      let(:project) { create(:project, group_runners_enabled: group_runners_enabled) }
+      let(:group) { create(:group, projects: [project]) }
+      let(:group_runner) { create(:ci_runner, :group, :online, groups: [group]) }
+      let(:offline_runner) { create(:ci_runner, :group, groups: [group]) }
+
+      context 'for group runners disabled' do
+        let(:group_runners_enabled) { false }
+
+        it 'has no runners available' do
+          is_expected.to be_falsey
+        end
+
+        it 'has a group runner, but they are prohibited to use' do
+          group_runner
+
+          is_expected.to be_falsey
+        end
+      end
+
+      context 'for group runners enabled' do
+        let(:group_runners_enabled) { true }
+
+        it 'has a group runner' do
+          group_runner
+
+          is_expected.to be_truthy
+        end
+
+        it 'has an offline group runner' do
+          offline_runner
+
+          is_expected.to be_falsey
+        end
+
+        it 'checks the presence of group runner' do
+          group_runner
+
+          expect(project.any_online_runners? { |runner| runner == group_runner }).to be_truthy
+        end
+
+        it 'returns false if match cannot be found' do
+          group_runner
+
+          expect(project.any_online_runners? { false }).to be_falsey
+        end
+      end
+    end
+  end
+
   describe '#shared_runners' do
     let!(:runner) { create(:ci_runner, :instance) }