Skip to content

Commit a65ecef

Browse files
temawitemawi
authored
xds: ring_hash to use acceptResolvedAddresses() (#9617)
Part of a migration to move all LBs away from handleResolvedAddresses()
1 parent a97db60 commit a65ecef

File tree

2 files changed

+54
-27
lines changed

2 files changed

+54
-27
lines changed

‎xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ final class RingHashLoadBalancer extends LoadBalancer {
8181
}
8282

8383
@Override
84-
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
84+
public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
8585
logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses);
8686
List<EquivalentAddressGroup> addrList = resolvedAddresses.getAddresses();
8787
if (addrList.isEmpty()) {
8888
handleNameResolutionError(Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS "
8989
+ "resolution was successful, but returned server addresses are empty."));
90-
return;
90+
return false;
9191
}
9292
Map<EquivalentAddressGroup, EquivalentAddressGroup> latestAddrs = stripAttrs(addrList);
9393
Set<EquivalentAddressGroup> removedAddrs =
@@ -162,6 +162,8 @@ public void onSubchannelState(ConnectivityStateInfo newState) {
162162
for (Subchannel subchann : removedSubchannels) {
163163
shutdownSubchannel(subchann);
164164
}
165+
166+
return true;
165167
}
166168

167169
private static List<RingEntry> buildRing(

‎xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,10 @@ public void tearDown() {
156156
public void subchannelLazyConnectUntilPicked() {
157157
RingHashConfig config = new RingHashConfig(10, 100);
158158
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1); // one server
159-
loadBalancer.handleResolvedAddresses(
159+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
160160
ResolvedAddresses.newBuilder()
161161
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
162+
assertThat(addressesAccepted).isTrue();
162163
verify(helper).createSubchannel(any(CreateSubchannelArgs.class));
163164
Subchannel subchannel = Iterables.getOnlyElement(subchannels.values());
164165
verify(subchannel, never()).requestConnection();
@@ -187,9 +188,10 @@ public void subchannelLazyConnectUntilPicked() {
187188
public void subchannelNotAutoReconnectAfterReenteringIdle() {
188189
RingHashConfig config = new RingHashConfig(10, 100);
189190
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1); // one server
190-
loadBalancer.handleResolvedAddresses(
191+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
191192
ResolvedAddresses.newBuilder()
192193
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
194+
assertThat(addressesAccepted).isTrue();
193195
Subchannel subchannel = Iterables.getOnlyElement(subchannels.values());
194196
InOrder inOrder = Mockito.inOrder(helper, subchannel);
195197
inOrder.verify(helper).updateBalancingState(eq(IDLE), pickerCaptor.capture());
@@ -217,9 +219,10 @@ public void aggregateSubchannelStates_connectingReadyIdleFailure() {
217219
RingHashConfig config = new RingHashConfig(10, 100);
218220
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1);
219221
InOrder inOrder = Mockito.inOrder(helper);
220-
loadBalancer.handleResolvedAddresses(
222+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
221223
ResolvedAddresses.newBuilder()
222224
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
225+
assertThat(addressesAccepted).isTrue();
223226
inOrder.verify(helper, times(2)).createSubchannel(any(CreateSubchannelArgs.class));
224227
inOrder.verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
225228

@@ -278,9 +281,10 @@ public void aggregateSubchannelStates_twoOrMoreSubchannelsInTransientFailure() {
278281
RingHashConfig config = new RingHashConfig(10, 100);
279282
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1, 1);
280283
InOrder inOrder = Mockito.inOrder(helper);
281-
loadBalancer.handleResolvedAddresses(
284+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
282285
ResolvedAddresses.newBuilder()
283286
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
287+
assertThat(addressesAccepted).isTrue();
284288
inOrder.verify(helper, times(4)).createSubchannel(any(CreateSubchannelArgs.class));
285289
inOrder.verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
286290

@@ -336,9 +340,10 @@ public void aggregateSubchannelStates_twoOrMoreSubchannelsInTransientFailure() {
336340
public void subchannelStayInTransientFailureUntilBecomeReady() {
337341
RingHashConfig config = new RingHashConfig(10, 100);
338342
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
339-
loadBalancer.handleResolvedAddresses(
343+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
340344
ResolvedAddresses.newBuilder()
341345
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
346+
assertThat(addressesAccepted).isTrue();
342347
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
343348
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
344349
reset(helper);
@@ -378,9 +383,10 @@ public void updateConnectionIterator() {
378383
RingHashConfig config = new RingHashConfig(10, 100);
379384
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
380385
InOrder inOrder = Mockito.inOrder(helper);
381-
loadBalancer.handleResolvedAddresses(
386+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
382387
ResolvedAddresses.newBuilder()
383388
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
389+
assertThat(addressesAccepted).isTrue();
384390
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
385391
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
386392

@@ -394,9 +400,10 @@ public void updateConnectionIterator() {
394400
verifyConnection(1);
395401

396402
servers = createWeightedServerAddrs(1,1);
397-
loadBalancer.handleResolvedAddresses(
403+
addressesAccepted = loadBalancer.acceptResolvedAddresses(
398404
ResolvedAddresses.newBuilder()
399405
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
406+
assertThat(addressesAccepted).isTrue();
400407
inOrder.verify(helper)
401408
.updateBalancingState(eq(CONNECTING), any(SubchannelPicker.class));
402409
verifyConnection(1);
@@ -422,9 +429,10 @@ public void updateConnectionIterator() {
422429
public void ignoreShutdownSubchannelStateChange() {
423430
RingHashConfig config = new RingHashConfig(10, 100);
424431
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
425-
loadBalancer.handleResolvedAddresses(
432+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
426433
ResolvedAddresses.newBuilder()
427434
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
435+
assertThat(addressesAccepted).isTrue();
428436
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
429437
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
430438

@@ -442,9 +450,10 @@ public void ignoreShutdownSubchannelStateChange() {
442450
public void deterministicPickWithHostsPartiallyRemoved() {
443451
RingHashConfig config = new RingHashConfig(10, 100);
444452
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1, 1, 1);
445-
loadBalancer.handleResolvedAddresses(
453+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
446454
ResolvedAddresses.newBuilder()
447455
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
456+
assertThat(addressesAccepted).isTrue();
448457
InOrder inOrder = Mockito.inOrder(helper);
449458
inOrder.verify(helper, times(5)).createSubchannel(any(CreateSubchannelArgs.class));
450459
inOrder.verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
@@ -470,9 +479,10 @@ public void deterministicPickWithHostsPartiallyRemoved() {
470479
Attributes attr = addr.getAttributes().toBuilder().set(CUSTOM_KEY, "custom value").build();
471480
updatedServers.add(new EquivalentAddressGroup(addr.getAddresses(), attr));
472481
}
473-
loadBalancer.handleResolvedAddresses(
482+
addressesAccepted = loadBalancer.acceptResolvedAddresses(
474483
ResolvedAddresses.newBuilder()
475484
.setAddresses(updatedServers).setLoadBalancingPolicyConfig(config).build());
485+
assertThat(addressesAccepted).isTrue();
476486
verify(subchannels.get(Collections.singletonList(servers.get(0))))
477487
.updateAddresses(Collections.singletonList(updatedServers.get(0)));
478488
verify(subchannels.get(Collections.singletonList(servers.get(1))))
@@ -487,9 +497,10 @@ public void deterministicPickWithHostsPartiallyRemoved() {
487497
public void deterministicPickWithNewHostsAdded() {
488498
RingHashConfig config = new RingHashConfig(10, 100);
489499
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1); // server0 and server1
490-
loadBalancer.handleResolvedAddresses(
500+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
491501
ResolvedAddresses.newBuilder()
492502
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
503+
assertThat(addressesAccepted).isTrue();
493504
InOrder inOrder = Mockito.inOrder(helper);
494505
inOrder.verify(helper, times(2)).createSubchannel(any(CreateSubchannelArgs.class));
495506
inOrder.verify(helper).updateBalancingState(eq(IDLE), pickerCaptor.capture());
@@ -511,9 +522,10 @@ public void deterministicPickWithNewHostsAdded() {
511522
assertThat(subchannel.getAddresses()).isEqualTo(servers.get(1));
512523

513524
servers = createWeightedServerAddrs(1, 1, 1, 1, 1); // server2, server3, server4 added
514-
loadBalancer.handleResolvedAddresses(
525+
addressesAccepted = loadBalancer.acceptResolvedAddresses(
515526
ResolvedAddresses.newBuilder()
516527
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
528+
assertThat(addressesAccepted).isTrue();
517529
inOrder.verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
518530
inOrder.verify(helper).updateBalancingState(eq(READY), pickerCaptor.capture());
519531
assertThat(pickerCaptor.getValue().pickSubchannel(args).getSubchannel())
@@ -526,9 +538,10 @@ public void skipFailingHosts_pickNextNonFailingHostInFirstTwoHosts() {
526538
// Map each server address to exactly one ring entry.
527539
RingHashConfig config = new RingHashConfig(3, 3);
528540
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
529-
loadBalancer.handleResolvedAddresses(
541+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
530542
ResolvedAddresses.newBuilder()
531543
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
544+
assertThat(addressesAccepted).isTrue();
532545
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
533546
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class)); // initial IDLE
534547
reset(helper);
@@ -583,9 +596,10 @@ public void skipFailingHosts_firstTwoHostsFailed_pickNextFirstReady() {
583596
// Map each server address to exactly one ring entry.
584597
RingHashConfig config = new RingHashConfig(3, 3);
585598
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
586-
loadBalancer.handleResolvedAddresses(
599+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
587600
ResolvedAddresses.newBuilder()
588601
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
602+
assertThat(addressesAccepted).isTrue();
589603
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
590604
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class)); // initial IDLE
591605
reset(helper);
@@ -649,9 +663,10 @@ public void allSubchannelsInTransientFailure() {
649663
// Map each server address to exactly one ring entry.
650664
RingHashConfig config = new RingHashConfig(3, 3);
651665
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
652-
loadBalancer.handleResolvedAddresses(
666+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
653667
ResolvedAddresses.newBuilder()
654668
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
669+
assertThat(addressesAccepted).isTrue();
655670
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
656671
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
657672

@@ -687,9 +702,10 @@ public void firstSubchannelIdle() {
687702
// Map each server address to exactly one ring entry.
688703
RingHashConfig config = new RingHashConfig(3, 3);
689704
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
690-
loadBalancer.handleResolvedAddresses(
705+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
691706
ResolvedAddresses.newBuilder()
692707
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
708+
assertThat(addressesAccepted).isTrue();
693709
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
694710
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
695711

@@ -718,9 +734,10 @@ public void firstSubchannelConnecting() {
718734
// Map each server address to exactly one ring entry.
719735
RingHashConfig config = new RingHashConfig(3, 3);
720736
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
721-
loadBalancer.handleResolvedAddresses(
737+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
722738
ResolvedAddresses.newBuilder()
723739
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
740+
assertThat(addressesAccepted).isTrue();
724741
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
725742
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
726743

@@ -749,9 +766,10 @@ public void firstSubchannelFailure() {
749766
// Map each server address to exactly one ring entry.
750767
RingHashConfig config = new RingHashConfig(3, 3);
751768
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
752-
loadBalancer.handleResolvedAddresses(
769+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
753770
ResolvedAddresses.newBuilder()
754771
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
772+
assertThat(addressesAccepted).isTrue();
755773
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
756774
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
757775
// ring:
@@ -784,9 +802,10 @@ public void secondSubchannelConnecting() {
784802
// Map each server address to exactly one ring entry.
785803
RingHashConfig config = new RingHashConfig(3, 3);
786804
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
787-
loadBalancer.handleResolvedAddresses(
805+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
788806
ResolvedAddresses.newBuilder()
789807
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
808+
assertThat(addressesAccepted).isTrue();
790809
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
791810
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
792811
// ring:
@@ -822,9 +841,10 @@ public void secondSubchannelFailure() {
822841
// Map each server address to exactly one ring entry.
823842
RingHashConfig config = new RingHashConfig(3, 3);
824843
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
825-
loadBalancer.handleResolvedAddresses(
844+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
826845
ResolvedAddresses.newBuilder()
827846
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
847+
assertThat(addressesAccepted).isTrue();
828848
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
829849
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
830850
// ring:
@@ -864,9 +884,10 @@ public void thirdSubchannelConnecting() {
864884
// Map each server address to exactly one ring entry.
865885
RingHashConfig config = new RingHashConfig(3, 3);
866886
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
867-
loadBalancer.handleResolvedAddresses(
887+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
868888
ResolvedAddresses.newBuilder()
869889
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
890+
assertThat(addressesAccepted).isTrue();
870891
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
871892
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
872893
// ring:
@@ -908,9 +929,10 @@ public void stickyTransientFailure() {
908929
// Map each server address to exactly one ring entry.
909930
RingHashConfig config = new RingHashConfig(3, 3);
910931
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 1, 1);
911-
loadBalancer.handleResolvedAddresses(
932+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
912933
ResolvedAddresses.newBuilder()
913934
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
935+
assertThat(addressesAccepted).isTrue();
914936
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
915937
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
916938

@@ -943,9 +965,10 @@ public void stickyTransientFailure() {
943965
public void hostSelectionProportionalToWeights() {
944966
RingHashConfig config = new RingHashConfig(10000, 100000); // large ring
945967
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1, 10, 100); // 1:10:100
946-
loadBalancer.handleResolvedAddresses(
968+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
947969
ResolvedAddresses.newBuilder()
948970
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
971+
assertThat(addressesAccepted).isTrue();
949972
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
950973
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
951974

@@ -979,9 +1002,10 @@ public void hostSelectionProportionalToWeights() {
9791002
public void hostSelectionProportionalToRepeatedAddressCount() {
9801003
RingHashConfig config = new RingHashConfig(10000, 100000);
9811004
List<EquivalentAddressGroup> servers = createRepeatedServerAddrs(1, 10, 100); // 1:10:100
982-
loadBalancer.handleResolvedAddresses(
1005+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
9831006
ResolvedAddresses.newBuilder()
9841007
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
1008+
assertThat(addressesAccepted).isTrue();
9851009
verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class));
9861010
verify(helper).updateBalancingState(eq(IDLE), any(SubchannelPicker.class));
9871011

@@ -1027,9 +1051,10 @@ public void nameResolutionErrorWithNoActiveSubchannels() {
10271051
public void nameResolutionErrorWithActiveSubchannels() {
10281052
RingHashConfig config = new RingHashConfig(10, 100);
10291053
List<EquivalentAddressGroup> servers = createWeightedServerAddrs(1);
1030-
loadBalancer.handleResolvedAddresses(
1054+
boolean addressesAccepted = loadBalancer.acceptResolvedAddresses(
10311055
ResolvedAddresses.newBuilder()
10321056
.setAddresses(servers).setLoadBalancingPolicyConfig(config).build());
1057+
assertThat(addressesAccepted).isTrue();
10331058
verify(helper).createSubchannel(any(CreateSubchannelArgs.class));
10341059
verify(helper).updateBalancingState(eq(IDLE), pickerCaptor.capture());
10351060

0 commit comments

Comments
 (0)