shangxinli commented on code in PR #18942:
URL: https://github.com/apache/hudi/pull/18942#discussion_r4484660707
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java:
##########
@@ -491,6 +493,29 @@ private void updateEarliestPendingInstant(final
HoodieActiveTimeline activeTimel
}
}
+ /**
+ * Pending clustering carries CLUSTERING_ACTION on timeline layout 2 but
REPLACE_COMMIT_ACTION on layout 1, where
+ * insert_overwrite and delete_partition share the action and are separated
by the requested replace metadata's
+ * operationType. {@link HoodieTimeline#filterPendingClusteringTimeline()}
resolves that per layout, so it is read
+ * once here for both clustering metrics.
+ */
+ private void updatePendingClusteringInstantMetrics(final
HoodieActiveTimeline activeTimeline) {
+ HoodieTimeline pendingClustering;
+ try {
+ pendingClustering = activeTimeline.filterPendingClusteringTimeline();
+ } catch (Exception e) {
+ // postCommit does not catch, so an unreadable requested replace
metadata must not fail the commit.
+ log.debug("Failed to read pending clustering instants, reporting none",
e);
+ updateMetric(HoodieTimeline.CLUSTERING_ACTION,
PENDING_CLUSTERING_INSTANT_COUNT_STR, 0);
Review Comment:
nit: on a failure to read the pending clustering timeline this reports 0
pending clustering instants instead of skipping the metric, which could mask a
real stuck clustering as healthy in monitoring.
--
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]
