deepakpanda93 commented on code in PR #19572:
URL: https://github.com/apache/hudi/pull/19572#discussion_r3756115108


##########
website/versioned_docs/version-1.0.0/compaction.md:
##########
@@ -226,3 +226,38 @@ Offline compaction needs to submit the Flink task on the 
command line. The progr
 | `--seq` | `LIFO`  (Optional)   | The order in which compaction tasks are 
executed. Executing from the latest compaction plan by default. `LIFO`: 
executing from the latest plan. `FIFO`: executing from the oldest plan. |
 | `--service` | `false`  (Optional)  | Whether to start a monitoring service 
that checks and schedules new compaction task in configured interval. |
 | `--min-compaction-interval-seconds` | `600(s)` (optional)  | The checking 
interval for service mode, by default 10 minutes. |
+
+## Log Compaction
+
+Log compaction is a minor compaction for Merge-on-Read tables. Rather than 
merging log files into a new base file, it
+stitches several small log blocks into a larger one within the same file 
group. A file group that receives frequent
+small updates can therefore be kept efficient without paying the cost of 
rewriting its base file. Readers skip the log
+blocks that have already been stitched, so read amplification is reduced as 
well. Log compaction appears on the timeline
+as a `logcompaction` action.
+
+| Config Name | Default | Description |
+|---|---|---|
+| `hoodie.log.compaction.inline` | `false` (Optional) | When set to true, the 
log compaction service is triggered after each write. While being simpler 
operationally, this adds extra latency on the write path.<br /><br />`Config 
Param: INLINE_LOG_COMPACT`<br />`Since Version: 0.13.0` |

Review Comment:
   Confirmed and acted on — the section is dropped from `version-1.0.0` and 
`version-1.0.1` entirely. 7a99aa130ab0.
   
   I re-ran your check rather than take it on trust, and it reproduces exactly:
   
   | tag | `LOG_COMPACT` value | `case "log_compact"` | `case "logcompact"` |
   |---|---|---|---|
   | release-1.0.0 | `LOG_COMPACT("logcompact")` | 0 | 0 |
   | release-1.0.1 | `LOG_COMPACT("logcompact")` | 0 | 0 |
   | release-1.0.2 | `LOG_COMPACT("log_compact")` | 1 | 0 |
   | release-1.1.1 | `LOG_COMPACT("log_compact")` | 1 | 0 |
   | release-1.2.0 | `LOG_COMPACT("log_compact")` | 1 | 0 |
   
   Neither spelling has a case at 1.0.0/1.0.1, so it falls to `default: throw`. 
Documenting the config there was worse than a gap — it pointed users at a 
guaranteed failure.
   
   I took the drop rather than the caution, because a caution saying "this 
config does not work on this release" is not useful to a 1.0.x reader, and 
combined with your read-amplification finding the whole section had nothing 
left that was true for those versions.
   
   One thing your comment prompted that I had missed: reverting `compaction.md` 
alone would have left the `write_operations.md` pointer behind, linking to a 
`#log-compaction` anchor that no longer exists on those pages. Both files are 
now reverted on all four dropped versions, byte-identical to upstream (`git 
diff --quiet upstream/asf-site` is clean for them).



##########
website/versioned_docs/version-1.0.1/compaction.md:
##########
@@ -227,6 +227,41 @@ Offline compaction needs to submit the Flink task on the 
command line. The progr
 | `--service` | `false`  (Optional)  | Whether to start a monitoring service 
that checks and schedules new compaction task in configured interval. |
 | `--min-compaction-interval-seconds` | `600(s)` (optional)  | The checking 
interval for service mode, by default 10 minutes. |
 
+## Log Compaction
+
+Log compaction is a minor compaction for Merge-on-Read tables. Rather than 
merging log files into a new base file, it
+stitches several small log blocks into a larger one within the same file 
group. A file group that receives frequent
+small updates can therefore be kept efficient without paying the cost of 
rewriting its base file. Readers skip the log
+blocks that have already been stitched, so read amplification is reduced as 
well. Log compaction appears on the timeline
+as a `logcompaction` action.
+
+| Config Name | Default | Description |
+|---|---|---|
+| `hoodie.log.compaction.inline` | `false` (Optional) | When set to true, the 
log compaction service is triggered after each write. While being simpler 
operationally, this adds extra latency on the write path.<br /><br />`Config 
Param: INLINE_LOG_COMPACT`<br />`Since Version: 0.13.0` |

