You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "deepakpanda93 (via GitHub)" <gi...@apache.org> on 2026/08/07 11:55:06 UTC

[PR] docs(cleaning): document that clean metadata records instant (start) times [hudi]

deepakpanda93 opened a new pull request, #19552:
URL: https://github.com/apache/hudi/pull/19552

   ### 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 `earliestCommitToRetain` is a start time or a completion time — and
   picking 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 Metadata` subsection to the Cleaning page tabulating the four timestamp fields, and
   a matching field table to the `Cleaning` section of the 1.0 tech spec:
   
   | Field | Written to | Value |
   |---|---|---|
   | `earliestInstantToRetain.timestamp` | `HoodieCleanerPlan` (the `clean.requested` instant) | Instant time of the oldest commit this clean run retains |
   | `earliestCommitToRetain` | `HoodieCleanMetadata` (the completed `clean` instant) | Copied from the plan, so also an instant time |
   | `lastCompletedCommitTimestamp` | both | Instant time of the last completed write before the clean was planned — despite the name, a start time |
   | `startCleanTime` | `HoodieCleanMetadata` | Instant time of the clean action itself |
   
   Plus 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) and `website/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.md` has no versioned copies —
   the `learn` plugin is configured without versioning — so it has a single edit.
   
   ### Where this comes from in the code
   
   Checked at both `master` and the `release-1.2.0` tag; the relevant lines are identical in each.
   
   - `CleanPlanActionExecutor.java:111` and `:177` build the plan's `earliestInstantToRetain` from `hoodieInstant.requestedTime()`.
   - `CleanPlanner.java:655-657` — `getLastCompletedCommitTimestamp()` returns `getCommitTimeline().lastInstant().requestedTime()`. The source carries the same observation inline at `CleanPlanActionExecutor.java:178`: *"Note: This is the start time of the last completed ingestion before this clean."*
   - `CleanActionExecutor.java:172-179` and `:256` copy the plan values into `HoodieCleanMetadata`.
   - `CleanPlanner.java:241-245` filters completed commits by `instant.requestedTime()` against `earliestCommitToRetain`.
   
   ### 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 were
   read 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 no
   value can match by coincidence.
   
   Timeline (tail):
   
   | Action | requested | completion |
   |---|---|---|
   | commit | `20260807112136674` | `20260807112136958` |
   | commit | `20260807112137041` | `20260807112137362` |
   | commit | `20260807112137452` | `20260807112137746` |
   | clean  | `20260807112137760` | `20260807112137796` |
   
   Metadata read back from that last clean:
   
   | Field | Value | Is the... |
   |---|---|---|
   | `startCleanTime` | `20260807112137760` | clean's **requested** time (not `…137796`) |
   | `earliestCommitToRetain` | `20260807112136674` | that commit's **requested** time (not `…136958`) |
   | `lastCompletedCommitTimestamp` | `20260807112137452` | last commit's **requested** time (not `…137746`) |
   | `HoodieCleanerPlan.earliestInstantToRetain` | `20260807112136674` (action=commit, state=COMPLETED) | identical to the metadata, confirming it is copied from the plan |
   | `HoodieCleanerPlan.lastCompletedCommitTimestamp` | `20260807112137452` | same |
   
   **No 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 the
   metadata.
   
   Two notes on method. I first ran with `retained=1`, where `earliestCommitToRetain` and `lastCompletedCommitTimestamp`
   collapse to the same value and only one field would have been proven; the `retained=3` run above separates them into two
   distinct values, both still requested times. And for the incremental-planning sentence, which is not observable in
   metadata, I enabled `CleanPlanner` INFO logging:
   
   ```
   Incremental Cleaning mode is enabled. Looking up partition-paths that have changed since last clean at 20260807112242500.
   New Instant to retain Option{val=[20260807112242946__20260807112243250__commit__COMPLETED]}.
   ```
   
   `20260807112242500` is the **requested** time of `20260807112242500_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 technical
   specification (`/learn/tech-specs`).
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] docs(cleaning): document that clean metadata records instant (start) times [hudi]

Posted by "voonhous (via GitHub)" <gi...@apache.org>.
voonhous merged PR #19552:
URL: https://github.com/apache/hudi/pull/19552


-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] docs(cleaning): document that clean metadata records instant (start) times [hudi]

Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19552:
URL: https://github.com/apache/hudi/pull/19552#discussion_r3735761515


##########
website/docs/cleaning.md:
##########
@@ -86,6 +86,23 @@ takes precedence over the regex.
 | `hoodie.clean.partition.filter.regex` | (none) | Java regex pattern; only partitions whose path matches are cleaned. |
 | `hoodie.clean.partition.filter.selected` | (none) | Comma-separated list of partition paths to clean; takes precedence over the regex when both are set. |
 
+### Instant Times in Clean Metadata
+
+Hudi 1.x stamps every action with both a requested instant time and a completion time, and orders actions on the
+timeline by completion time — see [timeline](timeline.md). The cleaner's own plan and metadata, however, record
+**instant (start) times** throughout. Keep this in mind when reading them for debugging.
+
+| Field | Written to | Value |
+|---|---|---|
+| `earliestInstantToRetain.timestamp` | `HoodieCleanerPlan` (the `clean.requested` instant) | Instant time of the oldest commit this clean run retains. |
+| `earliestCommitToRetain` | `HoodieCleanMetadata` (the completed `clean` instant) | Copied from the plan, so also an instant time. |
+| `lastCompletedCommitTimestamp` | both | Instant time of the last completed write before the clean was planned. Despite the name, this is a start time, not a completion time. |
+| `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

Review Comment:
   Fair point — fixed in b55e39f1c808, and I made the bounds exact while I was there.
   
   `CleanPlanner.java:241-245` filters completed commits with:
   
   ```java
   requestedTime >= cleanMetadata.getEarliestCommitToRetain()   // previous clean's
   requestedTime <  newInstantToRetain.requestedTime()          // this clean's
   ```
   
   so 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 `earliestCommitToRetain` and 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.



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] docs(cleaning): document that clean metadata records instant (start) times [hudi]

Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19552:
URL: https://github.com/apache/hudi/pull/19552#discussion_r3735577342


##########
website/docs/cleaning.md:
##########
@@ -86,6 +86,23 @@ takes precedence over the regex.
 | `hoodie.clean.partition.filter.regex` | (none) | Java regex pattern; only partitions whose path matches are cleaned. |
 | `hoodie.clean.partition.filter.selected` | (none) | Comma-separated list of partition paths to clean; takes precedence over the regex when both are set. |
 
+### Instant Times in Clean Metadata
+
+Hudi 1.x stamps every action with both a requested instant time and a completion time, and orders actions on the
+timeline by completion time — see [timeline](timeline.md). The cleaner's own plan and metadata, however, record
+**instant (start) times** throughout. Keep this in mind when reading them for debugging.
+
+| Field | Written to | Value |
+|---|---|---|
+| `earliestInstantToRetain.timestamp` | `HoodieCleanerPlan` (the `clean.requested` instant) | Instant time of the oldest commit this clean run retains. |
+| `earliestCommitToRetain` | `HoodieCleanMetadata` (the completed `clean` instant) | Copied from the plan, so also an instant time. |
+| `lastCompletedCommitTimestamp` | both | Instant time of the last completed write before the clean was planned. Despite the name, this is a start time, not a completion time. |
+| `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

Review 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.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org