Merge branch 'feature/gb/build-update-exponential-backoff' into 'master'

Add build status update exponential backoff mechanism

See merge request gitlab-org/gitlab!42693

app/services/ci/update_build_state_service.rb

@@ -2,7 +2,7 @@
 
 module Ci
   class UpdateBuildStateService
-    Result = Struct.new(:status, keyword_init: true)
+    Result = Struct.new(:status, :backoff, keyword_init: true)
 
     ACCEPT_TIMEOUT = 5.minutes.freeze
 
@@ -28,7 +28,9 @@ module Ci
     private
 
     def accept_build_state!
-      if Time.current - ensure_pending_state.created_at > ACCEPT_TIMEOUT
+      state_created = ensure_pending_state.created_at
+
+      if Time.current - state_created > ACCEPT_TIMEOUT
         metrics.increment_trace_operation(operation: :discarded)
 
         return update_build_state!
@@ -40,7 +42,9 @@ module Ci
 
       metrics.increment_trace_operation(operation: :accepted)
 
-      Result.new(status: 202)
+      ::Gitlab::Ci::Runner::Backoff.new(state_created).then do |backoff|
+        Result.new(status: 202, backoff: backoff.to_seconds)
+      end
     end
 
     def overwrite_trace!

lib/api/ci/runner.rb

@@ -181,6 +181,7 @@ module API
             .new(job, declared_params(include_missing: false))
 
           service.execute.then do |result|
+            header 'X-GitLab-Trace-Update-Interval', result.backoff
             status result.status
           end
         end

lib/gitlab/ci/runner/backoff.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Runner
+      ##
+      # Runner Backoff class is an implementation of an exponential backoff
+      # used when a runner communicates with GitLab. We typically use it when a
+      # runner retries sending a build status after we created a build pending
+      # state.
+      #
+      # Backoff is calculated based on the backoff slot which is always a power
+      # of 2:
+      #
+      #   0s -  3s   duration -> 1   second backoff
+      #   4s -  7s   duration -> 2  seconds backoff
+      #   8s - 15s   duration -> 4  seconds backoff
+      #  16s  - 31s  duration -> 8  seconds backoff
+      #  32s - 63s   duration -> 16 seconds backoff
+      #  64s - 127s  duration -> 32 seconds backoff
+      # 127s - 256s+ duration -> 64 seconds backoff
+      #
+      # It means that first 15 requests made by a runner will need to respect
+      # following backoffs:
+      #
+      #  0s -> 1 second  backoff (backoff started, slot 0, 2^0 backoff)
+      #  1s -> 1 second  backoff
+      #  2s -> 1 second  backoff
+      #  3s -> 1 seconds backoff
+      #                          (slot 1 - 2^1 backoff)
+      #  4s -> 2 seconds backoff
+      #  6s -> 2 seconds backoff
+      #                          (slot 2 - 2^2 backoff)
+      #  8s -> 4 seconds backoff
+      # 12s -> 4 seconds backoff
+      #                          (slot 3 - 2^3 backoff)
+      # 16s -> 8 seconds backoff
+      # 24s -> 8 seconds backoff
+      #                          (slot 4 - 2^4 backoff)
+      # 32s -> 16 seconds backoff
+      # 48s -> 16 seconds backoff
+      #                          (slot 5 - 2^5 backoff)
+      # 64s -> 32 seconds backoff
+      # 96s -> 32 seconds backoff
+      #                          (slot 6 - 2^6 backoff)
+      # 128s -> 64 seconds backoff
+      #
+      # There is a cap on the backoff - it will never exceed 64 seconds.
+      #
+      class Backoff
+        def initialize(started)
+          @started = started
+
+          if duration < 0
+            raise ArgumentError, 'backoff duration negative'
+          end
+        end
+
+        def duration
+          Time.current - @started
+        end
+
+        def slot
+          return 0 if duration <= 1
+
+          Math.log(duration, 2).floor - 1
+        end
+
+        def to_seconds
+          2**[slot, 6].min
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/runner/backoff_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+require 'active_support/testing/time_helpers'
+
+RSpec.describe Gitlab::Ci::Runner::Backoff do
+  include ActiveSupport::Testing::TimeHelpers
+
+  describe '#duration' do
+    it 'returns backoff duration from start' do
+      freeze_time do
+        described_class.new(5.minutes.ago).then do |backoff|
+          expect(backoff.duration).to eq 5.minutes
+        end
+      end
+    end
+  end
+
+  describe '#slot' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:started, :slot) do
+      1   | 0
+      2   | 0
+      3   | 0
+      4   | 1
+      5   | 1
+      6   | 1
+      7   | 1
+      8   | 2
+      9   | 2
+      10  | 2
+      15  | 2
+      16  | 3
+      31  | 3
+      32  | 4
+      63  | 4
+      64  | 5
+      127 | 5
+      128 | 6
+      250 | 6
+      310 | 7
+      520 | 8
+      999 | 8
+    end
+
+    with_them do
+      it 'falls into an appropaite backoff slot' do
+        freeze_time do
+          backoff = described_class.new(started.seconds.ago)
+          expect(backoff.slot).to eq slot
+        end
+      end
+    end
+  end
+
+  describe '#to_seconds' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:started, :backoff) do
+      1   | 1
+      2   | 1
+      3   | 1
+      4   | 2
+      5   | 2
+      6   | 2
+      7   | 2
+      8   | 4
+      9   | 4
+      10  | 4
+      15  | 4
+      16  | 8
+      31  | 8
+      32  | 16
+      63  | 16
+      64  | 32
+      127 | 32
+      128 | 64
+      250 | 64
+      310 | 64
+      520 | 64
+      999 | 64
+    end
+
+    with_them do
+      it 'calculates backoff based on an appropriate slot' do
+        freeze_time do
+          described_class.new(started.seconds.ago).then do |delay|
+            expect(delay.to_seconds).to eq backoff
+          end
+        end
+      end
+    end
+  end
+end

