[HUDI-6678] Fix the acquisition of clean&rollback instants to archive

[Zouxxyy]

Change Logs

The current getCleanInstantsToArchive filters clean and rollback instants according to maxInstantsToKeep and minInstantsToKeep respectively. There are two disadvantages:

1. If user has only a few rollback instants (not satisfied with the archive), then they will exist forever, even if the commit instant has become very large, which can be very confusing for users.

eg:

// user: what is 202101.rollback?
202101.rollback, 202301.commit, 202302.commit ...

2. Archiving clean and rollback separately will cause holes in the active timeline.

eg:

archive:
2.clean, 3.clean, 4.clean

active:
1.commit, 5.commit ...

A continuous active timeline is our goal.

Therefore, modifying the logic of getCleanAndRollbackInstantsToArchive as follows:

// Since the commit instants to archive is continuous, we can use the newest commit instant to archive as the
// right boundary to collect the clean or rollback instants to archive.
//
//                                                  newestCommitInstantToArchive
//                                                               v
//  | commit1 clean1 commit2 commit3 clean2 commit4 rollback1 commit5 | commit6 clean3 commit7 ...
//  | <------------------  instants to archive  --------------------> |
//
//  CommitInstantsToArchive: commit1, commit2, commit3, commit4, commit5
//  CleanAndRollbackInstantsToArchive: clean1, clean2, rollback1

Impact

• modifying the logic of getCleanAndRollbackInstantsToArchive, fix two disadvantages above
• Optimize the processing of getCommitInstantsToArchive, divide it into two parts: 1) handle oldestInstantToRetain, 2) handle savepoint.
• Fix all test cases.

Risk level (write none, low medium or high below)

medium

Documentation Update

None

Contributor's checklist

• Read through contributor's guide
• Change Logs and Impact were stated clearly
• Adequate tests were added if applicable
• CI passed

[Zouxxyy]

@suryaprasanna @yihua @prashantwason can you help with a revew~

[danny0405]

Looks good from my side, cc @yihua for the final review.

[yihua]

Overall looks ok to me. Will take a second pass to inspect details.

[yihua]

+1 on @danny0405 's comment. Before this PR, the logic is to keep at least one clean and rollback instant on the active timeline. Archiving all rollback instants in the active timeline are ok, but there's caveat on archiving all clean instants (e.g., all completed clean instants are before the earliest commit to retain; one of the cases could be the user turns off cleaning for some time and turns it on again). The incremental cleaning may not get the latest clean instant.

However, it's not the right way either to keep at least one clean instant in the active timeline in the new logic, because that can block the archival of commits for the sake of a contiguous block of instants.

@Zouxxyy could you double check if there is any issue wrt to different cleaning modes under the new archival logic? e.g., incremental cleaning should fall back to full cleaning if there's no clean instant on the active timeline.

[Zouxxyy]

@yihua see #9416 (comment), It explains why incremental cleaning always works even with this PR.

[yihua]

@Zouxxyy could you also rebase the PR on the master?

[Zouxxyy]

@Zouxxyy could you also rebase the PR on the master?

Thanks for the review, will rebase at night, the conflict is a bit big

[danny0405]

Tests have passed: https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build/results?buildId=19917&view=results

[yihua]

[hudi-bot]

CI report:

• 4af0ff9 UNKNOWN
• 1d98932 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

• @hudi-bot run azure re-run the last Azure build

[yihua]

Taking a final pass now.

[yihua]

LGTM. Finally, the archival logic is intuitive and easy to follow. Thanks for improving it :)

[yihua]

nit: I think the logic can be further simplified by treating all instants to archive the same, i.e., getting all instants before the earliest commit to retain, without differentiating the action types. We can follow up in a separate PR.

[yihua]

@Zouxxyy the changes have not been cherry-picked to branch-0.x. It would be great to have this feature in the upcoming release 0.15.0. Do you have time to do that?

[Zouxxyy]

@Zouxxyy the changes have not been cherry-picked to branch-0.x. It would be great to have this feature in the upcoming release 0.15.0. Do you have time to do that?

@yihua OK, but I don’t seem to find the 0.15.0 branch

[yihua]

@Zouxxyy the changes have not been cherry-picked to branch-0.x. It would be great to have this feature in the upcoming release 0.15.0. Do you have time to do that?

@yihua OK, but I don’t seem to find the 0.15.0 branch

@Zouxxyy there is no 0.15.0 branch yet. 0.15.0 release branch will be forked out of branch-0.x, so you can raise a PR against branch-0.x.