-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-6423] Incremental cleaning should consider inflight compaction instant #9038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| public Option<HoodieInstant> getEarliestCommitToRetain() { | ||
| return CleanerUtils.getEarliestCommitToRetain( | ||
| hoodieTable.getMetaClient().getActiveTimeline().getCommitsTimeline(), | ||
| hoodieTable.getMetaClient().getActiveTimeline().getCommitsAndMergesTimeline(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, there was no consideration of inflight compaction instant. When partitioning by time, if the inflight compaction instant is in the previous partition and is skipped for incremental cleaning, it will cause the log file not be cleaned up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch ~
| */ | ||
| public HoodieTimeline getCommitsAndMergesTimeline() { | ||
| return getTimelineOfActions(CollectionUtils.createSet(COMMIT_ACTION, DELTA_COMMIT_ACTION, REPLACE_COMMIT_ACTION, COMPACTION_ACTION)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCommitsAndMergesTimeline -> getCommitsAndCompactionTimeline
Can we also add a test case for this incremental cleaning scenario, where partition path got switched and the old partition files could not be cleaned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
a65a29c to
34f8823
Compare
|
@hudi-bot run azure |
1 similar comment
|
@hudi-bot run azure |
|
There are many test failures: |
TestCleaner.testKeepLatestCommitsWithPendingCompactions has been fixed, seems that TestGlobalIndexEnableUpdatePartitions is not my problem. |
danny0405
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1,cc @yihua for visibility ~
| HoodieTableMetadataWriter metadataWriter = SparkHoodieBackedTableMetadataWriter.create(hadoopConf, config, context); | ||
|
|
||
| final String partition = "2016/03/15"; | ||
| String timePrefix = "00000000000"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean instant is after instant-000 before adjustment, this is unreasonable, it should be at the end
before adjustment:
localTimeline=[[000__commit__COMPLETED__20230704200732288], [000000001__clean__COMPLETED__20230704200742843], [001__commit__COMPLETED__20230704200733129], [003__commit__COMPLETED__20230704200734117], [==>004__compaction__REQUESTED__20230704200734125], [005__commit__COMPLETED__20230704200734948], [0055__commit__COMPLETED__20230704200735880], [==>006__compaction__REQUESTED__20230704200735885], [007__commit__COMPLETED__20230704200736807], [0075__commit__COMPLETED__20230704200737690], [==>008__compaction__REQUESTED__20230704200737694], [009__commit__COMPLETED__20230704200738629], [0095__commit__COMPLETED__20230704200739576],[==>010__compaction__REQUESTED__20230704200739580], [011__commit__COMPLETED__20230704200740426], [013__commit__COMPLETED__20230704200741363]
after adjustment:
localTimeline=[[00000000000000__commit__COMPLETED__20230704200400940], [00000000000001__commit__COMPLETED__20230704200401790], [00000000000003__commit__COMPLETED__20230704200402888], [==>00000000000004__compaction__REQUESTED__20230704200402896], [00000000000005__commit__COMPLETED__20230704200403841], [000000000000055__commit__COMPLETED__20230704200404879], [==>00000000000006__compaction__REQUESTED__20230704200404883], [00000000000007__commit__COMPLETED__20230704200405861], [000000000000075__commit__COMPLETED__20230704200406790], [==>00000000000008__compaction__REQUESTED__20230704200406797], [00000000000009__commit__COMPLETED__20230704200407808], [000000000000095__commit__COMPLETED__20230704200408834], [==>00000000000010__compaction__REQUESTED__20230704200408839], [00000000000011__commit__COMPLETED__20230704200410653], [00000000000013__commit__COMPLETED__20230704200411934], [00000000000014__clean__COMPLETED__20230704200413695]]
|
@hudi-bot run azure |
5b354dd to
68d67e5
Compare
|
Description and context please... Illustration of issue before fixIncremental has a sliding window that bounds it's cleaning partition as shown below. If there is an async table service action as shown in the illustration below, partitions might be skipped if it falls behind the cleaning window. Note: The illustration might not be the entirely correct in the determination of earliest commit to retain, but it does give a general illustration of the sliding window. FixIn #7568, clean will be blocked by any pending action. As such, by factoring COMPACTION actions into the active timeline, the sliding window of incremental range will be bounded correctly to not ahead of any pending writes made via compaction. |
|
Thanks for the description of the scene~~ Added to current pr change-logs. @voonhous |


Change Logs
Illustration of issue before fix
Incremental has a sliding window that bounds it's cleaning partition as shown below. If there is an async table service action as shown in the illustration below, partitions might be skipped if it falls behind the cleaning window.
Note: The illustration might not be the entirely correct in the determination of earliest commit to retain, but it does give a general illustration of the sliding window.
Fix
In #7568, clean will be blocked by any pending action. As such, by factoring COMPACTION actions into the active timeline, the sliding window of incremental range will be bounded correctly to not ahead of any pending writes made via compaction.
Impact
Describe any public API or user-facing feature change or any performance impact.
Risk level (write none, low medium or high below)
none
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist