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