jtuglu1 commented on code in PR #19422:
URL: https://github.com/apache/druid/pull/19422#discussion_r3327672625
##########
server/src/main/java/org/apache/druid/server/coordinator/balancer/DiskNormalizedCostBalancerStrategy.java:
##########
@@ -26,40 +26,40 @@
/**
* A {@link BalancerStrategy} which normalizes the cost of placing a segment
on a
- * server as calculated by {@link CostBalancerStrategy} by multiplying it by
the
- * server's disk usage ratio.
+ * server as calculated by {@link CostBalancerStrategy} by dividing by the
Review Comment:
Fixed. `docs/design/coordinator.md` and `docs/configuration/index.md` now
describe the projected-headroom formula instead of the old `cost * (diskUsed /
maxSize)` multiplier.
##########
server/src/main/java/org/apache/druid/server/coordinator/balancer/DiskNormalizedCostBalancerStrategy.java:
##########
@@ -85,17 +85,16 @@ protected double computePlacementCost(
return cost;
}
- // Guard against NaN propagation in the cost comparator if a server
- // somehow reports a non-positive maxSize. Such a server cannot hold
- // anything and will be rejected by canLoadSegment, so returning the
- // raw cost is safe.
+ // A server with non-positive maxSize cannot hold anything and will be
+ // rejected by canLoadSegment; return the raw cost to avoid NaN
propagation.
final long maxSize = server.getMaxSize();
if (maxSize <= 0) {
return cost;
}
- double usageRatio = (double) server.getSizeUsed() / maxSize;
- double normalizedCost = cost * usageRatio;
+ final double usageRatio = (double) server.getSizeUsed() / maxSize;
+ final double headroom = Math.max(EPSILON, 1.0 - usageRatio);
+ double normalizedCost = cost / headroom;
Review Comment:
Fixed. The marginal threshold test has been adjusted for the
projected-headroom formula, and the focused test now passes:
`mvn test -pl server -am
-Dtest="org.apache.druid.server.coordinator.balancer.DiskNormalizedCostBalancerStrategyTest"
-Dsurefire.failIfNoSpecifiedTests=false -Pskip-static-checks
-Dweb.console.skip=true -T1C`
I also added projected-size coverage for both load and move decisions, and
verified the diskNormalized assertions fail if the helper is temporarily
swapped to `CostBalancerStrategy`.
--
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]
