Skip to content

Commit e567b44

Browse files
ejona86ejona86
authored
core: Don't reuse channels in PickFirstLeafLB test
PickFirstLeafLB uses channel reference equality to see if it has re-created subchannels. The tests reusing channels breaks the expected invariant.
1 parent c29763d commit e567b44

File tree

1 file changed

+100
-66
lines changed

1 file changed

+100
-66
lines changed

‎core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java‎

Lines changed: 100 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,14 @@ public void uncaughtException(Thread t, Throwable e) {
123123
private ArgumentCaptor<CreateSubchannelArgs> createArgsCaptor;
124124
@Captor
125125
private ArgumentCaptor<SubchannelStateListener> stateListenerCaptor;
126-
private final Helper mockHelper = mock(Helper.class, delegatesTo(new MockHelperImpl()));
127-
@Mock
126+
private Helper mockHelper;
128127
private FakeSubchannel mockSubchannel1;
129-
@Mock
128+
private FakeSubchannel mockSubchannel1n2;
130129
private FakeSubchannel mockSubchannel2;
131-
@Mock
130+
private FakeSubchannel mockSubchannel2n2;
132131
private FakeSubchannel mockSubchannel3;
133-
@Mock
132+
private FakeSubchannel mockSubchannel3n2;
134133
private FakeSubchannel mockSubchannel4;
135-
@Mock
136134
private FakeSubchannel mockSubchannel5;
137135
@Mock // This LoadBalancer doesn't use any of the arg fields, as verified in tearDown().
138136
private PickSubchannelArgs mockArgs;
@@ -150,23 +148,28 @@ public void setUp() {
150148
SocketAddress addr = new FakeSocketAddress("server" + i);
151149
servers.add(new EquivalentAddressGroup(addr));
152150
}
153-
mockSubchannel1 = mock(FakeSubchannel.class);
154-
mockSubchannel2 = mock(FakeSubchannel.class);
155-
mockSubchannel3 = mock(FakeSubchannel.class);
156-
mockSubchannel4 = mock(FakeSubchannel.class);
157-
mockSubchannel5 = mock(FakeSubchannel.class);
158-
when(mockSubchannel1.getAttributes()).thenReturn(Attributes.EMPTY);
159-
when(mockSubchannel2.getAttributes()).thenReturn(Attributes.EMPTY);
160-
when(mockSubchannel3.getAttributes()).thenReturn(Attributes.EMPTY);
161-
when(mockSubchannel4.getAttributes()).thenReturn(Attributes.EMPTY);
162-
when(mockSubchannel5.getAttributes()).thenReturn(Attributes.EMPTY);
163-
164-
when(mockSubchannel1.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(0)));
165-
when(mockSubchannel2.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(1)));
166-
when(mockSubchannel3.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(2)));
167-
when(mockSubchannel4.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(3)));
168-
when(mockSubchannel5.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(4)));
169-
151+
mockSubchannel1 = mock(FakeSubchannel.class, delegatesTo(
152+
new FakeSubchannel(Arrays.asList(servers.get(0)), Attributes.EMPTY)));
153+
mockSubchannel1n2 = mock(FakeSubchannel.class, delegatesTo(
154+
new FakeSubchannel(Arrays.asList(servers.get(0)), Attributes.EMPTY)));
155+
mockSubchannel2 = mock(FakeSubchannel.class, delegatesTo(
156+
new FakeSubchannel(Arrays.asList(servers.get(1)), Attributes.EMPTY)));
157+
mockSubchannel2n2 = mock(FakeSubchannel.class, delegatesTo(
158+
new FakeSubchannel(Arrays.asList(servers.get(1)), Attributes.EMPTY)));
159+
mockSubchannel3 = mock(FakeSubchannel.class, delegatesTo(
160+
new FakeSubchannel(Arrays.asList(servers.get(2)), Attributes.EMPTY)));
161+
mockSubchannel3n2 = mock(FakeSubchannel.class, delegatesTo(
162+
new FakeSubchannel(Arrays.asList(servers.get(2)), Attributes.EMPTY)));
163+
mockSubchannel4 = mock(FakeSubchannel.class, delegatesTo(
164+
new FakeSubchannel(Arrays.asList(servers.get(3)), Attributes.EMPTY)));
165+
mockSubchannel5 = mock(FakeSubchannel.class, delegatesTo(
166+
new FakeSubchannel(Arrays.asList(servers.get(4)), Attributes.EMPTY)));
167+
168+
mockHelper = mock(Helper.class, delegatesTo(new MockHelperImpl(Arrays.asList(
169+
mockSubchannel1, mockSubchannel1n2,
170+
mockSubchannel2, mockSubchannel2n2,
171+
mockSubchannel3, mockSubchannel3n2,
172+
mockSubchannel4, mockSubchannel5))));
170173
loadBalancer = new PickFirstLeafLoadBalancer(mockHelper);
171174
}
172175

