fix(driver/s3-v2): make retry delayer stateless

What

Replaces the s3_v2 driver's shared, stateful retry backoff with a stateless closed-form delayer. Also fixes a fragile unset loop in the s3:seaweedfs integration job (folded in so this MR's own pipeline can pass).

Why (driver fix)

registry/storage/driver/s3-aws/v2/requestcontrol.go built a single cenkalti/backoff.ExponentialBackOff in newCustomDelayer and shared it across every request and retry for the whole life of the client. That object is stateful and not safe for concurrent use, which caused two bugs:

  1. Data race. Concurrent S3 requests read/write the backoff's internal currentInterval through BackoffDelay; go test -race flags it.
  2. Backoff never resets. The interval only grew and stuck at the 60s cap, so a fresh request's first retry could wait ~60s instead of starting near 500ms.

The v1 predecessor (s3-aws/s3wrapper.go) built a fresh backoff per request (CR commit 529df00a0); the v2 rewrite regressed that to a single shared object.

How (driver fix)

BackoffDelay now calls a stateless delayForAttempt that computes the delay in closed form from the attempt number the SDK passes in (attempts are numbered from 1):

interval = min(InitialInterval * Multiplier^(attempt-1), MaxInterval)
delay    = jitter(interval) in [interval*(1-rf), interval*(1+rf))

It holds no mutable state, so a single delayer is safe to share across concurrent requests and every request restarts the curve at attempt 1. The parameters are the same common.Default* values the driver used before, so retry timing is unchanged. The closed form is exact: cenkalti only ever caps an interval to MaxInterval when the raw exponential value would already exceed it, which min(...) reproduces.

This file no longer imports cenkalti/backoff directly, but the driver still depends on it elsewhere (azure v2, the common package, GC, importer, notifications), so it stays in go.mod.

CI fix (second commit)

The s3:seaweedfs integration job unset S3/AWS variables with for i in \env | grep -E 'S3|AWS' | cut -d= -f1`; do unset $i; done. envprints multi-line variable values across lines, so a continuation line matched andcut produced a stray token (-r), which unsetparsed as a flag and aborted the job undererrexit (unset: -r: invalid option) — before any build or test. The job now lists variable names only via compgen -vand usesunset --`. This is a pre-existing failure independent of the driver change.

Tests

requestcontrol_test.go covers delayForAttempt:

  • Table-driven bounds: the non-positive guard and attempt 1 start in the InitialInterval window, attempt 2 grows by the multiplier, a high attempt saturates at MaxInterval, and every delay is positive.
  • 1000 calls for attempt 1 stay within that window, and the observed min/max populate both halves (catching a one-sided jitter or sign error).
  • A concurrent test that fails under -race against shared mutable state.

The driver retryer/throttler (customRetryer, including the metrics.StorageBackendRetry call) is unchanged.

This backports artifact-registry!635.

Related to gitlab-org/ops/artifact-registry#204 (closed)

Edited by Pawel Rozlach

Merge request reports

Loading