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:
StoreGroupedScansServicecallsin_lockwithretries: 0. The job tries for the lease one time and does not sleep in the thread.- When the lease is held, the service raises the new
Security::StoreGroupedScansService::LeaseTakenError. This class is a subclass ofGitlab::SidekiqMiddleware::RetryError. Sidekiq retries the job, and the error does not go to Sentry or count against the execution SLI. StoreScansWorkergets asidekiq_retry_inblock. ForLeaseTakenError, 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 returnsniland Sidekiq uses its default delay.StoreScansWorkerretries 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
-
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 -
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::LeaseTakenErrorimmediately. 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.