Back off StoreScansWorker retries on lease contention

What does this MR do and why?

Security::StoreGroupedScansService takes an exclusive lease for each pipeline and report file type. The lease TTL is 30 minutes.

Before this change, a job that did not get the lease waited for it in its Sidekiq thread. It tried 11 times with 3 seconds between attempts, so the thread was blocked for about 30 seconds. Then the service raised Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError. Security::StoreScansWorker had retry: 3 and the default Sidekiq delays. All 4 attempts ran in about 3 to 4 minutes. If the lease holder took longer than that, the waiting job failed permanently and did not store its scans.

This MR makes these changes:

  1. StoreGroupedScansService calls in_lock with retries: 0. The job tries for the lease one time and does not sleep in the thread.
  2. When the lease is held, the service raises the new Security::StoreGroupedScansService::LeaseTakenError. This class is a subclass of Gitlab::SidekiqMiddleware::RetryError. Sidekiq retries the job, and the error does not go to Sentry or count against the execution SLI.
  3. StoreScansWorker gets a sidekiq_retry_in block. For LeaseTakenError, the retry delays are 1, 2, 4, 8 and 16 minutes. The total is 31 minutes, which is longer than the lease TTL. For all other errors, the block returns nil and Sidekiq uses its default delay.
  4. StoreScansWorker retries 5 times instead of 3, so that all of these retries can run. Other errors also get these 2 extra retries, with the default delay.

StoreScansService is the only production caller of StoreGroupedScansService and StoreGroupedSbomScansService. StoreScansWorker is the only caller of StoreScansService. There is no web or other caller that depends on the old wait behavior. The other callers are specs.

References

Screenshots or screen recordings

Not applicable. This change has no UI.

How to set up and validate locally

  1. Run the specs:

    bundle exec rspec \
      ee/spec/services/security/store_grouped_scans_service_spec.rb \
      ee/spec/workers/security/store_scans_worker_spec.rb \
      ee/spec/services/security/store_grouped_sbom_scans_service_spec.rb \
      ee/spec/services/security/store_scans_service_spec.rb
  2. Optional. In a Rails console, take the lease for a pipeline and then run the service for the same pipeline and file type:

    pipeline = Ci::Pipeline.last
    Gitlab::ExclusiveLease.new("store_grouped_scans:#{pipeline.id}:sast", timeout: 1.minute).try_obtain
    Security::StoreGroupedScansService.execute([], pipeline, 'sast')

    The service raises Security::StoreGroupedScansService::LeaseTakenError immediately. It does not wait for about 30 seconds.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Gregory Havenga

Merge request reports

Loading
Loading