[#12650] improvement(core): add OCC for fileset writes - #12656
Merged
Conversation
Code Coverage Report
Files |
… writes - FilesetCatalogOperations.dropFileset now removes the metadata before the storage locations. The metadata delete is the step that can be rejected on a concurrent alter, and deleting the files first left the data gone while the surviving fileset row still advertised it. - An alter starts above every snapshot the fileset still owns. A fileset written before the version reset was fixed can carry version rows above the version its metadata row records, and rebuilding one of those collided with uk_fid_ver_sto_del on every alter. - The overwrite that keeps the persisted fileset ID now rewrites the stored gravitino.identifier property to that ID, so the version snapshot no longer disagrees with fileset_meta.
jerryshao
reviewed
Aug 27, 2026
jerryshao
left a comment
jerryshao
left a comment
Contributor
There was a problem hiding this comment.
Automated review pass on the OCC changes. Two correctness concerns around orphaned storage on drop, plus a few smaller efficiency/style notes inline.
… OCC - dropFileset no longer fails the drop when a storage location cannot be removed. The metadata is already gone at that point, so rethrowing left the caller with a failed drop it could never retry and files nothing refers to any more. - The overwrite path applies the same version floor as the alter path. It derived the next version from the metadata row alone, which for a legacy fileset rewrote a stored snapshot instead of adding one. - RelationalEntityStore.put invalidates instead of caching on overwrite, because the database decides the identity and version of an overwritten row and the copy handed in can disagree with it. - Noted why the boolean returned by store.delete is still meaningful in dropFileset.
Open
6 tasks
- deleteAndGet only treats a NoSuchEntityException from the delete itself as "nothing to delete". The catch also covered the change log, the cast and the cleanup callback, so a failure there was reported to the caller as a fileset that never existed while the delete was rolled back. - The default deleteAndGet refuses a post-delete action instead of running it after the commit, which is the opposite of what the contract promises and what dropFileset relies on. - A storage location that disappears between the existence check and the delete no longer fails the drop; the filesystem returning false for a path that is already gone is the outcome the drop wanted. - filesetWriteFailure reports a conflict, not a missing fileset, when the name the caller used is now held by a different fileset. - Documented the trade-offs the review called out: the storage delete runs inside the metadata transaction, every alter writes a version snapshot, and the upsert depends on current_version being assigned last.
jerryshao
approved these changes
Aug 28, 2026
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.
What changes were proposed in this pull request?
current_versionas the OCC token for fileset alter and direct delete.This is a subtask of #12166 and follows the table OCC implementation in #12551.
Why are the changes needed?
Concurrent fileset writes could overwrite the winning version metadata, while a stale delete could remove data belonging to a newer fileset version. Overwrite could also reset the OCC version.
Fix: #12650
Does this PR introduce any user-facing change?
Stale fileset writes now fail with the existing optimistic-lock or not-found response instead of overwriting newer metadata. No API or property is added.
How was this patch tested?
:core:check -PskipITs -PskipDockerTests=true.git diff --check.