@@ -251,14 +254,14 @@ public void pickAfterResolved_shuffle() {
251254
PickResult pick2 = pickerCaptor.getValue().pickSubchannel(mockArgs);
252255
assertEquals(pick1, pick2);
253256
verifyNoMoreInteractions(mockHelper);
254-
assertThat(pick1.toString()).contains("subchannel=null");
257+
assertThat(pick1.getSubchannel()).isNull();
255258

256259
stateListener2.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
257260
verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
258261
PickResult pick3 = pickerCaptor.getValue().pickSubchannel(mockArgs);
259262
PickResult pick4 = pickerCaptor.getValue().pickSubchannel(mockArgs);
260263
assertEquals(pick3, pick4);
261-
assertThat(pick3.toString()).contains("subchannel=Mock");
264+
assertThat(pick3.getSubchannel()).isEqualTo(mockSubchannel2);
262265
}
263266

264267
@Test
@@ -569,7 +572,7 @@ public void pickWithDupAddressesUpDownUp() {
569572
InOrder inOrder = inOrder(mockHelper);
570573
SocketAddress socketAddress = servers.get(0).getAddresses().get(0);
571574
EquivalentAddressGroup badEag = new EquivalentAddressGroup(
572-
Lists.newArrayList(socketAddress, socketAddress), affinity);
575+
Lists.newArrayList(socketAddress, socketAddress));
573576
List<EquivalentAddressGroup> newServers = Lists.newArrayList(badEag);
574577

