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


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

Review Comment:
   Took the cheap win: `timeline.md`'s `**LOGCOMPACTION**` bullet now links to 
`compaction.md#log-compaction` in both surviving copies. 7a99aa130ab0. You are 
right that it is the most likely entry point and it dead-ended.
   
   On the unit inconsistency — I have left this section on **blocks** and not 
touched the other four places. The reasoning: the config that gates it is 
`hoodie.log.compaction.blocks.threshold`, and per your other comment it 
triggers on file count *or* block count, so blocks is at least as defensible as 
files and matches the config a reader will be setting. Normalising all five 
would mean editing `timeline.md`, `hudi_stack.md` and two spots in 
`tech-specs.md` across the versions that have them, which is a bigger and more 
opinionated change than this PR should carry. Flagging it as worth its own pass 
rather than doing it here.
   
   I did not consolidate with `tech-specs.md:667-674` either. The spec section 
and this one now overlap in substance, though this one has diverged: it carries 
the storage cost, the `deltacommit` detail and the MDT configs, none of which 
the spec has. That is arguably fine — spec versus user guide — but it is 
duplication either way.



##########
website/docs/write_operations.md:
##########
@@ -128,7 +128,7 @@ The following is an inside look on the Hudi write path and 
the sequence of event
 6. Update [Index](indexes.md): Now that the write is performed, we will go 
back and update the index.
 7. Commit: Finally we commit all of these changes atomically. ([Post-commit 
callback](platform_services_post_commit_callback.md) can be configured.)
 8. [Clean](cleaning.md) (if needed): Following the commit, cleaning is invoked 
