J-HowHuang commented on code in PR #15517:
URL: https://github.com/apache/pinot/pull/15517#discussion_r2039838035


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -431,6 +432,10 @@ private RebalanceResult doRebalance(TableConfig 
tableConfig, RebalanceConfig reb
         estimatedAverageSegmentSizeInBytes, allSegmentsFromIdealState, 
segmentsToMonitor);
 
     // Record the beginning of rebalance
+    // We emit this metric only when the rebalance does get to this code path, 
i.e. excluding dry-run and downtime
+    if (_controllerMetrics != null) {

Review Comment:
   We can't decrement by 1 in onSuccess/onNoop/onError because some code path 
that never go to `updateOnStart` also call them (e.g. dry-run).
   
   Using meter instead of gauge is because I saw this comment 
https://github.com/apache/pinot/blob/870cf576357e30a17f37f3c414a101fd88bc8a68/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java#L470-L473
   
   And also using gauge might run into race condition and end up having no 
rebalance running but still have a global count > 0, which could be quite 
misleading (haven't proved this will be the case but my initial thought)



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to