575578
loadBalancer.acceptResolvedAddresses(
@@ -727,7 +730,7 @@ public void nameResolutionSuccessAfterError() {
727730
@Test
728731
public void nameResolutionTemporaryError() {
729732
List<EquivalentAddressGroup> newServers = Lists.newArrayList(servers.get(0));
730-
InOrder inOrder = inOrder(mockHelper, mockSubchannel1);
733+
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel1n2);
731734
loadBalancer.acceptResolvedAddresses(
732735
ResolvedAddresses.newBuilder().setAddresses(newServers).setAttributes(affinity).build());
733736
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
@@ -744,14 +747,15 @@ public void nameResolutionTemporaryError() {
744747
loadBalancer.acceptResolvedAddresses(
745748
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build());
746749
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
747-
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
750+
inOrder.verify(mockSubchannel1n2).start(stateListenerCaptor.capture());
748751
SubchannelStateListener stateListener2 = stateListenerCaptor.getValue();
749752

750753
assertNull(pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
751754

752755
stateListener2.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
753756
inOrder.verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
754-
assertEquals(mockSubchannel1, pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
757+
assertEquals(mockSubchannel1n2,
758+
pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel());
755759
}
756760

757761

@@ -1027,7 +1031,7 @@ public void updateAddresses_disjoint_connecting() {
10271031
@Test
10281032
public void updateAddresses_disjoint_ready_twice() {
10291033
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2,
1030-
mockSubchannel3, mockSubchannel4);
1034+
mockSubchannel3, mockSubchannel4, mockSubchannel1n2, mockSubchannel2n2);
10311035
// Creating first set of endpoints/addresses
10321036
List<EquivalentAddressGroup> oldServers = Lists.newArrayList(servers.get(0), servers.get(1));
10331037
SubchannelStateListener stateListener2 = null;
@@ -1126,7 +1130,7 @@ public void updateAddresses_disjoint_ready_twice() {
11261130
ResolvedAddresses.newBuilder().setAddresses(newestServers).setAttributes(affinity).build());
11271131
inOrder.verify(mockSubchannel3).shutdown();
11281132
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
1129-
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
1133+
inOrder.verify(mockSubchannel1n2).start(stateListenerCaptor.capture());
11301134
stateListener = stateListenerCaptor.getValue();
11311135
assertEquals(CONNECTING, loadBalancer.getConcludedConnectivityState());
11321136
picker = pickerCaptor.getValue();
@@ -1135,7 +1139,7 @@ public void updateAddresses_disjoint_ready_twice() {
11351139
assertEquals(picker.pickSubchannel(mockArgs), picker.pickSubchannel(mockArgs));
11361140

11371141
// But the picker calls requestConnection() only once
1138-
inOrder.verify(mockSubchannel1).requestConnection();
1142+
inOrder.verify(mockSubchannel1n2).requestConnection();
11391143
assertEquals(PickResult.withNoResult(), pickerCaptor.getValue().pickSubchannel(mockArgs));
11401144
assertEquals(CONNECTING, loadBalancer.getConcludedConnectivityState());
11411145

@@ -1150,23 +1154,24 @@ public void updateAddresses_disjoint_ready_twice() {
11501154
stateListener.onSubchannelState(ConnectivityStateInfo.forTransientFailure(CONNECTION_ERROR));
11511155

11521156
// Starting connection attempt to address 2
1153-
if (!enableHappyEyeballs) {
1154-
inOrder.verify(mockSubchannel2).start(stateListenerCaptor.capture());
1155-
stateListener2 = stateListenerCaptor.getValue();
1156-
}
1157-
inOrder.verify(mockSubchannel2).requestConnection();
1157+
FakeSubchannel mockSubchannel2Attempt =
1158+
enableHappyEyeballs ? mockSubchannel2n2 : mockSubchannel2;
1159+
inOrder.verify(mockSubchannel2Attempt).start(stateListenerCaptor.capture());
1160+
stateListener2 = stateListenerCaptor.getValue();
1161+
inOrder.verify(mockSubchannel2Attempt).requestConnection();
11581162

11591163
// Connection attempt to address 2 is successful
11601164
stateListener2.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
11611165
assertEquals(READY, loadBalancer.getConcludedConnectivityState());
1162-
inOrder.verify(mockSubchannel1).shutdown();
1166+
inOrder.verify(mockSubchannel1n2).shutdown();
11631167

11641168
// Successful connection shuts down other subchannel
11651169
inOrder.verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
11661170
picker = pickerCaptor.getValue();
11671171

11681172
// Verify that picker still returns correct subchannel
1169-
assertEquals(PickResult.withSubchannel(mockSubchannel2), picker.pickSubchannel(mockArgs));
1173+
assertEquals(
1174+
PickResult.withSubchannel(mockSubchannel2Attempt), picker.pickSubchannel(mockArgs));
11701175
}
11711176

11721177
@Test
@@ -2048,7 +2053,7 @@ public void recreate_shutdown_subchannel() {
20482053

20492054
// Starting first connection attempt
20502055
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2,
2051-
mockSubchannel3, mockSubchannel4); // captor: captures
2056+
mockSubchannel3, mockSubchannel4, mockSubchannel1n2); // captor: captures
20522057

20532058
// Creating first set of endpoints/addresses
20542059
List<EquivalentAddressGroup> addrs =
@@ -2084,9 +2089,9 @@ public void recreate_shutdown_subchannel() {
20842089

20852090
// Calling pickSubchannel() requests a connection.
20862091
assertEquals(picker.pickSubchannel(mockArgs), picker.pickSubchannel(mockArgs));
2087-
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
2092+
inOrder.verify(mockSubchannel1n2).start(stateListenerCaptor.capture());
20882093
SubchannelStateListener stateListener3 = stateListenerCaptor.getValue();
2089-
inOrder.verify(mockSubchannel1).requestConnection();
2094+
inOrder.verify(mockSubchannel1n2).requestConnection();
20902095
when(mockSubchannel1.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(0)));
20912096

20922097
// gives the same result when called twice
@@ -2101,7 +2106,7 @@ public void recreate_shutdown_subchannel() {
21012106
// second subchannel connection attempt succeeds
21022107
inOrder.verify(mockSubchannel2).requestConnection();
21032108
stateListener2.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
2104-
inOrder.verify(mockSubchannel1).shutdown();
2109+
inOrder.verify(mockSubchannel1n2).shutdown();
21052110
inOrder.verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
21062111
assertEquals(READY, loadBalancer.getConcludedConnectivityState());
21072112

@@ -2146,7 +2151,7 @@ public void shutdown() {
21462151
public void ready_then_transient_failure_again() {
21472152
// Starting first connection attempt
21482153
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2,
2149-
mockSubchannel3, mockSubchannel4); // captor: captures
2154+
mockSubchannel3, mockSubchannel4, mockSubchannel1n2); // captor: captures
21502155

21512156
// Creating first set of endpoints/addresses
21522157
List<EquivalentAddressGroup> addrs =
@@ -2183,9 +2188,9 @@ public void ready_then_transient_failure_again() {
21832188

21842189
// Calling pickSubchannel() requests a connection, gives the same result when called twice.
21852190
assertEquals(picker.pickSubchannel(mockArgs), picker.pickSubchannel(mockArgs));
2186-
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
2191+
inOrder.verify(mockSubchannel1n2).start(stateListenerCaptor.capture());
21872192
SubchannelStateListener stateListener3 = stateListenerCaptor.getValue();
2188-
inOrder.verify(mockSubchannel1).requestConnection();
2193+
inOrder.verify(mockSubchannel1n2).requestConnection();
21892194
when(mockSubchannel3.getAllAddresses()).thenReturn(Lists.newArrayList(servers.get(0)));
21902195
stateListener3.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
21912196
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture());
@@ -2201,7 +2206,7 @@ public void ready_then_transient_failure_again() {
22012206
assertEquals(READY, loadBalancer.getConcludedConnectivityState());
22022207

22032208
// verify that picker returns correct subchannel
2204-
inOrder.verify(mockSubchannel1).shutdown();
2209+
inOrder.verify(mockSubchannel1n2).shutdown();
22052210
inOrder.verify(mockHelper).updateBalancingState(eq(READY), pickerCaptor.capture());
22062211
picker = pickerCaptor.getValue();
22072212
assertEquals(PickResult.withSubchannel(mockSubchannel2), picker.pickSubchannel(mockArgs));
@@ -2309,7 +2314,8 @@ public void happy_eyeballs_connection_results_happen_after_get_to_end() {
23092314
public void happy_eyeballs_pick_pushes_index_over_end() {
23102315
Assume.assumeTrue(enableHappyEyeballs); // This test is only for happy eyeballs
23112316

2312-
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2, mockSubchannel3);
2317+
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2, mockSubchannel3,
2318+
mockSubchannel2n2, mockSubchannel3n2);
23132319
Status error = Status.UNAUTHENTICATED.withDescription("simulated failure");
23142320

23152321
List<EquivalentAddressGroup> addrs =
@@ -2359,9 +2365,9 @@ public void happy_eyeballs_pick_pushes_index_over_end() {
23592365

23602366
// Try pushing after end with just picks
23612367
listeners[0].onSubchannelState(ConnectivityStateInfo.forNonError(READY));
2362-
for (SubchannelStateListener listener : listeners) {
2363-
listener.onSubchannelState(ConnectivityStateInfo.forNonError(IDLE));
2364-
}
2368+
verify(mockSubchannel2).shutdown();
2369+
verify(mockSubchannel3).shutdown();
2370+
listeners[0].onSubchannelState(ConnectivityStateInfo.forNonError(IDLE));
23652371
loadBalancer.acceptResolvedAddresses(
23662372
ResolvedAddresses.newBuilder().setAddresses(addrs).setAttributes(affinity).build());
23672373
inOrder.verify(mockHelper).updateBalancingState(eq(IDLE), pickerCaptor.capture());
@@ -2372,11 +2378,14 @@ public void happy_eyeballs_pick_pushes_index_over_end() {
23722378
}
23732379
assertEquals(IDLE, loadBalancer.getConcludedConnectivityState());
23742380

2375-
for (SubchannelStateListener listener : listeners) {
2376-
listener.onSubchannelState(ConnectivityStateInfo.forTransientFailure(error));
2377-
}
2381+
listeners[0].onSubchannelState(ConnectivityStateInfo.forTransientFailure(error));
2382+
inOrder.verify(mockSubchannel2n2).start(stateListenerCaptor.capture());
2383+
stateListenerCaptor.getValue().onSubchannelState(
2384+
ConnectivityStateInfo.forTransientFailure(error));
2385+
inOrder.verify(mockSubchannel3n2).start(stateListenerCaptor.capture());
2386+
stateListenerCaptor.getValue().onSubchannelState(
2387+
ConnectivityStateInfo.forTransientFailure(error));
23782388
assertEquals(TRANSIENT_FAILURE, loadBalancer.getConcludedConnectivityState());
2379-
23802389
}
23812390

23822391
@Test
@@ -2571,9 +2580,22 @@ private static class FakeSocketAddress extends SocketAddress {
25712580

25722581
@Override
25732582
public String toString() {
2574-
return "FakeSocketAddress-" + name;
2583+
return "FakeSocketAddress(" + name + ")";
2584+
}
2585+
2586+
@Override
2587+
public boolean equals(Object o) {
2588+
if (!(o instanceof FakeSocketAddress)) {
2589+
return false;
2590+
}
2591+
FakeSocketAddress that = (FakeSocketAddress) o;
2592+
return this.name.equals(that.name);
25752593
}
25762594

2595+
@Override
2596+
public int hashCode() {
2597+
return name.hashCode();
2598+
}
25772599
}
25782600

25792601
private void forwardTimeByConnectionDelay() {
@@ -2631,15 +2653,26 @@ public void updateAddresses(List<EquivalentAddressGroup> addrs) {
26312653

26322654
@Override
26332655
public void shutdown() {
2656+
listener.onSubchannelState(ConnectivityStateInfo.forNonError(SHUTDOWN));
26342657
}
26352658

26362659
@Override
26372660
public void requestConnection() {
2638-
listener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
2661+
}
2662+
2663+
@Override
2664+
public String toString() {
2665+
return "FakeSubchannel@" + hashCode() + "(" + eags + ")";
26392666
}
26402667
}
26412668

26422669
private class MockHelperImpl extends LoadBalancer.Helper {
2670+
private final List<Subchannel> subchannels;
2671+
2672+
public MockHelperImpl(List<? extends Subchannel> subchannels) {
2673+
this.subchannels = new ArrayList<Subchannel>(subchannels);
2674+
}
2675+
26432676
@Override
26442677
public ManagedChannel createOobChannel(EquivalentAddressGroup eag, String authority) {
26452678
return null;
@@ -2672,16 +2705,17 @@ public void refreshNameResolution() {
26722705

26732706
@Override
26742707
public Subchannel createSubchannel(CreateSubchannelArgs args) {
2675-
SocketAddress addr = args.getAddresses().get(0).getAddresses().get(0);
2676-
List<FakeSubchannel> fakeSubchannels =
2677-
Arrays.asList(mockSubchannel1, mockSubchannel2, mockSubchannel3, mockSubchannel4,
2678-
mockSubchannel5);
2679-
for (int i = 1; i <= 5; i++) {
2680-
if (addr.toString().equals(new FakeSocketAddress("server" + i).toString())) {
2681-
return fakeSubchannels.get(i - 1);
2708+
for (int i = 0; i < subchannels.size(); i++) {
2709+
Subchannel subchannel = subchannels.get(i);
2710+
List<EquivalentAddressGroup> addrs = subchannel.getAllAddresses();
2711+
verify(subchannel, atLeast(1)).getAllAddresses(); // ignore the interaction
2712+
if (!args.getAddresses().equals(addrs)) {
2713+
continue;
26822714
}
2715+
subchannels.remove(i);
2716+
return subchannel;
26832717
}
2684-
throw new IllegalArgumentException("Unexpected address: " + addr);
2718+
throw new IllegalArgumentException("Unexpected addresses: " + args.getAddresses());
26852719
}
26862720
}
2687-
}
2721+
}

0 commit comments

Comments
 (0)