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