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]