voonhous commented on code in PR #18942:
URL: https://github.com/apache/hudi/pull/18942#discussion_r3864408587


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java:
##########
@@ -456,8 +459,12 @@ public void updateClusteringFileCreationMetrics(long 
durationInMs) {
     reportMetrics(HoodieTimeline.CLUSTERING_ACTION, "fileCreationTime", 
durationInMs);
   }
 
-  public void updateTableServiceInstantMetrics(final HoodieActiveTimeline 
activeTimeline) {
-    updateEarliestPendingInstant(activeTimeline, 
EARLIEST_PENDING_CLUSTERING_INSTANT_STR, HoodieTimeline.CLUSTERING_ACTION);
+  public void updateTableServiceInstantMetrics(final HoodieActiveTimeline 
activeTimeline, final InstantGenerator instantGenerator) {
+    // Clustering is scheduled as CLUSTERING_ACTION only on timeline layout 2. 
On table version six it is scheduled as
+    // REPLACE_COMMIT_ACTION, which insert_overwrite and delete_partition 
share, so the clustering plan is what
+    // identifies it rather than the action name.

Review Comment:
   Edited the body directly so this does not hold up the merge: Summary, Impact 
and Risk now describe the `filterPendingClusteringTimeline()` revision, the 
metric is named `earliestInflightClusteringInstant`, and the completed-side 
caveat is stated in the issue section. Could you skim it and shout if anything 
misrepresents the change? Nothing else open on this thread.
   



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