if needed.
-9. [Compaction](compaction.md): If you are using MOR tables, compaction will 
either run inline, or be scheduled asynchronously
+9. [Compaction](compaction.md): If you are using MOR tables, compaction will 
either run inline, or be scheduled asynchronously. [Log 
compaction](compaction.md#log-compaction) may also run, stitching small log 
blocks together without rewriting the base file.

Review Comment:
   Applied essentially verbatim. 7a99aa130ab0.
   
   > 9. [Compaction](compaction.md): If you are using MOR tables, compaction 
will either run inline, or be scheduled asynchronously. If 
`hoodie.log.compaction.inline` is enabled, [log 
compaction](compaction.md#log-compaction) may also run, stitching small log 
blocks together without rewriting the base file.
   
   The asymmetry you point out is the whole problem with the original: sitting 
next to a sentence about a default-on service, "may also run" reads as "also 
happens automatically". Naming the config removes that reading.
   
   Confirmed in the rendered page on both `/docs` and `/docs/next`, and 
confirmed the four dropped versions are back to the original step 9 with no 
pointer.



##########
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.
+
+| 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` |
+| `hoodie.log.compaction.blocks.threshold` | `5` (Optional) | Log compaction 
can be scheduled once the number of log blocks crosses this threshold. 
Effective only when log compaction is enabled via 
`hoodie.log.compaction.inline`.<br /><br />`Config Param: 
LOG_COMPACTION_BLOCKS_THRESHOLD`<br />`Since Version: 0.13.0` |
+
+:::note
+`hoodie.log.compaction.inline` is the only built-in way to schedule log 
compaction on a data table. There is no
+asynchronous log compaction service, SQL procedure, Hudi CLI command, or 
standalone utility for it, unlike compaction.
+Programmatic scheduling is available through the write client's 
`scheduleLogCompaction` and `logCompact` methods.
+:::
+
+The metadata table runs its own log compaction, controlled by a separate pair 
of configs:
+
+| Config Name | Default | Description |
+|---|---|---|
+| `hoodie.metadata.log.compaction.enable` | `false` (Optional) | Enables log 
compaction for the metadata table.<br /><br />`Config Param: 
ENABLE_LOG_COMPACTION_ON_METADATA_TABLE`<br />`Since Version: 0.14.0` |
+| `hoodie.metadata.log.compaction.blocks.threshold` | `5` (Optional) | Number 
of log blocks above which log compaction is scheduled on the metadata table.<br 
/><br />`Config Param: LOG_COMPACT_BLOCKS_THRESHOLD`<br />`Since Version: 
0.14.0` |
+
+:::caution
+`hoodie.log.compaction.enable` also appears in the configuration reference, 
but it is not a switch to set on your table.
+Hudi applies it internally to the metadata table's own write config, deriving 
its value from
+`hoodie.metadata.log.compaction.enable`. Setting it on a data table has no 
effect: use
+`hoodie.log.compaction.inline` for the data table, and 
`hoodie.metadata.log.compaction.enable` for the metadata table.

Review Comment:
   Description refreshed. It was indeed written against the pre-`d4016ac` 
revision and had gone stale twice over — "the three configs" and the claim 
about keeping the `hoodie.log.compaction.enable` row narrow, when that row had 
been removed.
   
   The rewritten description now covers the five documented configs plus the 
`:::caution`, states the `next` + 1.2.0 restriction with both blockers and the 
source evidence, explains each correctness fix, and re-states the build and 
warning-parity verification as run against the current revision rather than an 
earlier one.
   
   Thanks for verifying the `createMetadataWriteConfig` derivation across all 
six refs — that is the claim in the `:::caution` I was least able to check 
exhaustively, and having the line numbers at 1.0.0 through master is more 
coverage than I had.
   
   One methodological point your other comment surfaced, which I have now put 
in the description explicitly: the byte-identical-warning-set check detects 
**new** warnings, not pre-existing ones. So "warning parity" is a weaker 
guarantee than it sounds, and I have stopped presenting it as if it caught 
everything.



##########
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.
+
+| 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` |
+| `hoodie.log.compaction.blocks.threshold` | `5` (Optional) | Log compaction 
can be scheduled once the number of log blocks crosses this threshold. 
Effective only when log compaction is enabled via 
`hoodie.log.compaction.inline`.<br /><br />`Config Param: 
LOG_COMPACTION_BLOCKS_THRESHOLD`<br />`Since Version: 0.13.0` |
+
+:::note
+`hoodie.log.compaction.inline` is the only built-in way to schedule log 
compaction on a data table. There is no
+asynchronous log compaction service, SQL procedure, Hudi CLI command, or 
standalone utility for it, unlike compaction.
+Programmatic scheduling is available through the write client's 
`scheduleLogCompaction` and `logCompact` methods.
+:::
+
+The metadata table runs its own log compaction, controlled by a separate pair 
of configs:
+
+| Config Name | Default | Description |
+|---|---|---|
+| `hoodie.metadata.log.compaction.enable` | `false` (Optional) | Enables log 
compaction for the metadata table.<br /><br />`Config Param: 
ENABLE_LOG_COMPACTION_ON_METADATA_TABLE`<br />`Since Version: 0.14.0` |
+| `hoodie.metadata.log.compaction.blocks.threshold` | `5` (Optional) | Number 
of log blocks above which log compaction is scheduled on the metadata table.<br 
/><br />`Config Param: LOG_COMPACT_BLOCKS_THRESHOLD`<br />`Since Version: 
0.14.0` |

Review Comment:
   Verified, and not taken on in this PR.
   
   The anchor is dangling as you describe: `metadata.md:151` links to 
`compaction.md#delegating-mdt-compaction-to-an-external-platform` in both 
`next` and `version-1.2.0`, that heading exists in no version, 
`onBrokenAnchors` is unset in `docusaurus.config.js` so it defaults to `warn`, 
and the warning is present in **both** my baseline and my build — which is 
precisely why a diff-based check cannot see it. Good catch on the mechanism, 
not just the symptom.
   
   I am leaving it out deliberately. Taking it would mean adding a `### 
Delegating MDT Compaction to an External Platform` subsection covering 
`hoodie.metadata.table.service.manager.*`, and that content is about MDT 
**compaction** delegation generally rather than log compaction specifically — 
nesting it under `## Log Compaction` would put a general facility inside a 
feature section, and hoisting it to a sibling `##` grows this PR into a 
restructure of the page while it is being narrowed for correctness. The async 
delegation is already mentioned in prose where it is relevant, so nothing in 
the current diff depends on that heading existing.
   
   Happy to do it as a follow-up, and it would close the dangling anchor 
properly rather than as a side effect. If you would rather it land here, say so 
and I will add it.



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