spec/requests/api/ci/runner/jobs_put_spec.rb

@@ -46,64 +46,59 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
       end
 
       context 'when status is given' do
-        it 'mark job as succeeded' do
+        it 'marks job as succeeded' do
           update_job(state: 'success')
 
-          job.reload
-          expect(job).to be_success
+          expect(job.reload).to be_success
+          expect(response.header).not_to have_key('X-GitLab-Trace-Update-Interval')
         end
 
-        it 'mark job as failed' do
+        it 'marks job as failed' do
           update_job(state: 'failed')
 
-          job.reload
-          expect(job).to be_failed
+          expect(job.reload).to be_failed
           expect(job).to be_unknown_failure
+          expect(response.header).not_to have_key('X-GitLab-Trace-Update-Interval')
         end
 
         context 'when failure_reason is script_failure' do
           before do
             update_job(state: 'failed', failure_reason: 'script_failure')
-            job.reload
           end
 
-          it { expect(job).to be_script_failure }
+          it { expect(job.reload).to be_script_failure }
         end
 
         context 'when failure_reason is runner_system_failure' do
           before do
             update_job(state: 'failed', failure_reason: 'runner_system_failure')
-            job.reload
           end
 
-          it { expect(job).to be_runner_system_failure }
+          it { expect(job.reload).to be_runner_system_failure }
         end
 
         context 'when failure_reason is unrecognized value' do
           before do
             update_job(state: 'failed', failure_reason: 'what_is_this')
-            job.reload
           end
 
-          it { expect(job).to be_unknown_failure }
+          it { expect(job.reload).to be_unknown_failure }
         end
 
         context 'when failure_reason is job_execution_timeout' do
           before do
             update_job(state: 'failed', failure_reason: 'job_execution_timeout')
-            job.reload
           end
 
-          it { expect(job).to be_job_execution_timeout }
+          it { expect(job.reload).to be_job_execution_timeout }
         end
 
         context 'when failure_reason is unmet_prerequisites' do
           before do
             update_job(state: 'failed', failure_reason: 'unmet_prerequisites')
-            job.reload
           end
 
-          it { expect(job).to be_unmet_prerequisites }
+          it { expect(job.reload).to be_unmet_prerequisites }
         end
 
         context 'when unmigrated live trace chunks exist' do
@@ -119,6 +114,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
 
                   expect(job.pending_state).to be_present
                   expect(response).to have_gitlab_http_status(:accepted)
+                  expect(response.header['X-GitLab-Trace-Update-Interval']).to be > 0
                 end
               end
 
@@ -151,6 +147,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
                   expect(job.reload).to be_success
                   expect(job.pending_state).not_to be_present
                   expect(response).to have_gitlab_http_status(:ok)
+                  expect(response.header).not_to have_key('X-GitLab-Trace-Update-Interval')
                 end
               end
             end

spec/services/ci/update_build_state_service_spec.rb

@@ -100,6 +100,12 @@ RSpec.describe Ci::UpdateBuildStateService do
         expect(result.status).to eq 200
       end
 
+      it 'does not set a backoff value' do
+        result = subject.execute
+
+        expect(result.backoff).to be_nil
+      end
+
       it 'increments trace finalized operation metric' do
         execute_with_stubbed_metrics!
 
@@ -126,6 +132,12 @@ RSpec.describe Ci::UpdateBuildStateService do
         expect(result.status).to eq 202
       end
 
+      it 'sets a request backoff value' do
+        result = subject.execute
+
+        expect(result.backoff.to_i).to be > 0
+      end
+
       it 'schedules live chunks for migration' do
         expect(Ci::BuildTraceChunkFlushWorker)
           .to receive(:perform_async)