feat(catalog/rest): add client for atomic multi-table transaction commits#2786
Draft
steveis wants to merge 3 commits into
Draft
feat(catalog/rest): add client for atomic multi-table transaction commits#2786steveis wants to merge 3 commits into
steveis wants to merge 3 commits into
Conversation
…mits
Implements the client side of the REST spec's
POST /v1/{prefix}/transactions/commit:
- Transaction::prepare_commit() runs a transaction's actions and returns
the resulting TableCommit without sending it to a catalog, as the
per-table building block of a multi-table commit.
- RestCatalog::commit_transaction(Vec<TableCommit>) submits the group in
one CommitTransactionRequest; the catalog applies all or nothing.
CommitTableRequest already documented that identifier must be present
for CommitTransactionRequest; this supplies the missing caller. No
client-side retry: on conflict every prepared table may be stale, so
re-preparing the group is the caller's decision (409 errors are marked
retryable).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…atomicity; explicit 503 Review feedback: successes return no refreshed metadata (200/204 empty body), so callers must reload affected tables before further transactions; 409 = did-not-apply, marked retryable; 500/502/503/504 = commit-state-unknown, deliberately NOT retryable since resubmitting can double-apply. 503 now has an explicit state-unknown arm instead of falling through to the generic error path.
Author
|
Pushed 9596abc addressing review feedback from Andrei Tserakhau (built the equivalent in iceberg-go):
Design notes for reviewers: the base |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
What changes are included in this PR?
Client support for the REST spec's
POST /v1/{prefix}/transactions/commit(atomic multi-table commits):Transaction::prepare_commit()— runs the transaction's actions against its base table and returns the resultingTableCommitwithout sending it, as the per-table building block. UnlikeTransaction::commit, it does not refresh or retry; conflict handling across a transaction group belongs to the caller (documented).CommitTransactionRequestwire type (table_changes: Vec<CommitTableRequest>) —CommitTableRequestalready documented thatidentifiermust be present for this request.RestCatalog::commit_transaction(Vec<TableCommit>)— submits the group; HTTP 409 maps to a retryableCatalogCommitConflictserror, other statuses mirrorupdate_table's handling. An empty commit list is a no-op that does not contact the server.Kept on
RestCatalograther than theCatalogtrait since multi-table atomicity is a catalog capability, not a universal guarantee — happy to discuss trait-level capability exposure as a follow-up.Use case: a Postgres→Iceberg CDC producer committing per-cadence row deltas to many tables plus a changelog table, atomically. Validated end-to-end against Lakekeeper (which implements the endpoint): two tables, one commit, changes visible on both or neither.
Are these changes tested?
Unit tests:
prepare_commitproduces the expected updates/requirements without a catalog and errors on an empty transaction; mockito-based REST tests for the happy path (body carriestable-changesfor both tables), 409 → retryable conflict, and the empty no-op.cargo test -p iceberg --lib(1385) andcargo test -p iceberg-catalog-rest --lib(47) pass.AI assistance disclosure
Per the Guidelines for AI-assisted Contributions: this PR was drafted with AI assistance (Anthropic's Claude, via Claude Code), then reviewed, tested, and validated by the author.
cargo fmt,clippy, the full unit suites, and public-api snapshots were run locally; the endpoint was validated end-to-end against Lakekeeper.Areas reviewers may want to focus on (our known uncertainties):
prepare_commitdeliberately does not refresh the base table or retry; we'd like feedback on whether that contract (conflict handling belongs to the caller) matches where the transaction API is heading.update_table; 404 is mapped toTableNotFound, though for this endpoint a 404 could also mean the catalog does not implement transactions — happy to adjust the message or kind.