hudi-agent commented on code in PR #19868:
URL: https://github.com/apache/hudi/pull/19868#discussion_r3960950185
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataMetrics.java:
##########
@@ -84,6 +85,22 @@ public class HoodieMetadataMetrics implements Serializable {
public static final String LOG_COMPACTION_FAILURES =
"logcompaction_failures";
public static final String PENDING_COMPACTIONS_FAILURES =
"pending_compactions_failures";
+ // Metadata table compaction health. The existing per-partition
baseFileCount/logFileCount gauges
+ // show the current shape of the metadata table, but not whether compaction
is keeping up with it.
+ // Completed delta commits on the metadata table since the last completed
compaction, sampled before
+ // table services run. On a healthy table this sawtooths up to
hoodie.metadata.compact.max.delta.commits
+ // and falls back after each compaction, so alert on a multiple of that
config rather than on the
+ // config value itself. A value that climbs past the peak and keeps going
means metadata table
+ // compaction is not being scheduled or is failing.
+ public static final String STAT_DELTA_COMMITS_SINCE_LAST_COMPACTION =
"deltaCommitsSinceLastCompaction";
+ // Base and log file counts summed across all enabled metadata partitions.
+ public static final String STAT_TOTAL_BASE_FILE_COUNT = "totalBaseFileCount";
+ public static final String STAT_TOTAL_LOG_FILE_COUNT = "totalLogFileCount";
+ // Log files per base file across the whole metadata table, as a percentage,
since gauges are long
+ // valued: 100 means one log file per base file, 500 means five. A rising
value means readers must
Review Comment:
🤖 nit: the existing per-partition stats are named `STAT_COUNT_BASE_FILES` /
`STAT_COUNT_LOG_FILES` (COUNT first), but these new totals flip the order to
`STAT_TOTAL_BASE_FILE_COUNT` (COUNT last). Might be worth aligning the naming
pattern, e.g. `STAT_TOTAL_COUNT_BASE_FILES`, so the two families read
consistently.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataMetrics.java:
##########
@@ -164,6 +181,48 @@ public void updateSizeMetrics(HoodieTableMetaClient
metaClient, HoodieBackedTabl
for (Map.Entry<String, String> e : stats.entrySet()) {
setMetric(e.getKey(), Long.parseLong(e.getValue()));
}
+ long totalBaseFiles = sumStat(stats, metadataPartitions,
STAT_COUNT_BASE_FILES);
Review Comment:
🤖 Is it intended that these three gauges only show up when detailed metrics
are on? The only caller of `updateSizeMetrics`
(`HoodieBackedTableMetadataWriter` ~L1389) is guarded by
`isDetailedMetricsEnabled()`, so `deltaCommitsSinceLastCompaction` will be
emitted by default while
`totalBaseFileCount`/`totalLogFileCount`/`logToBaseFileRatioPercent` won't —
might be worth calling out in the description, since the docs here say to read
them together.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -1543,8 +1543,20 @@ public void performTableServices(Option<String>
inFlightInstantTimestamp, boolea
.filterCompletedInstants()
.lastInstant();
if (!lastInstant.isPresent()) {
+ metrics.ifPresent(m -> m.updateDeltaCommitsSinceLastCompaction(0L));
Review Comment:
🤖 nit: there are now two separate `metrics.ifPresent(m ->
m.updateDeltaCommitsSinceLastCompaction(...))` calls a few lines apart (one for
the early-return zero case, one for the computed value). Consider factoring the
delta-commit calculation into a small helper so the metric is updated from a
single call site.
<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]
