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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to