zeroshade commented on issue #2443: URL: https://github.com/apache/iceberg-go/issues/2443#issuecomment-6776148919
Thanks @HotSushi for the write-up and @AynSiddiqui for picking it up. Splitting the work like Java does (headers in #2492, then retries and the lifetime limit in a follow-up) works for me. A few decisions to settle before the retry PR: 1. **Key scope: one key per HTTP call and its resends, not per transaction.** Since #2462, transactions retry on `ErrCommitFailed` (409) by default and rebuild the update against fresh metadata. Each of those attempts is a different request and needs a new key. If the key were reused across them, the server would return the stored 409 for the first attempt every time and the retry loop could never succeed. Only resends of the exact same request after a 5xx, timeout or connection error should reuse the key. Never resend on a 4xx. 2. **The lifetime limits resends, measured from the first attempt.** After `idempotency-key-lifetime` has passed, the server may have forgotten the key. A late resend of a commit that actually landed would then most likely fail its own requirements with a 409, reporting failure for a commit that succeeded. Once the window has passed, return `ErrCommitStateUnknown` instead of resending. A missing or zero lifetime means no keyed resends. 3. **A configured default `Idempotency-Key` header.** If someone sets `Idempotency-Key` as a static default header, the same key goes out on several different mutations, and a server may return one call's stored response for another. When the server advertises a lifetime, I'd drop a configured default on every request instead of only overriding it on the covered mutations, or reject it at config time. 4. **Create view:** the spec doesn't list the header there, but Java and the proposed PyIceberg change both send it. I'd match them. Also worth noting for the docs: once the header is present, Go's `net/http` treats the POST as safe to resend and resends it on its own when a reused connection drops. So #2492 already changes behaviour without an explicit retry loop. That's fine because it's gated on the server advertising support, and `TestMutationTransportReplay` covers it. When the retry PR lands, the scan planning POSTs should follow the same rules, since they currently generate and resend keys whether or not the server advertises support. -- 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]
