Repository navigation
[core] Carry table UUID and writer identity in catalog rollback - #10520
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
The new rollback identity plumbing is correct for SnapshotLoaderImpl.copyWithBranch(), but the branch-aware CatalogEnvironment copy path still leaks the source UUID. Please fix this before merge. CatalogEnvironment.copy(Identifier) always preserves uuid, even when the identifier changes branch. This method is used by FileStoreTableFactory.createOtherBranchTable() and FallbackReadFileStoreTable.switchWrappedToBranch() to create a table view for another branch. Since AbstractFileStore now calls catalogTableRollback(null, null) and forwards this UUID, rollback from that view sends the source branch UUID with the target branch identifier. The REST service can then reject a valid rollback because the expected table incarnation belongs to the other branch. Please clear the UUID when copying to a different branch (while retaining it for the same branch), or otherwise make the branch-copy call pass null.
leaves12138
left a comment
There was a problem hiding this comment.
The branch-aware UUID handling is fixed: CatalogEnvironment.copy(Identifier) now preserves the UUID only for the same branch and clears it for cross-branch views. The added coverage for CatalogEnvironment, SnapshotLoaderImpl, rollback, and fallback branch views passes (46 tests). Approving.
Purpose
Catalog rollback needs the table UUID to identify the expected table incarnation and an optional commitUser to identify the writer holding a catalog lock. Replace Catalog and RESTApi rollbackTo with the five-argument API, extend RollbackTableRequest, and forward the context through DelegateCatalog and CatalogEnvironment.
CatalogEnvironment and SnapshotLoaderImpl forward a known UUID, independently of commitUser, so ordinary table.rollbackTo carries the expected table identity. Same-branch snapshot views and catalog environment copies retain the UUID; views copied to another branch omit it because that branch can have a different UUID which has not been loaded. This also applies to fallback/primary branch table views created by FileStoreTableFactory and switched fallback tables. The previous Java rollback overloads and DTO constructor are removed, and repository implementations and callers are migrated. This API is a prerequisite for #10491.
Tests
JDK 8: 105 targeted tests passed, including UUID forwarding without an owner, nullable UUIDs, same-branch and cross-branch environment/snapshot copies, fallback/primary branch table rollback, snapshot/tag wire payloads, runtime branch routing, empty catalog contexts, ordinary REST rollback, compaction conflict recovery, and the existing unlocked commit failure path. Checkstyle, Spotless, enforcer, and root RAT checks passed.
Manual integration against a local H2/filesystem REST service passed: actual FileStoreCommit followed by table.rollbackTo through SnapshotLoaderImpl, snapshot file cleanup, snapshot ID reuse, wrong-UUID rejection, and rejection of rollback through an old table object after the table is dropped and recreated. Loaded branch rollback, cross-branch snapshot views, commit rollback from factory-created fallback branch environments, and switched fallback table.rollbackTo also passed while preserving the main branch head.