laserninja opened a new pull request, #12474: URL: https://github.com/apache/gravitino/pull/12474
> Second half of the split. Stacked on #12470 - the first commit here belongs to that PR, so review the second commit (`+1143/-32`) only. I'll rebase onto main and undraft once #12470 merges. ### What changes were proposed in this pull request? Puts the storage seam from #12470 behind the REST endpoints. - **`IcebergIdempotencyManager`** runs reserve, execute, finalize around a mutation. Retries replay the stored response; a key reused for another operation, or one whose first request is still in flight, gets `409 Conflict` (the latter with `Retry-After`); `5xx` releases the key so the client can retry with it. - **Endpoint wiring** for create, update, drop, register, and rename across tables, namespaces, and views (13 endpoints). Each endpoint delegates to a private method holding its original body, so the idempotency seam is visible at the call site and the mutation itself reads unchanged. - **`GET /v1/config`** advertises `idempotency-key-lifetime` as the top-level `CatalogConfig` field the spec defines, not a `defaults` entry as the design document's example showed, and only when enabled, since the spec reads its absence as no support. Iceberg 1.11's `ConfigResponse.Builder.withIdempotencyKeyLifetime()` already supports it. One deliberate deviation from the spec: `401`, `403`, and `419` **release** the key rather than finalizing it. The spec finalizes deterministic terminal `4xx`, but auth outcomes depend on the caller's credentials rather than catalog state, so replaying them would keep failing a retry that has since obtained a valid token. Known limitation, documented in the manager's Javadoc: a replayed response carries the original status and body but not headers, so a replayed create/update does not carry the `ETag` the first response did. ### Why are the changes needed? Epic #12049. Completes phase 1 of the design. Phase 2, the durable multi-node JDBC store, follows separately. Fix: #12049 ### Does this PR introduce _any_ user-facing change? Yes. Clients can send `Idempotency-Key: <UUIDv7>` on mutation requests for safe retries, and `GET /v1/config` includes `idempotency-key-lifetime` when enabled. Documents the four properties added in #12470. Off by default, so default behavior is unchanged. ### How was this patch tested? - `TestIcebergIdempotencyManager` - replay, distinct keys, bodiless `204` replay, terminal `4xx` replay, `5xx`/`401`/thrown-exception release, invalid key rejected without executing, operation-binding mismatch, in-flight `409` with `Retry-After`, query-parameter-sorted bindings. - `TestIcebergIdempotencyKey` - end-to-end through Jersey: a replayed create fires no second `IcebergCreateTableEvent`, a fresh key re-executes into `409 AlreadyExists`, a stored `409` replays, a key reused on `DELETE` is rejected and the table survives, a drop replays `204` where a retry would otherwise `404`, and `GET /v1/config` carries `PT30M`. - `TestIcebergConfig` - the lifetime is absent when disabled. `iceberg-rest-server` suite passes on JDK 17: 418 tests, 0 failures. ```bash ./gradlew :iceberg:iceberg-rest-server:test -PskipITs ``` -- 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]