Review Comment:
   Same finding, same fix — `version-1.0.1` is reverted along with 
`version-1.0.0`. See the reply on the 1.0.0 thread for the reproduced enum 
table. 7a99aa130ab0.



##########
website/versioned_docs/version-1.1.1/compaction.md:
##########
@@ -252,6 +252,41 @@ Offline compaction needs to submit the Flink task on the 
command line. The progr
 | `--service`                         | `false`  (Optional)  | Whether to 
start a monitoring service that checks and schedules new compaction task in 
configured interval.                                                            
                                                                                
                                          |
 | `--min-compaction-interval-seconds` | `600(s)` (optional)  | The checking 
interval for service mode, by default 10 minutes.                               
                                                                                
                                                                                
                                    |
 
+## Log Compaction
+
+Log compaction is a minor compaction for Merge-on-Read tables. Rather than 
merging log files into a new base file, it
+stitches several small log blocks into a larger one within the same file 
group. A file group that receives frequent
+small updates can therefore be kept efficient without paying the cost of 
rewriting its base file. Readers skip the log
+blocks that have already been stitched, so read amplification is reduced as 
well. Log compaction appears on the timeline

Review Comment:
   Confirmed, and this is the one I would have shipped wrong. The section is 
now restricted to `next` + `version-1.2.0`. 7a99aa130ab0.
   
   Verified the branch presence at each tag rather than reasoning from the 
config default:
   
   | tag | `scanInternalV2` refs | `if (enableOptimizedLogBlocksScan)` | 
`scanInternalV1` refs |
   |---|---|---|---|
   | release-1.0.0 | 2 | 1 | 2 |
   | release-1.0.2 | 2 | 1 | 2 |
   | release-1.1.1 | 2 | 1 | 2 |
   | release-1.2.0 | **0** | **0** | **0** |
   
   At 1.2.0 the branch is gone entirely and the `COMPACTED_BLOCK_TIMES` skip 
sits unconditionally in the main scan path (line 378 there, versus line 506 
inside `scanInternalV2` at 1.1.1). So the sentence is true at 1.2.0 and false 
everywhere earlier, exactly as you said.
   
   Your point that read amplification goes **up** on stock 1.0.x/1.1.1 settings 
is what makes this a correctness problem rather than an omission — the section 
was selling a benefit that inverts in practice. Taking your suggestion to 
restrict the whole section rather than surgically remove one sentence per copy: 
it resolves this together with HUDI-9220 and keeps the six copies from drifting.
   
   The sentence now reads "so there are fewer blocks to merge on read", which 
is the mechanism rather than the metric, and it only ships on the version where 
the skip is unconditional.



##########
website/docs/compaction.md:
##########
@@ -283,6 +283,41 @@ Offline compaction needs to submit the Flink task on the 
command line. The progr
 The retry options (`--retry`, `--retry-last-failed-job`, 
`--job-max-processing-time-ms`) are only effective in single-run mode, not in 
service mode. Service mode has implicit retry semantics via its continuous 
monitoring loop. A warning will be logged if `--retry-last-failed-job` is 
enabled but `--job-max-processing-time-ms` is not set to a positive value.
 :::
 
+## Log Compaction
+
+Log compaction is a minor compaction for Merge-on-Read tables. Rather than 
merging log files into a new base file, it
+stitches several small log blocks into a larger one within the same file 
group. A file group that receives frequent
+small updates can therefore be kept efficient without paying the cost of 
rewriting its base file. Readers skip the log
+blocks that have already been stitched, so read amplification is reduced as 
well. Log compaction appears on the timeline
+as a `logcompaction` action.

Review Comment:
   Confirmed and fixed. `transitionLogCompactionInflightToComplete` creates the 
completed instant with `DELTA_COMMIT_ACTION` — I checked at both 
`release-1.1.1` and `release-1.2.0` and the line is identical in each. Your 
point about someone listing `.hoodie/` and concluding it never ran is the 
failure mode that makes this worth stating explicitly.
   
   Wording is close to your suggestion but split into its own paragraph, since 
the surrounding sentences now cover the storage cost:
   
   > Log compaction is scheduled on the timeline as a `logcompaction` action, 
in the `requested` and `inflight` states. On completion it is committed as a 
`deltacommit`, so there is no completed `logcompaction` instant to look for.
   
   7a99aa130ab0.



-- 
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