Skip to content

Commit 786523d

Browse files
committed
xds: WRR rr_fallback should trigger with one endpoint weight
From gRFC A58: > When less than two subchannels have load info, all subchannels will > get the same weight and the policy will behave the same as round_robin
1 parent b108ed3 commit 786523d

File tree

2 files changed

+20
-8
lines changed

2 files changed

+20
-8
lines changed

xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -598,13 +598,15 @@ static final class StaticStrideScheduler {
598598
if (numWeightedChannels > 0) {
599599
unscaledMeanWeight = sumWeight / numWeightedChannels;
600600
unscaledMaxWeight = Math.min(unscaledMaxWeight, (float) (K_MAX_RATIO * unscaledMeanWeight));
601-
usesRoundRobin = false;
602601
} else {
603-
// Fall back to round robin if all values are non-positives
604-
usesRoundRobin = true;
602+
// Fall back to round robin if all values are non-positives. Note that
603+
// numWeightedChannels == 1 also behaves like RR because the weights are all the same, but
604+
// the weights aren't 1, so it doesn't go through this path.
605605
unscaledMeanWeight = 1;
606606
unscaledMaxWeight = 1;
607607
}
608+
// We need at least two weights for WRR to be distinguishable from round_robin.
609+
usesRoundRobin = numWeightedChannels < 2;
608610

609611
// Scales weights s.t. max(weights) == K_MAX_WEIGHT, meanWeight is scaled accordingly.
610612
// Note that, since we cap the weights to stay within K_MAX_RATIO, meanWeight might not

xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,13 +1190,17 @@ public void metrics() {
11901190
verifyLongCounterRecord("grpc.lb.wrr.endpoint_weight_not_yet_usable", 1, 2);
11911191
verifyLongCounterRecord("grpc.lb.wrr.endpoint_weight_not_yet_usable", 1, 3);
11921192

1193-
// Send each child LB state an ORCA update with some valid utilization/qps data so that weights
1194-
// can be calculated.
1193+
// Send one child LB state an ORCA update with some valid utilization/qps data so that weights
1194+
// can be calculated, but it's still essentially round_robin
11951195
Iterator<ChildLbState> childLbStates = wrr.getChildLbStates().iterator();
11961196
((WeightedChildLbState)childLbStates.next()).new OrcaReportListener(
11971197
weightedConfig.errorUtilizationPenalty).onLoadReport(
11981198
InternalCallMetricRecorder.createMetricReport(0.1, 0, 0.1, 1, 0, new HashMap<>(),
11991199
new HashMap<>(), new HashMap<>()));
1200+
1201+
fakeClock.forwardTime(1, TimeUnit.SECONDS);
1202+
1203+
// Now send a second child LB state an ORCA update, so there's real weights
12001204
((WeightedChildLbState)childLbStates.next()).new OrcaReportListener(
12011205
weightedConfig.errorUtilizationPenalty).onLoadReport(
12021206
InternalCallMetricRecorder.createMetricReport(0.1, 0, 0.1, 1, 0, new HashMap<>(),
@@ -1210,9 +1214,15 @@ public void metrics() {
12101214
// weights were updated
12111215
reset(mockMetricRecorder);
12121216

1213-
// We go forward in time past the default 10s blackout period before weights can be considered
1214-
// for wrr. The eights would get updated as the default update interval is 1s.
1215-
fakeClock.forwardTime(11, TimeUnit.SECONDS);
1217+
// We go forward in time past the default 10s blackout period for the first child. The weights
1218+
// would get updated as the default update interval is 1s.
1219+
fakeClock.forwardTime(9, TimeUnit.SECONDS);
1220+
1221+
verifyLongCounterRecord("grpc.lb.wrr.rr_fallback", 1, 1);
1222+
1223+
// And after another second the other children have weights
1224+
reset(mockMetricRecorder);
1225+
fakeClock.forwardTime(1, TimeUnit.SECONDS);
12161226

12171227
// Since we have weights on all the child LB states, the weight update should not result in
12181228
// further rr_fallback metric entries.

0 commit comments

Comments
 (0)