docs(cleaning): document that clean metadata records instant (start) times - #19552
Conversation
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the docs update! The changes clearly document that Hudi's cleaner records instant (start) times across its plan and metadata fields, and the new tables in cleaning.md and the tech spec are consistent with each other and with the cited code paths. One small clarity note on the incremental-cleaning phrasing; please defer to a Hudi committer or PMC member (e.g. @yihua) for final confirmation.
cc @yihua
| | `startCleanTime` | `HoodieCleanMetadata` | Instant time of the clean action itself. | | ||
|
|
||
| Incremental clean planning follows the same convention: it selects the commits whose **requested** instant time falls | ||
| between the previous clean's `earliestCommitToRetain` and the current one, and scans only the partitions those commits |
There was a problem hiding this comment.
🤖 This sentence reads clearly overall, but "between the previous clean's earliestCommitToRetain and the current one" leaves "the current one" a little ambiguous — it could help to spell out that it means the current clean's earliestCommitToRetain, so readers don't have to infer the endpoint.
There was a problem hiding this comment.
Fair point — fixed in b55e39f, and I made the bounds exact while I was there.
CleanPlanner.java:241-245 filters completed commits with:
requestedTime >= cleanMetadata.getEarliestCommitToRetain() // previous clean's
requestedTime < newInstantToRetain.requestedTime() // this clean'sso the range is half-open — inclusive at the lower end, exclusive at the upper. And newInstantToRetain is the instant returned by CleanPlanner#getEarliestCommitToRetain (CleanPlanActionExecutor.java:129), which is the same one written into the plan as earliestInstantToRetain (:177) and then copied to HoodieCleanMetadata.earliestCommitToRetain. So naming the endpoint "this clean's earliestCommitToRetain" is accurate, not just clearer.
New wording:
Incremental clean planning follows the same convention: it selects the commits whose requested instant time is at or after the previous clean's
earliestCommitToRetainand before this clean's, then scans only the partitions those commits touched.
Applied to both cleaning.md copies and to the tech spec, which carried the same sentence. Build is clean with no new warnings.
Review feedback on apache#19552: "between the previous clean's earliestCommitToRetain and the current one" left the upper endpoint to be inferred. Name it, and while here state the bounds exactly. CleanPlanner:241-245 filters completed commits with requestedTime >= cleanMetadata.getEarliestCommitToRetain() requestedTime < newInstantToRetain.requestedTime() so the range is half-open: inclusive of the previous clean's earliestCommitToRetain, exclusive of this clean's. newInstantToRetain is the instant returned by CleanPlanner#getEarliestCommitToRetain (CleanPlanActionExecutor:129), the same one written to the plan as earliestInstantToRetain (:177), so "this clean's earliestCommitToRetain" names it accurately. Applied to both cleaning.md copies and the tech spec, which carried the same sentence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the docs update! This PR adds a clear field-by-field table documenting that clean plan/metadata timestamps are instant (start) times, plus the incremental-clean planning range, across cleaning.md, its 1.2.0 versioned copy, and the tech spec. I verified each field mapping and the incremental range bounds against the current cleaner source and they are accurate. Please route to a Hudi committer or PMC member for final confirmation.
cc @yihua
…times Hudi 1.x stamps every action with both a requested instant time and a completion time and orders the timeline by completion time, but nothing in the docs said which of the two the cleaner records. A user reading clean metadata to debug cannot tell whether earliestCommitToRetain is a start time or a completion time. It is a start time, on every field, verified at release-1.2.0 and master: - HoodieCleanerPlan.earliestInstantToRetain is built from hoodieInstant.requestedTime() (CleanPlanActionExecutor:111 and :177), and HoodieCleanMetadata.earliestCommitToRetain is copied from the plan (CleanActionExecutor:172-179 and :265). - lastCompletedCommitTimestamp is getCommitTimeline().lastInstant().requestedTime() (CleanPlanner:655-657), so despite the name it is a start time; the source carries the same note at CleanPlanActionExecutor:178. - startCleanTime is inflightInstant.requestedTime() on both the normal and the empty-clean path (CleanActionExecutor:228-232 and :256). - Incremental clean planning likewise ranges over instant.requestedTime() (CleanPlanner:241-245). Add a section to the Cleaning page tabulating these four fields, and a matching field table to the Cleaning section of the 1.0 tech spec. Note this documents behavior as it stands. The parent ticket HUDI-8077, which would have moved clean metadata to completion time, is resolved Won't Do and its PR apache#11972 was closed unmerged. Applied to next and to version-1.2.0, the current released docs. The tech spec is served unversioned from the learn plugin, so it has a single copy. Closes apache#17274.
Review feedback on apache#19552: "between the previous clean's earliestCommitToRetain and the current one" left the upper endpoint to be inferred. Name it, and while here state the bounds exactly. CleanPlanner:241-245 filters completed commits with requestedTime >= cleanMetadata.getEarliestCommitToRetain() requestedTime < newInstantToRetain.requestedTime() so the range is half-open: inclusive of the previous clean's earliestCommitToRetain, exclusive of this clean's. newInstantToRetain is the instant returned by CleanPlanner#getEarliestCommitToRetain (CleanPlanActionExecutor:129), the same one written to the plan as earliestInstantToRetain (:177), so "this clean's earliestCommitToRetain" names it accurately. Applied to both cleaning.md copies and the tech spec, which carried the same sentence.
Two values in the new clean-metadata table are easy to misread, and neither is visible in a single-writer reproduction. lastCompletedCommitTimestamp mixes the two orderings. CleanPlanner:655-656 is getCommitTimeline().lastInstant().map(HoodieInstant::requestedTime), and getCommitTimeline() (CleanPlanner:121-126) is getCompletedCommitsTimeline(), which 1.x orders by completion time. So the instant is picked by completion order and then recorded as a start time. Given commits A (requested t1, completed t4) and B (requested t2, completed t3) with t1 < t2 < t3 < t4, the field holds t1 even though the completed commit B started at t2. Saying "the last completed write" invited reading it as the largest requested time. earliestCommitToRetain can be an empty string. CleanerUtils:126-156 has no branch for KEEP_LATEST_FILE_VERSIONS, so getEarliestCommitToRetain() is empty under that policy, and CleanPlanActionExecutor:177 stores null in the plan. That policy still cleans - getPartitionPathsToClean falls through to getPartitionPathsForFullCleaning (CleanPlanner:161-165) - so the run produces clean stats, CleanActionExecutor:184 substitutes "" for the missing instant, and CleanerUtils.convertCleanMetadata:93 copies it straight into HoodieCleanMetadata. A reader debugging retention on that policy sees an empty field and no explanation. Applied to both cleaning.md copies as a short list under the table, and folded into the corresponding tech spec rows.
b55e39f to
31d35b0
Compare
31d35b0 to
c46c28f
Compare
Describe the issue this Pull Request addresses
Closes #17274. (JIRA: HUDI-8258, a subtask of HUDI-8894.)
Hudi 1.x stamps every action with both a requested instant time and a completion time, and orders the timeline by
completion time. Nothing in the docs said which of the two the cleaner records, so someone reading clean metadata to
debug a retention problem has no way to tell whether
earliestCommitToRetainis a start time or a completion time — andpicking the wrong interpretation sends them looking at the wrong commit.
It is a start time, on every field. The issue asks for exactly this to be stated on the Cleaning and Tech Spec pages.
Summary and Changelog
Added an
### Instant Times in Clean Metadatasubsection to the Cleaning page tabulating the four timestamp fields, anda matching field table to the
Cleaningsection of the 1.0 tech spec:earliestInstantToRetain.timestampHoodieCleanerPlan(theclean.requestedinstant)earliestCommitToRetainHoodieCleanMetadata(the completedcleaninstant)lastCompletedCommitTimestampstartCleanTimeHoodieCleanMetadataPlus a note that incremental clean planning follows the same convention, ranging over the requested instant times of
completed commits.
Files:
website/docs/cleaning.md(next) andwebsite/versioned_docs/version-1.2.0/cleaning.md(current released docs),per the next-plus-current convention used in #19473 and #19551.
website/learn/tech-specs.mdhas no versioned copies —the
learnplugin is configured without versioning — so it has a single edit.Where this comes from in the code
Checked at both
masterand therelease-1.2.0tag; the relevant lines are identical in each.CleanPlanActionExecutor.java:111and:177build the plan'searliestInstantToRetainfromhoodieInstant.requestedTime().CleanPlanner.java:655-657—getLastCompletedCommitTimestamp()returnsgetCommitTimeline().lastInstant().requestedTime(). The source carries the same observation inline atCleanPlanActionExecutor.java:178: "Note: This is the start time of the last completed ingestion before this clean."CleanActionExecutor.java:172-179and:256copy the plan values intoHoodieCleanMetadata.CleanPlanner.java:241-245filters completed commits byinstant.requestedTime()againstearliestCommitToRetain.Reproduction
Source reading alone felt insufficient here, because the page promises what a user will see — and the read path runs
CleanMetadataMigrator.upgradeToLatest, which I had not verified leaves these timestamps alone. So the values below wereread back through
CleanerUtils.getCleanerMetadata, the same call a debugging user would make.Setup: Spark 3.5.7 with
hudi-spark3.5-bundle_2.12:1.2.0, a local-filesystem COW table,hoodie.clean.commits.retained=3,hoodie.clean.incremental.enabled=true, 8 write batches. Requested and completion times differ on every instant, so novalue can match by coincidence.
Timeline (tail):
2026080711213667420260807112136958202608071121370412026080711213736220260807112137452202608071121377462026080711213776020260807112137796Metadata read back from that last clean:
startCleanTime20260807112137760…137796)earliestCommitToRetain20260807112136674…136958)lastCompletedCommitTimestamp20260807112137452…137746)HoodieCleanerPlan.earliestInstantToRetain20260807112136674(action=commit, state=COMPLETED)HoodieCleanerPlan.lastCompletedCommitTimestamp20260807112137452No recorded value appears anywhere in the completion-time column. Every completion time on that timeline
(
134434, 135308, 135737, 136087, 136511, 136630, 136958, 137008, 137362, 137415, 137746, 137796) is absent from themetadata.
Two notes on method. I first ran with
retained=1, whereearliestCommitToRetainandlastCompletedCommitTimestampcollapse to the same value and only one field would have been proven; the
retained=3run above separates them into twodistinct values, both still requested times. And for the incremental-planning sentence, which is not observable in
metadata, I enabled
CleanPlannerINFO logging:20260807112242500is the requested time of20260807112242500_20260807112242806.commit, not its completion.A note on scope
The parent ticket HUDI-8077, which would have moved clean metadata onto completion time, is resolved Won't Do and its
PR #11972 was closed unmerged. This PR therefore documents the behaviour as it actually stands, which matches the
issue's own wording that "the start/instant time is still used in the clean metadata".
Impact
Documentation only. No code, config, or behaviour change.
Risk Level
none
Documentation Update
This PR is the documentation update — the Cleaning page (
/docs/cleaning,/docs/next/cleaning) and the technicalspecification (
/learn/tech-specs).Contributor's checklist