Skip to content

Commit 028afbe

Browse files
authored
xds: Implement equals in WRRLBConfig
Just an is a8de9f0, lack of equals causes cluster_resolver to consider every update a different configuration and restart itself. Handling NaN should really be prevented with validation, but it looks like that would lead to yak shaving at the moment. b/435208946
1 parent afdbecb commit 028afbe

File tree

2 files changed

+62
-0
lines changed

2 files changed

+62
-0
lines changed

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.util.Collection;
4949
import java.util.HashSet;
5050
import java.util.List;
51+
import java.util.Objects;
5152
import java.util.Random;
5253
import java.util.Set;
5354
import java.util.concurrent.ScheduledExecutorService;
@@ -738,6 +739,32 @@ private WeightedRoundRobinLoadBalancerConfig(long blackoutPeriodNanos,
738739
this.errorUtilizationPenalty = errorUtilizationPenalty;
739740
}
740741

742+
@Override
743+
public boolean equals(Object o) {
744+
if (!(o instanceof WeightedRoundRobinLoadBalancerConfig)) {
745+
return false;
746+
}
747+
WeightedRoundRobinLoadBalancerConfig that = (WeightedRoundRobinLoadBalancerConfig) o;
748+
return this.blackoutPeriodNanos == that.blackoutPeriodNanos
749+
&& this.weightExpirationPeriodNanos == that.weightExpirationPeriodNanos
750+
&& this.enableOobLoadReport == that.enableOobLoadReport
751+
&& this.oobReportingPeriodNanos == that.oobReportingPeriodNanos
752+
&& this.weightUpdatePeriodNanos == that.weightUpdatePeriodNanos
753+
// Float.compare considers NaNs equal
754+
&& Float.compare(this.errorUtilizationPenalty, that.errorUtilizationPenalty) == 0;
755+
}
756+
757+
@Override
758+
public int hashCode() {
759+
return Objects.hash(
760+
blackoutPeriodNanos,
761+
weightExpirationPeriodNanos,
762+
enableOobLoadReport,
763+
oobReportingPeriodNanos,
764+
weightUpdatePeriodNanos,
765+
errorUtilizationPenalty);
766+
}
767+
741768
static final class Builder {
742769
long blackoutPeriodNanos = 10_000_000_000L; // 10s
743770
long weightExpirationPeriodNanos = 180_000_000_000L; //3min

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.google.common.collect.ImmutableMap;
3636
import com.google.common.collect.Lists;
3737
import com.google.common.collect.Maps;
38+
import com.google.common.testing.EqualsTester;
3839
import com.google.protobuf.Duration;
3940
import io.grpc.Attributes;
4041
import io.grpc.CallOptions;
@@ -215,6 +216,40 @@ public void pickChildLbTF() throws Exception {
215216
weightedPicker.pickSubchannel(mockArgs);
216217
}
217218

219+
@Test
220+
public void config_equalsTester() {
221+
WeightedRoundRobinLoadBalancerConfig defaults =
222+
WeightedRoundRobinLoadBalancerConfig.newBuilder().build();
223+
new EqualsTester()
224+
.addEqualityGroup(
225+
WeightedRoundRobinLoadBalancerConfig.newBuilder().build(),
226+
WeightedRoundRobinLoadBalancerConfig.newBuilder().build(),
227+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
228+
.setBlackoutPeriodNanos(defaults.blackoutPeriodNanos).build())
229+
.addEqualityGroup(
230+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
231+
.setBlackoutPeriodNanos(5).build())
232+
.addEqualityGroup(
233+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
234+
.setWeightExpirationPeriodNanos(5).build())
235+
.addEqualityGroup(
236+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
237+
.setEnableOobLoadReport(true).build())
238+
.addEqualityGroup(
239+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
240+
.setOobReportingPeriodNanos(5).build())
241+
.addEqualityGroup(
242+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
243+
.setWeightUpdatePeriodNanos(5).build())
244+
.addEqualityGroup(
245+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
246+
.setErrorUtilizationPenalty(0.5F).build())
247+
.addEqualityGroup(
248+
WeightedRoundRobinLoadBalancerConfig.newBuilder()
249+
.setErrorUtilizationPenalty(Float.NaN).build())
250+
.testEquals();
251+
}
252+
218253
@Test
219254
public void wrrLifeCycle() {
220255
syncContext.execute(() -> wrr.acceptResolvedAddresses(ResolvedAddresses.newBuilder()

0 commit comments

Comments
 (0)