-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-349]: Added new cleaning policy based on number of hours #3646
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
Changes from 9 commits
97113ff
92d085d
9c1364c
f93ee61
0cd1fe6
68b0a68
f58beda
3c79136
676d999
29e8200
9527702
efa6056
7eea9ec
8e7e497
cc0a44a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| import org.apache.hudi.common.model.HoodieRecordPayload; | ||
| import org.apache.hudi.common.model.HoodieReplaceCommitMetadata; | ||
| import org.apache.hudi.common.model.HoodieTableType; | ||
| import org.apache.hudi.common.table.timeline.HoodieActiveTimeline; | ||
| import org.apache.hudi.common.table.timeline.HoodieInstant; | ||
| import org.apache.hudi.common.table.timeline.HoodieTimeline; | ||
| import org.apache.hudi.common.table.timeline.TimelineMetadataUtils; | ||
|
|
@@ -50,8 +51,12 @@ | |
|
|
||
| import java.io.IOException; | ||
| import java.io.Serializable; | ||
| import java.time.Instant; | ||
| import java.time.ZoneId; | ||
| import java.time.ZonedDateTime; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.Date; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -123,6 +128,7 @@ public Stream<String> getSavepointedDataFiles(String savepointTime) { | |
| public List<String> getPartitionPathsToClean(Option<HoodieInstant> earliestRetainedInstant) throws IOException { | ||
| switch (config.getCleanerPolicy()) { | ||
| case KEEP_LATEST_COMMITS: | ||
| case KEEP_LATEST_BY_HOURS: | ||
| return getPartitionPathsForCleanByCommits(earliestRetainedInstant); | ||
| case KEEP_LATEST_FILE_VERSIONS: | ||
| return getPartitionPathsForFullCleaning(); | ||
|
|
@@ -247,6 +253,10 @@ private List<CleanFileInfo> getFilesToCleanKeepingLatestVersions(String partitio | |
| return deletePaths; | ||
| } | ||
|
|
||
| private List<CleanFileInfo> getFilesToCleanKeepingLatestCommits(String partitionPath) { | ||
| return getFilesToCleanKeepingLatestCommits(partitionPath, config.getCleanerCommitsRetained(), HoodieCleaningPolicy.KEEP_LATEST_COMMITS); | ||
| } | ||
|
|
||
| /** | ||
| * Selects the versions for file for cleaning, such that it | ||
| * <p> | ||
|
|
@@ -261,8 +271,7 @@ private List<CleanFileInfo> getFilesToCleanKeepingLatestVersions(String partitio | |
| * <p> | ||
| * This policy is the default. | ||
| */ | ||
| private List<CleanFileInfo> getFilesToCleanKeepingLatestCommits(String partitionPath) { | ||
| int commitsRetained = config.getCleanerCommitsRetained(); | ||
| private List<CleanFileInfo> getFilesToCleanKeepingLatestCommits(String partitionPath, int commitsRetained, HoodieCleaningPolicy policy) { | ||
| LOG.info("Cleaning " + partitionPath + ", retaining latest " + commitsRetained + " commits. "); | ||
| List<CleanFileInfo> deletePaths = new ArrayList<>(); | ||
|
|
||
|
|
@@ -299,14 +308,24 @@ private List<CleanFileInfo> getFilesToCleanKeepingLatestCommits(String partition | |
| // do not clean up a savepoint data file | ||
| continue; | ||
| } | ||
| // Dont delete the latest commit and also the last commit before the earliest commit we | ||
| // are retaining | ||
| // The window of commit retain == max query run time. So a query could be running which | ||
| // still | ||
| // uses this file. | ||
| if (fileCommitTime.equals(lastVersion) || (fileCommitTime.equals(lastVersionBeforeEarliestCommitToRetain))) { | ||
| // move on to the next file | ||
| continue; | ||
|
|
||
| if (policy == HoodieCleaningPolicy.KEEP_LATEST_COMMITS) { | ||
| // Dont delete the latest commit and also the last commit before the earliest commit we | ||
| // are retaining | ||
| // The window of commit retain == max query run time. So a query could be running which | ||
| // still | ||
| // uses this file. | ||
| if (fileCommitTime.equals(lastVersion) || (fileCommitTime.equals(lastVersionBeforeEarliestCommitToRetain))) { | ||
| // move on to the next file | ||
| continue; | ||
| } | ||
| } else if (policy == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) { | ||
| // This block corresponds to KEEP_LATEST_BY_HOURS policy | ||
| // Do not delete the latest commit. | ||
| if (fileCommitTime.equals(lastVersion)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know why do have (fileCommitTime.equals(lastVersionBeforeEarliestCommitToRetain) in L318. I would prefer to keep the same logic unless we have strong reasons to remove. As I mentioned, cleaning based on num hours is just a diff way to conveying retain N commits. So, lets try to keep the logic similar across both. Easier to maintain.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cleaner policies are designed to enable the longest running query to succeed. Let us take a small example where ingestion is happening every 30 minutes and list of commits -> 0. 1. 2 Hence to allow this query to finish, we take into account With the new policy KEEP_LATEST_BY_HOURS, when 3rd commit is completed, 1st commit will be retained automatically by the logic. Hence the code in its current form works fine. |
||
| // move on to the next file | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Always keep the last commit | ||
|
|
@@ -330,6 +349,19 @@ private List<CleanFileInfo> getFilesToCleanKeepingLatestCommits(String partition | |
| } | ||
| return deletePaths; | ||
| } | ||
|
|
||
| /** | ||
| * This method finds the files to be cleaned based on the number of hours. If {@code config.getCleanerHoursRetained()} is set to 5, | ||
| * all the files with commit time earlier than 5 hours will be removed. Also the latest file for any file group is retained. | ||
| * This policy gives much more flexibility to users for retaining data for running incremental queries as compared to | ||
| * KEEP_LATEST_COMMITS cleaning policy. The default number of hours is 5. | ||
| * @param partitionPath partition path to check | ||
| * @return list of files to clean | ||
| */ | ||
| private List<CleanFileInfo> getFilesToCleanKeepingLatestHours(String partitionPath) { | ||
|
nsivabalan marked this conversation as resolved.
|
||
| int commitsToRetain = 0; | ||
|
pratyakshsharma marked this conversation as resolved.
Outdated
|
||
| return getFilesToCleanKeepingLatestCommits(partitionPath, commitsToRetain, HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS); | ||
| } | ||
|
|
||
| private List<CleanFileInfo> getReplacedFilesEligibleToClean(List<String> savepointedFiles, String partitionPath, Option<HoodieInstant> earliestCommitToRetain) { | ||
| final Stream<HoodieFileGroup> replacedGroups; | ||
|
|
@@ -388,6 +420,8 @@ public List<CleanFileInfo> getDeletePaths(String partitionPath) { | |
| deletePaths = getFilesToCleanKeepingLatestCommits(partitionPath); | ||
| } else if (policy == HoodieCleaningPolicy.KEEP_LATEST_FILE_VERSIONS) { | ||
| deletePaths = getFilesToCleanKeepingLatestVersions(partitionPath); | ||
| } else if (policy == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) { | ||
| deletePaths = getFilesToCleanKeepingLatestHours(partitionPath); | ||
| } else { | ||
| throw new IllegalArgumentException("Unknown cleaning policy : " + policy.name()); | ||
| } | ||
|
|
@@ -402,9 +436,16 @@ public List<CleanFileInfo> getDeletePaths(String partitionPath) { | |
| public Option<HoodieInstant> getEarliestCommitToRetain() { | ||
| Option<HoodieInstant> earliestCommitToRetain = Option.empty(); | ||
| int commitsRetained = config.getCleanerCommitsRetained(); | ||
| int hoursRetained = config.getCleanerHoursRetained(); | ||
| if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_COMMITS | ||
| && commitTimeline.countInstants() > commitsRetained) { | ||
| earliestCommitToRetain = commitTimeline.nthInstant(commitTimeline.countInstants() - commitsRetained); | ||
| earliestCommitToRetain = commitTimeline.nthInstant(commitTimeline.countInstants() - commitsRetained); //15 instants total, 10 commits to retain, this gives 6th instant in the list | ||
| } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) { | ||
| Instant instant = Instant.now(); | ||
| ZonedDateTime commitDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have any precedence in hudi code base for doing time based calculations. Can you explore and let me know. Wanna maintain some uniformity if we have any.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me check.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should sync this to how the instant times are generated. Another way to think about this could be: Keeping the instant timestamp code together will prevent any surprises from a wrong time calculation cleaning data unintentionally.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nsivabalan I checked. There is no uniformity as of now. We use joda dateTime library in TimestampBasedKeyGenerator, java.time.ZonedDateTime in SlashEncodedHourPartitionValueExtractor, and java.util.Date in HoodieActiveTimeline. @prashantwason nthInstant is what we are using throughout the codebase. I guess there is no need to introduce a new method
pratyakshsharma marked this conversation as resolved.
Outdated
|
||
| String retainInstantsAfter = HoodieActiveTimeline.formatDate(Date.from(commitDateTime.minusHours(hoursRetained).toInstant())); | ||
|
pratyakshsharma marked this conversation as resolved.
Outdated
|
||
| earliestCommitToRetain = Option.fromJavaOptional(commitTimeline.getInstants().filter(i -> HoodieTimeline.compareTimestamps(i.getTimestamp(), | ||
| HoodieTimeline.GREATER_THAN_OR_EQUALS, retainInstantsAfter)).findFirst()); | ||
| } | ||
| return earliestCommitToRetain; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.