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


##########
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:
   I was thinking more, does it make sense to update this metric in 
`ZkBasedTableRebalanceObserver` similar to how `TABLE_REBALANCE_IN_PROGRESS` is 
updated today (except for increasing it, only increase it in the 
`ZkBasedTableRebalanceObserver::updateOnStart` instead of whenever `onTrigger` 
is called)? I think the `onSuccess`/`onNoop`/`onError` triggers are always 
called (do verify this)?
   
   Also any sense in using a Gauge instead? I guess main drawback with Gauge is 
you'll need to find a way to calculate the exact value to set rather than a 
simple increment / decrement. 
   
   I recall that meters may not be great to use if we need to decrement - do 
some research on that if you can?



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