Hi All, I agree that checking request contents is almost redundant in this case.
If the randomness quality of Idempotency-Key value is good, collisions are very unlikely on the server side. Given that, any content checks the server performs are essentially validating that clients correctly reuse the generated Idempotency-Key value. (this is mostly the same as my comment on the related Polaris discussion). I'd like to propose making the content check optional so that servers may or may not implement it according to their design principles and constraints and emphasizing that clients should use unique keys (e.g. UUIDs)... basically going with option 2 from Peter's email. I believe this is in line with the SHOULD word used for this case in the IETF draft [1] (section 2.7). [1] https://datatracker.ietf.org/doc/html/draft-ietf-httpapi-idempotency-key-header-06 Thanks, Dmitri. On Thu, Sep 18, 2025 at 7:56 AM Péter Váry <peter.vary.apa...@gmail.com> wrote: > Thanks Huaxin for the proposal, and sorry for the late review - I had a > bit of a busy week. > I have one main question, which I have also added as a comment to the doc: > - Why do we try to compare the request contents when the Idempotency-Key > is the same for the requests? The comparison algorithm is a bit > complicated, and seems brittle to me. Consistent field ordering, maps, and > maybe even inconsistency in upper case/lower case letters might mean > technically the same request. > > In my previous roles (admittedly more than 10 years ago) I was extensively > working on APIs like this, and we have never really succeeded in creating a > good enough "are these 2 requests are really the same semantically" checks. > > I would simplify these requirements, unless there are serious arguments > for the existence of these checks: > > 1. Either check for exact matches - without any magic - this could be > used for detecting issues where the duplication happens on the network > side, or > 2. Rely entirely on the clients to provide the correct Idempotency-Key. > > I would prefer the 2nd. > Otherwise I agree with the contents of the proposal. It is nicely done! > (edited) > > Yufei Gu <flyrain...@gmail.com> ezt írta (időpont: 2025. szept. 18., Cs, > 2:54): > >> Thanks for the proposal. It's a nice feature to make retry more reliable >> and efficient. Left some comments. >> >> Yufei >> >> >> On Mon, Sep 15, 2025 at 3:53 PM Kevin Liu <kevinjq...@apache.org> wrote: >> >>> >>> Thanks for writing up the proposal! Makes sense to add idempotency to >>> mutation requests. >>> >>> It would be helpful to add this feature to both the catalog test >>> framework and the iceberg-rest-fixture >>> <https://github.com/apache/iceberg/blob/754679ddccdf81a97dc65d40f1a2a6fb9f6ee9b0/open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTCatalogServer.java#L112>. >>> The latter is used by the subprojects for testing and would come in handy >>> when we want to test out the client implementation. >>> >>> For other reviewers, the Stripe documentation on idempotency was a >>> helpful read, https://docs.stripe.com/api/idempotent_requests. >>> >>> >>> Best, >>> Kevin Liu >>> >>> On Mon, Sep 15, 2025 at 11:38 AM Szehon Ho <szehon.apa...@gmail.com> >>> wrote: >>> >>>> Hi, >>>> >>>> Sounds like fairly standard practice and makes sense to me in the first >>>> read. >>>> >>>> Thanks, >>>> Szehon >>>> >>>> On Mon, Sep 15, 2025 at 10:09 AM Russell Spitzer < >>>> russellspit...@apache.org> wrote: >>>> >>>>> I think based on the feedback on the proposal and in recent syncs we >>>>> should probably move forward with the actual Spec Change PR so we can see >>>>> what this looks like and move on to a discussion of how the Catalog test >>>>> framework should test this. >>>>> >>>>> On 2025/08/22 18:26:23 huaxin gao wrote: >>>>> > Hi all, >>>>> > >>>>> > I’d like to propose a change to Iceberg’s REST API to make mutation >>>>> > requests safely retryable. >>>>> > >>>>> > *The Problem* >>>>> > If a POST mutation (e.g., updateTable) succeeds in the catalog but >>>>> the >>>>> > client doesn’t receive the response (timeout, connection closed, >>>>> etc.), a >>>>> > second attempt can hit 409 Conflict. The client interprets the 409 >>>>> as a >>>>> > failed commit and deletes the associated metadata files, causing >>>>> > catalog/storage inconsistency. >>>>> > >>>>> > *The Proposed Solution* >>>>> > Introduces an optional Idempotency-Key HTTP header on REST mutation >>>>> > endpoints and has the Iceberg client pass it through. >>>>> > >>>>> > *Semantics *(first processed request wins): >>>>> > >>>>> > - >>>>> > >>>>> > Same key + same canonical payload -> return the original result >>>>> (no >>>>> > re-execution). >>>>> > - >>>>> > >>>>> > Same key + different payload -> 422 (Unprocessable Content). >>>>> > >>>>> > *Capability discovery:* catalogs can advertise support and retention >>>>> so >>>>> > clients know when a retry is safe, e.g. >>>>> > >>>>> > { >>>>> > "idempotency-tokens-respected": true, >>>>> > "idempotency-token-lifetime": "30m" } >>>>> > >>>>> > *Scope in Iceberg:* update the OpenAPI to include the header, and add >>>>> > client pass-through + honoring capability discovery. No server >>>>> > implementation is mandated—catalogs (e.g., Polaris) can implement >>>>> > storage/TTL/replay as they choose. >>>>> > >>>>> > *Standards alignment:* uses the industry-standard header name and >>>>> matches >>>>> > the IETF HTTPAPI Idempotency-Key draft >>>>> > < >>>>> https://datatracker.ietf.org/doc/html/draft-ietf-httpapi-idempotency-key-header >>>>> > >>>>> > semantics. >>>>> > >>>>> > *Compatibility:* fully backward compatible. Servers that don’t >>>>> support it >>>>> > can ignore the header; clients can detect support via capability >>>>> discovery. >>>>> > >>>>> > Here is the proposal >>>>> > < >>>>> https://docs.google.com/document/d/1WyiIk08JRe8AjWh63txIP4i2xcIUHYQWFrF_1CCS3uw/edit?tab=t.0 >>>>> >. >>>>> > Looking forward to your thoughts. >>>>> > >>>>> > Thanks, >>>>> > >>>>> > Huaxin >>>>> > >>>>> >>>>
