1991santhu commented on PR #1640: URL: https://github.com/apache/iceberg-go/pull/1640#issuecomment-5210588594
Thanks — the 90s/60s case is a real bug and it was a regression in my own previous commit, not something pre-existing. The replay loop only raises the floor while walking intervals below `maxWait`, so when `minWait >= maxWait` it never runs at all and the floor stayed at `d/2`. I reproduced it before changing anything: lowest observed wait was 30.0014s against a configured 90s minimum, exactly as you described. Fixed in a48bc64 by applying `minWait` as a floor before the replay rather than relying on the replay to reach it. Where `minWait` is at or past the cap the floor now exceeds the interval and the wait comes back unspread, which is the only honest answer when the configured minimum can't be respected any other way. Regression test uses your 90s/60s numbers, and I checked it fails without the fix. Shortened the comment to the invariants. You were right that 31 lines was disproportionate, and I'll note the removed text ended by claiming the wait could never fall below `minWait` — which is exactly what it did. The derivation is in the commit message now. Added the `acquireLocks`-level test. It runs the retry loop through the real call site and bounds the aggregate delay. One caveat worth stating: the lower bound is asserted tightly but the ceiling is loose, because a wall-clock upper bound is the kind of assertion that flakes on a busy runner, and a flaky timing test is worse than none. On the release note: agreed, and I can confirm the number. Defaults are 100ms/1min/4 retries, so the unjittered schedule is 100+200+400+800ms = 1.5s and each draw can add up to one further interval, giving a worst case of 3s. On `options.go` accepting negative durations and `minWait >= maxWait`: agreed that validating there would make the contract clearer, but both predate this PR and fixing them changes behaviour for configurations that currently parse, so I'd rather not widen this one. Happy to open a follow-up if you want it. -- 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]
