hudi-agent commented on code in PR #18803:
URL: https://github.com/apache/hudi/pull/18803#discussion_r3430783758
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -683,6 +683,13 @@ public final class HoodieMetadataConfig extends
HoodieConfig {
+ "with the actual record count stored in the metadata table. This
validation runs in a distributed manner "
+ "using the compute engine. Disabled by default as it adds overhead
to the initialization process.");
+ public static final ConfigProperty<Boolean> ENABLE_DETAILED_METRICS =
ConfigProperty
+ .key(METADATA_PREFIX + ".enable.detailed.metrics")
+ .defaultValue(false)
+ .markAdvanced()
+ .sinceVersion("1.3.0")
+ .withDocumentation("Enables detailed metadata table metrics");
Review Comment:
🤖 nit: the doc string "Enables detailed metadata table metrics" doesn't tell
a user what "detailed" actually includes. Could you mention the specific
metrics this enables (e.g. size and file count per metadata partition)?
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -1349,6 +1360,11 @@ public Builder withAutoDeletePartitions(boolean
autoDeletePartitions) {
return this;
}
+ public Builder enableDetailedMetadataMetrics(boolean enableMetrics) {
Review Comment:
🤖 nit: the parameter name `enableMetrics` here is easy to confuse with the
existing `enableMetrics(boolean)` builder method for general metrics — could
you rename it to `enable` or `enableDetailedMetrics` to make the distinction
clearer?
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -1020,6 +1027,10 @@ public boolean isDropMetadataIndex(String indexName) {
return subIndexNameToDrop.contains(indexName);
}
+ public boolean shouldEnableDetailedMetrics() {
Review Comment:
🤖 nit: the existing sibling method is `isMetricsEnabled()`, so
`shouldEnableDetailedMetrics()` feels inconsistent — could you rename it to
`isDetailedMetricsEnabled()` to match the `is*` pattern used throughout this
class?
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
