somandal commented on code in PR #15266:
URL: https://github.com/apache/pinot/pull/15266#discussion_r2029695528
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalanceProgressStats.java:
##########
@@ -118,6 +126,209 @@ public RebalanceStateStats
getCurrentToTargetConvergence() {
return _currentToTargetConvergence;
}
+ public RebalanceProgressStats getRebalanceProgressStatsOverall() {
+ return _rebalanceProgressStatsOverall;
+ }
+
+ public RebalanceProgressStats getRebalanceProgressStatsCurrentStep() {
+ return _rebalanceProgressStatsCurrentStep;
+ }
+
+ /**
+ * Updates the overall and step progress stats based on the latest
calculated 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 currentStepStats latest step level stats calculated in this
iteration
+ */
+ public void updateOverallAndStepStatsFromLatestStepStats(
+ TableRebalanceProgressStats.RebalanceProgressStats currentStepStats) {
+ // Fetch the step level and overall stats that were calculated in the last
convergence check. These will be used
+ // to calculate the overall stats in the current convergence check
+ TableRebalanceProgressStats.RebalanceProgressStats previousStepStats =
getRebalanceProgressStatsCurrentStep();
+ TableRebalanceProgressStats.RebalanceProgressStats previousOverallStats =
getRebalanceProgressStatsOverall();
+
+ // Calculate the new segments added / deleted since the last step to track
overall change in segments. These are
+ // only new segments tracked by table rebalance (segments that aren't
tracked by rebalance convergence check are
+ // ignored because the convergence check does not wait for them to
complete)
+ int numAdditionalSegmentsAdded =
+ currentStepStats._totalSegmentsToBeAdded -
previousStepStats._totalSegmentsToBeAdded;
+ int numAdditionalSegmentsDeleted =
+ currentStepStats._totalSegmentsToBeDeleted -
previousStepStats._totalSegmentsToBeDeleted;
+
+ // Calculate the carry-over segment adds / deletes from the previous
rebalance step if any (can happen if
+ // bestEfforts=true or if there are deletions left over from the last
step) and the difference in the number of
+ // segments processed since the previous stats update.
+ // Example of what carry-over will look like:
+ // previousStepStats: totalSegmentsToBeAdded = 10
+ // previousStepStats: totalRemainingSegmentsToBeAdded = 15 (this can
only be larger than totalSegmentsToBeAdded
+ // if
carry-over segments exist)
+ // Carry-over: 15 - 10 = 5 segments -> which were added in the last IS
update by rebalance, but which didn't
+ // finish getting added before making the next assignment
IS update
+ // Using the above, we calculate the 'upperBoundOnSegmentsAdded/Deleted'
as the maximum segments to use when
+ // calculating the number of segments processed (number of segments that
got added / deleted in EV based on expected
+ // IS) since the last step. We want to skip including the carry-over
segments if any as they are tracked separately.
+ int upperBoundOnSegmentsAdded =
+ previousStepStats._totalRemainingSegmentsToBeAdded >
previousStepStats._totalSegmentsToBeAdded
+ ? previousStepStats._totalSegmentsToBeAdded :
previousStepStats._totalRemainingSegmentsToBeAdded;
+ int upperBoundOnSegmentsDeleted =
+ previousStepStats._totalRemainingSegmentsToBeDeleted >
previousStepStats._totalSegmentsToBeDeleted
+ ? previousStepStats._totalSegmentsToBeDeleted :
previousStepStats._totalRemainingSegmentsToBeDeleted;
+
+ // Number of adds / deletes processed in the last step. Take the absolute
here since if new segments were added /
+ // deleted in the current step that are being tracking, we need to treat
those as new segments that remain to
+ // be processed.
+ int numSegmentAddsProcessedInLastStep = Math.abs(upperBoundOnSegmentsAdded
+ - currentStepStats._totalRemainingSegmentsToBeAdded);
+ int numSegmentDeletesProcessedInLastStep =
Math.abs(upperBoundOnSegmentsDeleted
+ - currentStepStats._totalRemainingSegmentsToBeDeleted);
+
+ // Number of untracked segments that were added
+ int numberNewUntrackedSegmentsAdded =
currentStepStats._totalUniqueNewUntrackedSegmentsDuringRebalance
+ - previousStepStats._totalUniqueNewUntrackedSegmentsDuringRebalance;
+
+ TableRebalanceProgressStats.RebalanceProgressStats currentOverallStats =
+ new TableRebalanceProgressStats.RebalanceProgressStats();
+
+ // New number of total segment adds / deletes should include any new
segments added from the previous stats
+ currentOverallStats._totalSegmentsToBeAdded =
previousOverallStats._totalSegmentsToBeAdded
+ + numAdditionalSegmentsAdded;
+ currentOverallStats._totalSegmentsToBeDeleted =
previousOverallStats._totalSegmentsToBeDeleted
+ + numAdditionalSegmentsDeleted;
+
+ // If carry-over segments are present, keep the upper bound of remaining
segments to be added / deleted at the same
+ // level as the previous stats. This is so we track progress to handle the
carry-over segments before we start
+ // tracking progress against the segments that need to be handled as part
of the current rebalance step.
+ if (currentStepStats._totalCarryOverSegmentsToBeAdded > 0) {
+ currentOverallStats._totalRemainingSegmentsToBeAdded =
previousOverallStats._totalRemainingSegmentsToBeAdded;
+ } else {
+ if (numAdditionalSegmentsAdded == 0) {
+ // No new additional segments added, subtract the number of processed
segments from the last step
+ currentOverallStats._totalRemainingSegmentsToBeAdded =
+ previousOverallStats._totalRemainingSegmentsToBeAdded -
numSegmentAddsProcessedInLastStep;
+ } else {
+ if (currentStepStats._totalRemainingSegmentsToBeAdded <=
previousStepStats._totalRemainingSegmentsToBeAdded) {
+ // Case I: currentStepStats._totalRemainingSegmentsToBeAdded
+ // <= previousStepStats._totalRemainingSegmentsToBeAdded
+ // Example:
+ // currentStepStats: totalSegmentsToBeAdded = 20
+ // previousStepStats: totalSegmentsToBeAdded = 10
+ // numAdditionalSegmentsAdded = 20 - 10 = 10
+ // currentStepStats: totalRemainingSegmentsToBeAdded = 5
+ // previousStepStats: totalRemainingSegmentsToBeAdded = 5
+ // remainingSegmentsWithoutConsideringProcessed = 5 + 10 = 15
+ // netProcessedSegments = 15 - 5 = 10 -> how much to subtract
after adding additional segments to remaining
+ int remainingSegmentsWithoutConsideringProcessed =
+ previousStepStats._totalRemainingSegmentsToBeAdded +
numAdditionalSegmentsAdded;
+ int netProcessedSegments =
remainingSegmentsWithoutConsideringProcessed
+ - currentStepStats._totalRemainingSegmentsToBeAdded;
+ currentOverallStats._totalRemainingSegmentsToBeAdded =
previousOverallStats._totalRemainingSegmentsToBeAdded
+ + numAdditionalSegmentsAdded - netProcessedSegments;
+ } else {
+ // Case II: currentStepStats._totalRemainingSegmentsToBeAdded
+ // > previousStepStats._totalRemainingSegmentsToBeAdded
+ // Example:
+ // currentStepStats: totalSegmentsToBeAdded = 20
+ // previousStepStats: totalSegmentsToBeAdded = 10
+ // numAdditionalSegmentsAdded = 20 - 10 = 10
+ // currentStepStats: totalRemainingSegmentsToBeAdded = 15
+ // previousStepStats: totalRemainingSegmentsToBeAdded = 10
+ // In the above scenario, the new 'numAdditionalSegmentsAdded' also
need to be tracked as
+ // 'totalRemainingSegmentsToBeAdded' for the current step, thus we
need to add the number of segments
+ // processed
+ currentOverallStats._totalRemainingSegmentsToBeAdded =
+ previousOverallStats._totalRemainingSegmentsToBeAdded +
numSegmentAddsProcessedInLastStep;
+ }
+ }
+ }
Review Comment:
yeah makes sense, simplified it. i was calculating overall segments
differently earlier (untracked as part of total) and I think this Math.abs() is
some artifact from code
--
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
