Repository navigation
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.
|
Pushed 9596abc addressing review feedback from Andrei Tserakhau (built the equivalent in iceberg-go):
Design notes for reviewers: the base |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
1 participant
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.