voonhous commented on code in PR #19572:
URL: https://github.com/apache/hudi/pull/19572#discussion_r3757140205
##########
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.
Review Comment:
The async path does exist and the config keys are right, but the sentence as
written is a footgun: following it without a table service manager deployed
will stall metadata table compaction.
**Delegation stops execution; nothing in Hudi picks it up.** At
`release-1.2.0`, `HoodieBackedTableMetadataWriter:2235-2244` still *schedules*
the MDT log compaction, then skips running it:
```java
Option<String> scheduledLogCompaction =
writeClient.scheduleLogCompaction(Option.empty());
if (scheduledLogCompaction.isPresent()) {
LOG.info("Log compaction is scheduled for timestamp {}",
scheduledLogCompaction.get());
if (shouldDelegateToTableServiceManager(metadataWriteConfig,
ActionType.logcompaction)) {
LOG.info("Skipping execution of log compaction on MDT as it is delegated
to table service manager.");
} else {
writeClient.logCompact(scheduledLogCompaction.get(), true);
}
}
```
`runPendingTableServicesOperationsAndRefreshTimeline` at `:2170-2176` skips
`runAnyPendingLogCompactions()` on the same condition, so already-pending
instants are not picked up on a later commit either.
**The executor is an external service that Hudi does not ship.**
`hoodie.table.service.manager.uris` defaults to `http://localhost:9091`, and
the only thing in the tree is `HoodieTableServiceManagerClient` -- there is no
server at `release-1.2.0` or on `apache/master`.
`HoodieTableServiceManagerConfig`'s own javadoc says so:
```java
/**
* Configurations used by the Hudi Table Service Manager.
*
* TODO: enable docs gen by adding {@link ConfigClassProperty} after TSM is
landed (HUDI-3475)
*/
```
That TODO is also why these keys are absent from the generated TSM section
of `configurations.md`.
**And it interlocks with the note you added two paragraphs down.**
`validateCompactionScheduling` (`HoodieBackedTableMetadataWriter:2281-2296`)
returns false while any pending log compaction *or* compaction instant exists.
So the combined outcome of following this sentence without a TSM is:
`logcompaction.requested` instants accumulate on the MDT timeline, nothing
executes them, and MDT major compaction stops being scheduled too.
Not asking you to drop the sentence -- the information is worth having.
Please make the prerequisite explicit, something like:
> The metadata table runs its own log compaction, controlled by a separate
pair of configs. Unlike the data table, its execution can be delegated to an
external table service manager by setting
`hoodie.metadata.table.service.manager.enabled=true` together with
`hoodie.metadata.table.service.manager.actions=logcompaction`. Note that this
only stops the writer from executing log compaction inline: a table service
manager must be deployed and reachable at `hoodie.table.service.manager.uris`,
otherwise pending `logcompaction` instants accumulate and, per the note below,
metadata table compaction stops being scheduled as well.
One thing that would strengthen it if you want it: on Flink at 1.2.0 the
MDT's log compaction is executed by the existing compaction pipeline
(`MetadataTableCompactionPlanHandler` /
`CompactionUtil.scheduleMetadataCompaction`), with no TSM involved. So "async
MDT log compaction" means two different things depending on the engine, and
only the TSM route carries this prerequisite.
Leaving this thread open; everything else from my pass is resolved.
##########
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:
Review Comment:
Point 2 is correctly captured -- `validateCompactionScheduling`
(`HoodieBackedTableMetadataWriter:2281-2296` at `release-1.2.0`) returns false
while either a pending log compaction or a pending compaction exists, gated on
`metadataWriteConfig.isLogCompactionEnabled()`, with the proc-time-ordering
comment your sentence paraphrases. Accurate as written.
On the open question you flagged rather than guessed at: **you were right
not to write anything, and the gap is genuinely moot at 1.2.0.** I checked the
combination you named:
```console
$ git grep -n "isOptimizedLogBlocksScanEnabled\|OptimizedLogBlockScan"
release-1.2.0 \
--
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java
(no output)
$ git grep -rn
"scanInternalV1\|scanInternalV2\|enableOptimizedLogBlocksScan" release-1.2.0 \
-- 'hudi-common/src/main/java/org/apache/hudi/common/table/log/'
release-1.2.0:.../HoodieMergedLogRecordReader.java:176: private boolean
enableOptimizedLogBlocksScan = false;
```
The `HoodieBackedTableMetadata:551` call site that existed at 1.1.1 is gone
at 1.2.0, and there is no scanV1/scanV2 branch left anywhere under
`common/table/log/` -- the single remaining reference is a vestigial builder
field with no reader behind it. #17520 removed the branch for the MDT reader as
well as the data-table reader, so
`hoodie.metadata.optimized.log.blocks.scan.enable` cannot gate the skip on
either path at 1.2.0. There is nothing left to document, which is exactly the
conclusion you reached without the verification.
Resolving.
##########
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:
Agreed, and your reasoning is better than my suggestion. Nesting a general
MDT-compaction-delegation facility under `## Log Compaction` would misfile it
the same way putting log compaction beside UPSERT in the write-operation
taxonomy would have -- and hoisting it to a sibling `##` turns a correctness
narrowing into a page restructure. Both are the wrong trade here.
Leaving this thread open as a marker so the dangling anchor is not lost,
rather than resolving it. It is not a blocker for this PR.
--
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]
