laserninja commented on PR #10675: URL: https://github.com/apache/gravitino/pull/10675#issuecomment-4188299996
Thanks @FANNG1 for the review! You're absolutely right. The previous implementation called `updateTable()` sequentially per table, so if table 2 failed, table 1 was already committed — not atomic at all. I've replaced it with a **two-phase validate-then-commit** approach: **Phase 1 — validate ALL before committing any:** - Load each table's `TableOperations` - Validate ALL `UpdateRequirement`s against current metadata - Build the updated `TableMetadata` for each table - If any requirement fails → reject the whole transaction, **zero tables modified** **Phase 2 — commit all (only reached if phase 1 fully passes):** - Call `TableOperations.commit(base, updated)` for each table - Update the metadata cache per table This is the standard pattern for Iceberg REST catalog server implementations (best-effort atomicity). True cross-table rollback after a phase-2 commit failure requires catalog-level transaction support (e.g., atomic storage rename), which varies by backend. The two-phase approach ensures **no table is modified when any pre-condition fails**, which is the most important guarantee clients rely on. -- 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]
