J-HowHuang commented on code in PR #19176:
URL: https://github.com/apache/pinot/pull/19176#discussion_r3737361505
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/DefaultRebalancePreChecker.java:
##########
@@ -327,22 +332,51 @@ protected RebalancePreCheckerResult
checkDiskUtilization(PreCheckContext preChec
long diskUtilizationGain = newSegmentSet.size() * avgSegmentSize;
long diskUtilizationLoss = removedSegmentSet.size() * avgSegmentSize;
- long diskUtilizationFootprint =
- diskUsage.getUsedSpaceBytes() + diskUtilizationGain - (worstCase ? 0
: diskUtilizationLoss);
- double diskUtilizationFootprintRatio =
- (double) diskUtilizationFootprint / diskUsage.getTotalSpaceBytes();
-
- if (diskUtilizationFootprintRatio >= threshold) {
- isDiskUtilSafe = false;
- message.append(sep)
- .append(server)
- .append(String.format(" (%d%%)", (short)
(diskUtilizationFootprintRatio * 100)));
- sep = ", ";
- }
+ // While the rebalance is running, the segments being added can co-exist
with the ones being removed
+ addIfOverThreshold(serversUnsafeDuringRebalance, server,
+ (double) (diskUsage.getUsedSpaceBytes() + diskUtilizationGain) /
diskUsage.getTotalSpaceBytes(), threshold);
+ addIfOverThreshold(serversUnsafeAfterRebalance, server,
+ (double) (diskUsage.getUsedSpaceBytes() + diskUtilizationGain -
diskUtilizationLoss)
+ / diskUsage.getTotalSpaceBytes(), threshold);
+ }
+
+ // A server over the threshold once the rebalance is done is over it
during the rebalance as well, so the end state
+ // is what to report first: it is both the more severe problem and the one
that has to be solved by adding capacity
+ // rather than by tuning the rebalance config
+ if (!serversUnsafeAfterRebalance.isEmpty()) {
+ return RebalancePreCheckerResult.error(
+ getUnsafeDiskUtilizationMessage("AFTER rebalance",
serversUnsafeAfterRebalance, threshold));
+ }
+ String withinThreshold = String.format("Within threshold (<%d%%)", (short)
(threshold * 100));
+ if (serversUnsafeDuringRebalance.isEmpty()) {
+ return RebalancePreCheckerResult.pass(withinThreshold);
}
- return isDiskUtilSafe ? RebalancePreCheckerResult.pass(
- String.format("Within threshold (<%d%%)", (short) (threshold * 100)))
- : RebalancePreCheckerResult.error(message.toString());
+ // lowDiskMode is the only way to rule the transient disk usage above out,
since it waits for the segments to be
+ // deleted before adding the new ones. It is however only honored by the
incremental rebalance path, which downtime
+ // skips altogether by replacing the IdealState with the target assignment
in one go
+ RebalanceConfig rebalanceConfig = preCheckContext.getRebalanceConfig();
+ if (rebalanceConfig.isLowDiskMode() && !rebalanceConfig.isDowntime()) {
Review Comment:
Afaik `bestEfforts` is orthogonal to `lowDiskMode`, can you explain your
concern?
--
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]
