1991santhu opened a new pull request, #1640: URL: https://github.com/apache/iceberg-go/pull/1640
## The problem `calculateBackoff` is a pure function of the attempt number and the configured bounds: ```go return minWait << attempt ``` so every client computes the same sequence of delays — 100ms, 200ms, 400ms, 800ms — for a given configuration. That matters more here than in a typical retry loop, because of *why* a client is in the loop. `AcquireLock` only retries when the lock was not granted immediately, which means another client holds it. Multiple waiters are the precondition, not an unlucky coincidence. They therefore sleep for identical durations and issue their `CheckLock` calls at the same instants, so each polling round reaches the metastore as a burst — the load pattern backoff exists to prevent. ## The change Apply equal jitter to the value `calculateBackoff` returns: ```go waitTime := applyJitter(calculateBackoff(attempt, opts.LockMinWaitTime, opts.LockMaxWaitTime)) ``` **Jitter is applied at the call site, not inside `calculateBackoff`.** That is deliberate. `TestCalculateBackoff` asserts exact durations for each attempt, and determinism is a reasonable property for that function to guarantee — randomising it in place would mean rewriting those assertions to accept ranges. Leaving the calculation pure keeps the existing contract and its tests untouched, and makes the new behaviour separately testable. Half the interval is kept as a floor rather than using full jitter, so a waiter never re-polls immediately after a failed check. ## Testing `go test ./catalog/hive/...` passes, run with `-count=2`. `go vet` clean. Existing `TestCalculateBackoff` is unmodified and still passes. New tests cover: the result always lies within `[d/2, d]`, sampled 500 times across four magnitudes; the value varies across 500 draws, which fails if the implementation regresses to deterministic; and non-positive inputs pass through unchanged. ## Note I have not reproduced multi-client metastore contention — the reasoning is from the code. If maintainers would prefer full jitter over equal jitter, or the bound configurable rather than fixed at half, both are small changes and I am happy to make them. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
