Merge branch 'extract_some_overrident_lb_calls' into 'master'

Database Load Balancing feature available in GitLab Free [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!62739

app/models/ci/build.rb

@@ -214,6 +214,8 @@ module Ci
     before_save :ensure_token
     before_destroy { unscoped_project }
 
+    after_save :stick_build_if_status_changed
+
     after_create unless: :importing? do |build|
       run_after_commit { BuildHooksWorker.perform_async(build.id) }
     end
@@ -1082,6 +1084,13 @@ module Ci
 
     private
 
+    def stick_build_if_status_changed
+      return unless saved_change_to_status?
+      return unless running?
+
+      ::Gitlab::Database::LoadBalancing::Sticking.stick(:build, id)
+    end
+
     def status_commit_hooks
       @status_commit_hooks ||= []
     end

app/models/ci/runner.rb

@@ -338,6 +338,14 @@ module Ci
     end
 
     def tick_runner_queue
+      ##
+      # We only stick a runner to primary database to be able to detect the
+      # replication lag in `EE::Ci::RegisterJobService#execute`. The
+      # intention here is not to execute `Ci::RegisterJobService#execute` on
+      # the primary database.
+      #
+      ::Gitlab::Database::LoadBalancing::Sticking.stick(:runner, id)
+
       SecureRandom.hex.tap do |new_update|
         ::Gitlab::Workhorse.set_key_and_notify(runner_queue_key, new_update,
           expire: RUNNER_QUEUE_EXPIRY_TIME, overwrite: true)
@@ -355,13 +363,20 @@ module Ci
     end
 
     def heartbeat(values)
-      values = values&.slice(:version, :revision, :platform, :architecture, :ip_address, :config) || {}
-      values[:contacted_at] = Time.current
+      ##
+      # We can safely ignore writes performed by a runner heartbeat. We do
+      # not want to upgrade database connection proxy to use the primary
+      # database after heartbeat write happens.
+      #
+      ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes do
+        values = values&.slice(:version, :revision, :platform, :architecture, :ip_address, :config) || {}
+        values[:contacted_at] = Time.current
 
-      cache_attributes(values)
+        cache_attributes(values)
 
-      # We save data without validation, it will always change due to `contacted_at`
-      self.update_columns(values) if persist_cached_data?
+        # We save data without validation, it will always change due to `contacted_at`
+        self.update_columns(values) if persist_cached_data?
+      end
     end
 
     def pick_build!(build)

app/services/ci/register_job_service.rb

@@ -128,6 +128,12 @@ module Ci
     # rubocop: enable CodeReuse/ActiveRecord
 
     def retrieve_queue(queue_query_proc)
+      ##
+      # We want to reset a load balancing session to discard the side
+      # effects of writes that could have happened prior to this moment.
+      #
+      ::Gitlab::Database::LoadBalancing::Session.clear_session
+
       @metrics.observe_queue_time(:retrieve, @runner.runner_type) do
         queue_query_proc.call
       end

doc/administration/database_load_balancing.md

@@ -4,9 +4,10 @@ group: Database
 info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
 ---
 
-# Database Load Balancing **(PREMIUM SELF)**
+# Database Load Balancing **(FREE SELF)**
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1283) in [GitLab Premium](https://about.gitlab.com/pricing/) 9.0.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1283) in [GitLab Premium](https://about.gitlab.com/pricing/) 9.0.
+> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60894) from GitLab Premium to GitLab Free in 14.0. See the [instructions below](#load-balancing-in-gitlab).
 
 Distribute read-only queries among multiple database servers.
 
@@ -103,6 +104,17 @@ the following. This will balance the load between `host1.example.com` and
 
 1. Save the file and [restart GitLab](restart_gitlab.md#installations-from-source) for the changes to take effect.
 
+### Load balancing in GitLab Free
+
+Database load balancing was moved from GitLab Premium to GitLab Free in
+14.0 with some [known limitations](https://gitlab.com/gitlab-org/gitlab/-/issues/327902).
+For example, requesting new jobs for runners may fail. Hence, this feature is **not ready for production use** in GitLab Free.
+
+To use database load balancing in GitLab Free, set the `ENABLE_LOAD_BALANCING_FOR_FOSS`
+[environment
+variable](https://docs.gitlab.com/omnibus/settings/environment-variables.html)
+to `true`.
+
 ### Enable the load balancer for Sidekiq
 
 Sidekiq mostly writes to the database, which means that most of its traffic hits the

ee/app/models/ee/ci/build.rb

@@ -31,7 +31,6 @@ module EE
 
         has_many :security_scans, class_name: 'Security::Scan'
 
-        after_save :stick_build_if_status_changed
         after_commit :track_ci_secrets_management_usage, on: :create
         delegate :service_specification, to: :runner_session, allow_nil: true
 
@@ -62,13 +61,6 @@ module EE
         project.shared_runners_minutes_limit_enabled? && runner&.minutes_cost_factor(project.visibility_level)&.positive?
       end
 
-      def stick_build_if_status_changed
-        return unless saved_change_to_status?
-        return unless running?
-
-        ::Gitlab::Database::LoadBalancing::Sticking.stick(:build, id)
-      end
-
       def log_geo_deleted_event
         # It is not needed to generate a Geo deleted event
         # since Legacy Artifacts are migrated to multi-build artifacts

ee/app/models/ee/ci/runner.rb

@@ -5,27 +5,6 @@ module EE
     module Runner
       extend ActiveSupport::Concern
 
-      def tick_runner_queue
-        ##
-        # We only stick a runner to primary database to be able to detect the
-        # replication lag in `EE::Ci::RegisterJobService#execute`. The
-        # intention here is not to execute `Ci::RegisterJobService#execute` on
-        # the primary database.
-        #
-        ::Gitlab::Database::LoadBalancing::Sticking.stick(:runner, id)
-
-        super
-      end
-
-      def heartbeat(values)
-        ##
-        # We can safely ignore writes performed by a runner heartbeat. We do
-        # not want to upgrade database connection proxy to use the primary
-        # database after heartbeat write happens.
-        #
-        ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes { super }
-      end
-
       def minutes_cost_factor(visibility_level)
         ::Gitlab::Ci::Minutes::CostFactor.new(runner_matcher).for_visibility(visibility_level)
       end

ee/app/services/ee/ci/register_job_service.rb

@@ -28,16 +28,6 @@ module EE
         end
       end
 
-      def retrieve_queue(queue_query_proc)
-        ##
-        # We want to reset a load balancing session to discard the side
-        # effects of writes that could have happened prior to this moment.
-        #
-        ::Gitlab::Database::LoadBalancing::Session.clear_session
-
-        super
-      end
-
       def builds_for_shared_runner
         # if disaster recovery is enabled, we disable quota
         if ::Feature.enabled?(:ci_queueing_disaster_recovery, runner, type: :ops, default_enabled: :yaml)

ee/spec/models/ci/build_spec.rb

@@ -107,20 +107,6 @@ RSpec.describe Ci::Build do
     end
   end
 
-  describe '#stick_build_if_status_changed' do
-    it 'sticks the build if the status changed' do
-      job = create(:ci_build, :pending)
-
-      allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
-        .and_return(true)
-
-      expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick)
-        .with(:build, job.id)
-
-      job.update!(status: :running)
-    end
-  end
-
   describe '#variables' do
     subject { job.variables }
 

ee/spec/models/ee/ci/runner_spec.rb

@@ -3,22 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe EE::Ci::Runner do
-  describe '#tick_runner_queue' do
-    it 'sticks the runner to the primary and calls the original method' do
-      runner = create(:ci_runner)
-
-      allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
-        .and_return(true)
-
-      expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick)
-        .with(:runner, runner.id)
-
-      expect(Gitlab::Workhorse).to receive(:set_key_and_notify)
-
-      runner.tick_runner_queue
-    end
-  end
-
   describe '#minutes_cost_factor' do
     subject { runner.minutes_cost_factor(visibility_level) }
 

spec/models/ci/build_spec.rb

@@ -320,6 +320,20 @@ RSpec.describe Ci::Build do
     end
   end
 
+  describe '#stick_build_if_status_changed' do
+    it 'sticks the build if the status changed' do
+      job = create(:ci_build, :pending)
+
+      allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
+        .and_return(true)
+
+      expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick)
+        .with(:build, job.id)
+
+      job.update!(status: :running)
+    end
+  end
+
   describe '#enqueue' do
     let(:build) { create(:ci_build, :created) }
 

spec/models/ci/runner_spec.rb

@@ -365,6 +365,22 @@ RSpec.describe Ci::Runner do
     it { is_expected.to eq([@runner1])}
   end
 
+  describe '#tick_runner_queue' do
+    it 'sticks the runner to the primary and calls the original method' do
+      runner = create(:ci_runner)
+
+      allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
+        .and_return(true)
+
+      expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick)
+        .with(:runner, runner.id)
+
+      expect(Gitlab::Workhorse).to receive(:set_key_and_notify)
+
+      runner.tick_runner_queue
+    end
+  end
+
   describe '#can_pick?' do
     using RSpec::Parameterized::TableSyntax