shangxinli commented on code in PR #20310:
URL: https://github.com/apache/hudi/pull/20310#discussion_r4297531243
##########
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:
Retracting this - the direction is safe.
`getCommitsTimeline().firstInstant()` is always at or before what
`getFirstNonSavepointCommit()` returned, so on a table with a pre-existing hole
this pins MDT archival earlier and retains more, never archives past something
still needed. The residual is bounded MDT growth when an old savepoint from
that era survives, not worth blocking on.
--
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]
