shangxinli commented on code in PR #20310:
URL: https://github.com/apache/hudi/pull/20310#discussion_r4289842753
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineUtils.java:
##########
@@ -362,26 +360,15 @@ public static HoodieCommitMetadata getCommitMetadata(
* for the archival in metadata table.
* <p>
* the qualified earliest instant is chosen as the earlier one between the
earliest
- * commit (COMMIT, DELTA_COMMIT, and REPLACE_COMMIT only, considering
non-savepoint
- * commit only if enabling archive beyond savepoint) and the earliest
inflight
+ * commit (COMMIT, DELTA_COMMIT, and REPLACE_COMMIT only) and the earliest
inflight
* instant (all actions).
*
- * @param dataTableActiveTimeline the active timeline of the data table.
- * @param shouldArchiveBeyondSavepoint whether to archive beyond savepoint.
+ * @param dataTableActiveTimeline the active timeline of the data table.
* @return the instant meeting the requirement.
*/
- public static Option<HoodieInstant> getEarliestInstantForMetadataArchival(
- HoodieActiveTimeline dataTableActiveTimeline, boolean
shouldArchiveBeyondSavepoint) {
+ public static Option<HoodieInstant>
getEarliestInstantForMetadataArchival(HoodieActiveTimeline
dataTableActiveTimeline) {
// This is for commits only, not including CLEAN, ROLLBACK, etc.
- // When archive beyond savepoint is enabled, there are chances that there
could be holes
- // in the timeline due to archival and savepoint interplay. So, the first
non-savepoint
- // commit in the data timeline is considered as beginning of the active
timeline.
- Option<HoodieInstant> earliestCommit = shouldArchiveBeyondSavepoint
- ? dataTableActiveTimeline.getTimelineOfActions(
- CollectionUtils.createSet(
- COMMIT_ACTION, DELTA_COMMIT_ACTION, REPLACE_COMMIT_ACTION,
CLUSTERING_ACTION, SAVEPOINT_ACTION))
- .getFirstNonSavepointCommit()
- : dataTableActiveTimeline.getCommitsTimeline().firstInstant();
+ Option<HoodieInstant> earliestCommit =
dataTableActiveTimeline.getCommitsTimeline().firstInstant();
Review Comment:
**major:** The branch removed here existed for timelines that *already* have
holes - its comment said so - and those tables still exist, which is exactly
what the new `SavepointHelpers.validateNoArchivedCommitsAfterSavepoint` guards
against on the restore path. Dropping `getFirstNonSavepointCommit()` leaves MDT
archival on such a table deriving its earliest instant from the retained
savepointed commit below the hole, and also drops `CLUSTERING_ACTION` from the
set that was considered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
