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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
