somandal commented on code in PR #15517:
URL: https://github.com/apache/pinot/pull/15517#discussion_r2039903191


##########
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:
   For gauge you can track the total number of ongoing rebalances in an 
AtomicInteger and use that to increment / decrement and set the gauge value 
(you don't need to use the deprecated method). I did something similar for 
`BaseSegmentOperationsThrottler`. I think for this purpose gauge is probably a 
better fit.
   
   what race condition are you worried about? can you elaborate?
   
   I thought some more about how to count rebalances. I think if we want to 
simplify and count all rebalances (even no-ops, downtime, dryRun) that is fine, 
these are quick and the metric won't be high for too long.
   
   Meters are more for measuring rate. I don't think we care about rate of 
rebalances here, the absolute number is what is more useful. Can you do some 
research on the behavior of Meters for decrementing them and whether that is 
even a recommended pattern?
   
https://javadoc.io/doc/io.dropwizard.metrics/metrics-core/3.2.1/com/codahale/metrics/Meter.html
 



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