somandal commented on code in PR #15266:
URL: https://github.com/apache/pinot/pull/15266#discussion_r2029524674
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalanceProgressStats.java:
##########
@@ -19,33 +19,25 @@
package org.apache.pinot.controller.helix.core.rebalance;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
+import java.util.Objects;
/**
* These are rebalance stats as to how the current state is, when compared to
the target state.
* Eg: If the current has 4 segments whose replicas (16) don't match the
target state, _segmentsToRebalance
* is 4 and _replicasToRebalance is 16.
*/
Review Comment:
done
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalanceProgressStats.java:
##########
@@ -118,6 +128,165 @@ 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 additional segments were added in the previous step, these need to
be added to the remaining to be added
+ // number as they are new segments that should be tracked for completion
+ // Example:
+ // currentStepStats: totalSegmentsToBeAdded = 20
+ // previousStepStats: totalSegmentsToBeAdded = 10
+ // numAdditionalSegmentsAdded = 20 - 10 = 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
+ // If 'numAdditionalSegmentsAdded' = 0, that means no new segments were
added in the previous step, and if there
+ // was a change in the segments processed in the last step, they
completed processing
+ currentOverallStats._totalRemainingSegmentsToBeAdded =
numAdditionalSegmentsAdded == 0
+ ? previousOverallStats._totalRemainingSegmentsToBeAdded -
numSegmentAddsProcessedInLastStep
+ : previousOverallStats._totalRemainingSegmentsToBeAdded +
numSegmentAddsProcessedInLastStep;
Review Comment:
updated, please take a look
--
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]
