fix(common): keep committed logs visible under an uncommitted base instant - #19785
Open
zhaoyudi-creator wants to merge 1 commit into
Open
Conversation
…stant A MOR file group can end up with its latest file slice keyed on a base instant that never committed. For example, under NBCC with the bucket index a delta commit fails (or is rolled back) while later delta commits on the same file group succeed. Log files are attributed to a slice by completion time, so the failed instant's log and the later committed logs land in one slice whose base instant is the failed one. isFileSliceCommitted only checked whether the base instant itself was committed, so the whole slice -- including its committed log files -- was treated as uncommitted and dropped from the reader view, silently losing committed data. Consider a slice committed when its base instant is committed OR any of its log files carries a committed delta commit time. The reader still trims the individual uncommitted log via filterUncommittedFiles / filterUncommittedLogs, so only the committed logs are read. The change is inert for table version < 8: there every log file in a slice shares the slice's base instant as its file-name token (HoodieAppendHandle reuses the previous commit for the log instant), so an uncommitted base implies all-uncommitted log tokens and the new clause cannot fire. Tests: - TestHoodieFileGroup#testCommittedLogsMakeSliceWithUncommittedBaseInstantVisible verifies a slice with an inflight base instant and committed logs is now returned by getAllFileSlices(). - TestHoodieTableFileSystemView#testCommittedLogsRemainVisibleWithUncommittedBaseInstant verifies the file-system view surfaces the slice and trims only the uncommitted log. Closes apache#19774
Contributor
|
sorry, after some analysis, I found that patch1 is more reasonable fix: #19774 (comment) |
Collaborator
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19785 +/- ##
============================================
- Coverage 78.15% 75.46% -2.69%
+ Complexity 33691 32539 -1152
============================================
Files 2540 2540
Lines 141413 141415 +2
Branches 17123 17124 +1
============================================
- Hits 110516 106719 -3797
- Misses 23195 26833 +3638
- Partials 7702 7863 +161
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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
4 participants
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.
Change Logs
A MOR file group can have its latest file slice keyed on a base instant
that never committed. For example, under NBCC with the bucket index a
delta commit fails (or is rolled back) while later delta commits on the
same file group succeed; log files are attributed to a slice by completion
time, so the failed instant's log and the later committed logs land in one
slice whose base instant is the failed one.
isFileSliceCommittedonly checked whether the base instant itself wascommitted, so the whole slice — including its committed log files — was
treated as uncommitted and dropped from the reader view, silently losing
committed data.
Fix: also consider a slice committed when any of its log files carries a
committed delta commit time. Read-side
filterUncommittedFiles/filterUncommittedLogsstill trims the individual uncommitted log, so onlycommitted logs are read.
The change is inert for table version < 8: there every log file in a slice
shares the slice's base instant as its file-name token, so an uncommitted
base implies all-uncommitted log tokens and the new clause cannot fire.
Impact
Restores correct MOR read results when a file slice's base instant belongs
to a failed/rolled-back delta commit. No public API change.
Risk level: low
Narrowly widens slice visibility; covered by two new unit tests and
verified to be a no-op for pre-v8 layouts.
Documentation Update
None.
Contributor's checklist