somandal commented on code in PR #15266:
URL: https://github.com/apache/pinot/pull/15266#discussion_r2021940809
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/ZkBasedTableRebalanceObserver.java:
##########
@@ -271,17 +315,279 @@ public static
TableRebalanceProgressStats.RebalanceStateStats getDifferenceBetwe
String instanceName = instanceStateEntry.getKey();
String sourceInstanceState = sourceInstanceStateMap.get(instanceName);
if (!targetStateInstanceState.equals(sourceInstanceState)) {
- rebalanceStats._replicasToRebalance++;
+ rebalanceStats._totalSegmentsToRebalance++;
hasSegmentConverged = false;
}
}
if (!hasSegmentConverged) {
- rebalanceStats._segmentsToRebalance++;
+ rebalanceStats._uniqueSegmentsToRebalance++;
}
}
int totalSegments = targetState.size();
- rebalanceStats._percentSegmentsToRebalance =
- (totalSegments == 0) ? 0 : ((double)
rebalanceStats._segmentsToRebalance / totalSegments) * 100.0;
+ rebalanceStats._percentRemainingSegmentsToRebalance =
+ (totalSegments == 0) ? 0 : ((double)
rebalanceStats._uniqueSegmentsToRebalance / totalSegments) * 100.0;
return rebalanceStats;
}
+
+ /**
+ * Updates the overall progress stats based on the current step's progress
stats. This should be called
+ * during the EV-IS convergence trigger to ensure the overall stats reflect
the changes as they are made.
+ * @param rebalanceProgressStats the rebalance stats
+ * @param lastStepStats step level stats from the last iteration
+ * @param latestStepStats latest step level stats calculated in this
iteration
+ * @return the newly calculated overall progress stats
+ */
+ @VisibleForTesting
+ static TableRebalanceProgressStats.RebalanceProgressStats
updateOverallProgressStatsFromStep(
+ TableRebalanceProgressStats rebalanceProgressStats,
+ TableRebalanceProgressStats.RebalanceProgressStats lastStepStats,
+ TableRebalanceProgressStats.RebalanceProgressStats latestStepStats) {
+ int numAdditionalSegmentsAdded =
+ latestStepStats._totalSegmentsToBeAdded -
lastStepStats._totalSegmentsToBeAdded;
+ int numAdditionalSegmentsDeleted =
+ latestStepStats._totalSegmentsToBeDeleted -
lastStepStats._totalSegmentsToBeDeleted;
+ int numSegmentAddsProcessedInLastStep =
Math.abs(lastStepStats._totalRemainingSegmentsToBeAdded
+ - latestStepStats._totalRemainingSegmentsToBeAdded);
+ int numSegmentDeletesProcessedInLastStep =
Math.abs(lastStepStats._totalRemainingSegmentsToBeDeleted
+ - latestStepStats._totalRemainingSegmentsToBeDeleted);
+ int numberNewUntrackedSegmentsAdded =
latestStepStats._totalUniqueNewUntrackedSegmentsDuringRebalance
+ - lastStepStats._totalUniqueNewUntrackedSegmentsDuringRebalance;
+
+ TableRebalanceProgressStats.RebalanceProgressStats overallProgressStats =
+ rebalanceProgressStats.getRebalanceProgressStatsOverall();
+
+ TableRebalanceProgressStats.RebalanceProgressStats newOverallProgressStats
=
+ new TableRebalanceProgressStats.RebalanceProgressStats();
+
+ newOverallProgressStats._totalSegmentsToBeAdded =
overallProgressStats._totalSegmentsToBeAdded
+ + numAdditionalSegmentsAdded;
+ newOverallProgressStats._totalSegmentsToBeDeleted =
overallProgressStats._totalSegmentsToBeDeleted
+ + numAdditionalSegmentsDeleted;
+ newOverallProgressStats._totalRemainingSegmentsToBeAdded =
numAdditionalSegmentsAdded == 0
+ ? overallProgressStats._totalRemainingSegmentsToBeAdded -
numSegmentAddsProcessedInLastStep
+ : overallProgressStats._totalRemainingSegmentsToBeAdded +
numSegmentAddsProcessedInLastStep;
+ newOverallProgressStats._totalRemainingSegmentsToBeDeleted =
numAdditionalSegmentsDeleted == 0
+ ? overallProgressStats._totalRemainingSegmentsToBeDeleted -
numSegmentDeletesProcessedInLastStep
+ : overallProgressStats._totalRemainingSegmentsToBeAdded +
numSegmentDeletesProcessedInLastStep;
+ newOverallProgressStats._totalRemainingSegmentsToConverge =
latestStepStats._totalRemainingSegmentsToConverge;
+ newOverallProgressStats._totalUniqueNewUntrackedSegmentsDuringRebalance =
+ overallProgressStats._totalUniqueNewUntrackedSegmentsDuringRebalance +
numberNewUntrackedSegmentsAdded;
+ newOverallProgressStats._percentageTotalSegmentsAddsRemaining =
+
calculatePercentageChange(newOverallProgressStats._totalSegmentsToBeAdded,
Review Comment:
I think it is correct:
```
private static double calculatePercentageChange(int totalSegmentsToChange,
int remainingSegmentsToChange) {
return totalSegmentsToChange == 0
? 0.0 : (double) remainingSegmentsToChange / (double)
totalSegmentsToChange * 100.0;
}
```
`_totalSegmentsToBeAdded` -> total segments to add
`_totalRemainingSegmentsToBeAdded` -> remaining segments to add
e.g.
Say _totalSegmentsToBeAdded = 100, and _totalRemainingSegmentsToBeAdded =
75, I want to do: (75.0 / 100.0 * 100.0 = 75%)
Or did you mean the parameters in the function should be swapped?
--
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
