Skip to content

Commit 6a9c990

Browse files
YifeiZhuangYifeiZhuang
authored
grpclb: support multiple authorities in lb backends for all SRV records (#7951)
1 parent 972fda2 commit 6a9c990

File tree

11 files changed

+192
-188
lines changed

11 files changed

+192
-188
lines changed

‎api/src/main/java/io/grpc/LoadBalancer.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -930,11 +930,16 @@ public Subchannel createSubchannel(CreateSubchannelArgs args) {
930930
*
931931
* @since 1.4.0
932932
*/
933-
// TODO(ejona): Allow passing a List<EAG> here and to updateOobChannelAddresses, but want to
934-
// wait until https://github.com/grpc/grpc-java/issues/4469 is done.
935-
// https://github.com/grpc/grpc-java/issues/4618
936933
public abstract ManagedChannel createOobChannel(EquivalentAddressGroup eag, String authority);
937934

935+
/**
936+
* Accept a list of EAG for multiple authorities: https://github.com/grpc/grpc-java/issues/4618
937+
* */
938+
public ManagedChannel createOobChannel(List<EquivalentAddressGroup> eag,
939+
String authority) {
940+
throw new UnsupportedOperationException();
941+
}
942+
938943
/**
939944
* Updates the addresses used for connections in the {@code Channel} that was created by {@link
940945
* #createOobChannel(EquivalentAddressGroup, String)}. This is superior to {@link
@@ -949,6 +954,15 @@ public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressG
949954
throw new UnsupportedOperationException();
950955
}
951956

957+
/**
958+
* Updates the addresses with a new EAG list. Connection is continued when old and new addresses
959+
* overlap.
960+
* */
961+
public void updateOobChannelAddresses(ManagedChannel channel,
962+
List<EquivalentAddressGroup> eag) {
963+
throw new UnsupportedOperationException();
964+
}
965+
952966
/**
953967
* Creates an out-of-band channel for LoadBalancer's own RPC needs, e.g., talking to an external
954968
* load-balancer service, that is specified by a target string. See the documentation on

‎core/src/main/java/io/grpc/internal/InternalSubchannel.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,12 @@ public void updateAddresses(final List<EquivalentAddressGroup> newAddressGroups)
339339
Preconditions.checkNotNull(newAddressGroups, "newAddressGroups");
340340
checkListHasNoNulls(newAddressGroups, "newAddressGroups contains null entry");
341341
Preconditions.checkArgument(!newAddressGroups.isEmpty(), "newAddressGroups is empty");
342+
final List<EquivalentAddressGroup> newImmutableAddressGroups =
343+
Collections.unmodifiableList(new ArrayList<>(newAddressGroups));
342344

343345
syncContext.execute(new Runnable() {
344346
@Override
345347
public void run() {
346-
List<EquivalentAddressGroup> newImmutableAddressGroups =
347-
Collections.unmodifiableList(new ArrayList<>(newAddressGroups));
348348
ManagedClientTransport savedTransport = null;
349349
SocketAddress previousAddress = addressIndex.getCurrentAddress();
350350
addressIndex.updateGroups(newImmutableAddressGroups);

‎core/src/main/java/io/grpc/internal/ManagedChannelImpl.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1465,6 +1465,12 @@ public void run() {
14651465

14661466
@Override
14671467
public ManagedChannel createOobChannel(EquivalentAddressGroup addressGroup, String authority) {
1468+
return createOobChannel(Collections.singletonList(addressGroup), authority);
1469+
}
1470+
1471+
@Override
1472+
public ManagedChannel createOobChannel(List<EquivalentAddressGroup> addressGroup,
1473+
String authority) {
14681474
// TODO(ejona): can we be even stricter? Like terminating?
14691475
checkState(!terminated, "Channel is terminated");
14701476
long oobChannelCreationTime = timeProvider.currentTimeNanos();
@@ -1505,7 +1511,7 @@ void onStateChange(InternalSubchannel is, ConnectivityStateInfo newState) {
15051511
}
15061512

15071513
final InternalSubchannel internalSubchannel = new InternalSubchannel(
1508-
Collections.singletonList(addressGroup),
1514+
addressGroup,
15091515
authority, userAgent, backoffPolicyProvider, oobTransportFactory,
15101516
oobTransportFactory.getScheduledExecutorService(), stopwatchSupplier, syncContext,
15111517
// All callback methods are run from syncContext
@@ -1625,6 +1631,12 @@ public ChannelCredentials getUnsafeChannelCredentials() {
16251631

16261632
@Override
16271633
public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressGroup eag) {
1634+
updateOobChannelAddresses(channel, Collections.singletonList(eag));
1635+
}
1636+
1637+
@Override
1638+
public void updateOobChannelAddresses(ManagedChannel channel,
1639+
List<EquivalentAddressGroup> eag) {
16281640
checkArgument(channel instanceof OobChannel,
16291641
"channel must have been returned from createOobChannel");
16301642
((OobChannel) channel).updateAddresses(eag);

‎core/src/main/java/io/grpc/internal/OobChannel.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,8 @@ public String toString() {
193193
delayedTransport.reprocess(subchannelPicker);
194194
}
195195

196-
void updateAddresses(EquivalentAddressGroup eag) {
197-
subchannel.updateAddresses(Collections.singletonList(eag));
196+
void updateAddresses(List<EquivalentAddressGroup> eag) {
197+
subchannel.updateAddresses(eag);
198198
}
199199

200200
@Override

‎core/src/main/java/io/grpc/util/ForwardingLoadBalancerHelper.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import io.grpc.NameResolver;
3232
import io.grpc.NameResolverRegistry;
3333
import io.grpc.SynchronizationContext;
34+
import java.util.List;
3435
import java.util.concurrent.ScheduledExecutorService;
3536

3637
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1771")
@@ -50,11 +51,21 @@ public ManagedChannel createOobChannel(EquivalentAddressGroup eag, String autho
5051
return delegate().createOobChannel(eag, authority);
5152
}
5253

54+
@Override
55+
public ManagedChannel createOobChannel(List<EquivalentAddressGroup> eag, String authority) {
56+
return delegate().createOobChannel(eag, authority);
57+
}
58+
5359
@Override
5460
public void updateOobChannelAddresses(ManagedChannel channel, EquivalentAddressGroup eag) {
5561
delegate().updateOobChannelAddresses(channel, eag);
5662
}
5763

64+
@Override
65+
public void updateOobChannelAddresses(ManagedChannel channel, List<EquivalentAddressGroup> eag) {
66+
delegate().updateOobChannelAddresses(channel, eag);
67+
}
68+
5869
@Deprecated
5970
@Override
6071
public ManagedChannelBuilder<?> createResolvingOobChannelBuilder(String target) {

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ public void oobTransportDoesNotAffectIdleness() {
402402
assertFalse(channel.inUseStateAggregator.isInUse());
403403

404404
// Now make an RPC on an OOB channel
405-
ManagedChannel oob = helper.createOobChannel(servers.get(0), "oobauthority");
405+
ManagedChannel oob = helper.createOobChannel(servers, "oobauthority");
406406
verify(mockTransportFactory, never())
407407
.newClientTransport(
408408
any(SocketAddress.class),
@@ -438,13 +438,13 @@ public void updateOobChannelAddresses_newAddressConnects() {
438438
verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
439439
deliverResolutionResult();
440440
Helper helper = helperCaptor.getValue();
441-
ManagedChannel oobChannel = helper.createOobChannel(servers.get(0), "localhost");
441+
ManagedChannel oobChannel = helper.createOobChannel(servers.subList(0,1), "localhost");
442442

443443
oobChannel.newCall(method, CallOptions.DEFAULT).start(mockCallListener, new Metadata());
444444
MockClientTransportInfo t0 = newTransports.poll();
445445
t0.listener.transportReady();
446446

447-
helper.updateOobChannelAddresses(oobChannel, servers.get(1));
447+
helper.updateOobChannelAddresses(oobChannel, servers.subList(1,2));
448448

449449
oobChannel.newCall(method, CallOptions.DEFAULT).start(mockCallListener, new Metadata());
450450
MockClientTransportInfo t1 = newTransports.poll();
@@ -462,15 +462,16 @@ public void updateOobChannelAddresses_existingAddressDoesNotConnect() {
462462
verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture());
463463
Helper helper = helperCaptor.getValue();
464464
deliverResolutionResult();
465-
ManagedChannel oobChannel = helper.createOobChannel(servers.get(0), "localhost");
465+
ManagedChannel oobChannel = helper.createOobChannel(servers.subList(0,1), "localhost");
466466

467467
oobChannel.newCall(method, CallOptions.DEFAULT).start(mockCallListener, new Metadata());
468468
MockClientTransportInfo t0 = newTransports.poll();
469469
t0.listener.transportReady();
470470

471471
List<SocketAddress> changedList = new ArrayList<>(servers.get(0).getAddresses());
472472
changedList.add(new FakeSocketAddress("aDifferentServer"));
473-
helper.updateOobChannelAddresses(oobChannel, new EquivalentAddressGroup(changedList));
473+
helper.updateOobChannelAddresses(oobChannel, Collections.singletonList(
474+
new EquivalentAddressGroup(changedList)));
474475

475476
oobChannel.newCall(method, CallOptions.DEFAULT).start(mockCallListener, new Metadata());
476477
assertNull(newTransports.poll());

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

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,8 @@ public void channelzMembership_subchannel() throws Exception {
704704
@Test
705705
public void channelzMembership_oob() throws Exception {
706706
createChannel();
707-
OobChannel oob = (OobChannel) helper.createOobChannel(addressGroup, AUTHORITY);
707+
OobChannel oob = (OobChannel) helper.createOobChannel(
708+
Collections.singletonList(addressGroup), AUTHORITY);
708709
// oob channels are not root channels
709710
assertNull(channelz.getRootChannel(oob.getLogId().getId()));
710711
assertTrue(channelz.containsSubchannel(oob.getLogId()));
@@ -1621,8 +1622,10 @@ public void subchannelsNoConnectionShutdownNow() {
16211622
public void oobchannels() {
16221623
createChannel();
16231624

1624-
ManagedChannel oob1 = helper.createOobChannel(addressGroup, "oob1authority");
1625-
ManagedChannel oob2 = helper.createOobChannel(addressGroup, "oob2authority");
1625+
ManagedChannel oob1 = helper.createOobChannel(
1626+
Collections.singletonList(addressGroup), "oob1authority");
1627+
ManagedChannel oob2 = helper.createOobChannel(
1628+
Collections.singletonList(addressGroup), "oob2authority");
16261629
verify(balancerRpcExecutorPool, times(2)).getObject();
16271630

16281631
assertEquals("oob1authority", oob1.authority());
@@ -1755,7 +1758,8 @@ public void oobChannelHasNoChannelCallCredentials() {
17551758
.containsExactly(channelCredValue, callCredValue).inOrder();
17561759

17571760
// Verify that the oob channel does not
1758-
ManagedChannel oob = helper.createOobChannel(addressGroup, "oobauthority");
1761+
ManagedChannel oob = helper.createOobChannel(
1762+
Collections.singletonList(addressGroup), "oobauthority");
17591763

17601764
headers = new Metadata();
17611765
call = oob.newCall(method, callOptions);
@@ -1886,8 +1890,10 @@ public SwapChannelCredentialsResult answer(InvocationOnMock invocation) {
18861890
@Test
18871891
public void oobChannelsWhenChannelShutdownNow() {
18881892
createChannel();
1889-
ManagedChannel oob1 = helper.createOobChannel(addressGroup, "oob1Authority");
1890-
ManagedChannel oob2 = helper.createOobChannel(addressGroup, "oob2Authority");
1893+
ManagedChannel oob1 = helper.createOobChannel(
1894+
Collections.singletonList(addressGroup), "oob1Authority");
1895+
ManagedChannel oob2 = helper.createOobChannel(
1896+
Collections.singletonList(addressGroup), "oob2Authority");
18911897

18921898
oob1.newCall(method, CallOptions.DEFAULT).start(mockCallListener, new Metadata());
18931899
oob2.newCall(method, CallOptions.DEFAULT).start(mockCallListener2, new Metadata());
@@ -1915,8 +1921,10 @@ public void oobChannelsWhenChannelShutdownNow() {
19151921
@Test
19161922
public void oobChannelsNoConnectionShutdown() {
19171923
createChannel();
1918-
ManagedChannel oob1 = helper.createOobChannel(addressGroup, "oob1Authority");
1919-
ManagedChannel oob2 = helper.createOobChannel(addressGroup, "oob2Authority");
1924+
ManagedChannel oob1 = helper.createOobChannel(
1925+
Collections.singletonList(addressGroup), "oob1Authority");
1926+
ManagedChannel oob2 = helper.createOobChannel(
1927+
Collections.singletonList(addressGroup), "oob2Authority");
19201928
channel.shutdown();
19211929

19221930
verify(mockLoadBalancer).shutdown();
@@ -1934,8 +1942,8 @@ public void oobChannelsNoConnectionShutdown() {
19341942
@Test
19351943
public void oobChannelsNoConnectionShutdownNow() {
19361944
createChannel();
1937-
helper.createOobChannel(addressGroup, "oob1Authority");
1938-
helper.createOobChannel(addressGroup, "oob2Authority");
1945+
helper.createOobChannel(Collections.singletonList(addressGroup), "oob1Authority");
1946+
helper.createOobChannel(Collections.singletonList(addressGroup), "oob2Authority");
19391947
channel.shutdownNow();
19401948

19411949
verify(mockLoadBalancer).shutdown();
@@ -2116,7 +2124,8 @@ private void subtestNameResolutionRefreshWhenConnectionFailed(
21162124
channelBuilder.nameResolverFactory(nameResolverFactory);
21172125
createChannel();
21182126
if (isOobChannel) {
2119-
OobChannel oobChannel = (OobChannel) helper.createOobChannel(addressGroup, "oobAuthority");
2127+
OobChannel oobChannel = (OobChannel) helper.createOobChannel(
2128+
Collections.singletonList(addressGroup), "oobAuthority");
21202129
oobChannel.getSubchannel().requestConnection();
21212130
} else {
21222131
Subchannel subchannel =
@@ -3183,7 +3192,8 @@ public void channelTracing_subchannelStateChangeEvent() throws Exception {
31833192
public void channelTracing_oobChannelStateChangeEvent() throws Exception {
31843193
channelBuilder.maxTraceEvents(10);
31853194
createChannel();
3186-
OobChannel oobChannel = (OobChannel) helper.createOobChannel(addressGroup, "authority");
3195+
OobChannel oobChannel = (OobChannel) helper.createOobChannel(
3196+
Collections.singletonList(addressGroup), "authority");
31873197
timer.forwardNanos(1234);
31883198
oobChannel.handleSubchannelStateChange(
31893199
ConnectivityStateInfo.forNonError(ConnectivityState.CONNECTING));
@@ -3199,21 +3209,22 @@ public void channelTracing_oobChannelCreationEvents() throws Exception {
31993209
channelBuilder.maxTraceEvents(10);
32003210
createChannel();
32013211
timer.forwardNanos(1234);
3202-
OobChannel oobChannel = (OobChannel) helper.createOobChannel(addressGroup, "authority");
3212+
OobChannel oobChannel = (OobChannel) helper.createOobChannel(
3213+
Collections.singletonList(addressGroup), "authority");
32033214
assertThat(getStats(channel).channelTrace.events).contains(new ChannelTrace.Event.Builder()
32043215
.setDescription("Child OobChannel created")
32053216
.setSeverity(ChannelTrace.Event.Severity.CT_INFO)
32063217
.setTimestampNanos(timer.getTicker().read())
32073218
.setChannelRef(oobChannel)
32083219
.build());
32093220
assertThat(getStats(oobChannel).channelTrace.events).contains(new ChannelTrace.Event.Builder()
3210-
.setDescription("OobChannel for [[test-addr]/{}] created")
3221+
.setDescription("OobChannel for [[[test-addr]/{}]] created")
32113222
.setSeverity(ChannelTrace.Event.Severity.CT_INFO)
32123223
.setTimestampNanos(timer.getTicker().read())
32133224
.build());
32143225
assertThat(getStats(oobChannel.getInternalSubchannel()).channelTrace.events).contains(
32153226
new ChannelTrace.Event.Builder()
3216-
.setDescription("Subchannel for [[test-addr]/{}] created")
3227+
.setDescription("Subchannel for [[[test-addr]/{}]] created")
32173228
.setSeverity(ChannelTrace.Event.Severity.CT_INFO)
32183229
.setTimestampNanos(timer.getTicker().read())
32193230
.build());
@@ -3349,7 +3360,8 @@ private void channelsAndSubchannels_oob_instrumented0(boolean success) throws Ex
33493360
ClientStream mockStream = mock(ClientStream.class);
33503361
createChannel();
33513362

3352-
OobChannel oobChannel = (OobChannel) helper.createOobChannel(addressGroup, "oobauthority");
3363+
OobChannel oobChannel = (OobChannel) helper.createOobChannel(
3364+
Collections.singletonList(addressGroup), "oobauthority");
33533365
AbstractSubchannel oobSubchannel = (AbstractSubchannel) oobChannel.getSubchannel();
33543366
FakeClock callExecutor = new FakeClock();
33553367
CallOptions options =
@@ -3411,15 +3423,17 @@ public void channelsAndSubchannels_oob_instrumented_name() throws Exception {
34113423
createChannel();
34123424

34133425
String authority = "oobauthority";
3414-
OobChannel oobChannel = (OobChannel) helper.createOobChannel(addressGroup, authority);
3426+
OobChannel oobChannel = (OobChannel) helper.createOobChannel(
3427+
Collections.singletonList(addressGroup), authority);
34153428
assertEquals(authority, getStats(oobChannel).target);
34163429
}
34173430

34183431
@Test
34193432
public void channelsAndSubchannels_oob_instrumented_state() throws Exception {
34203433
createChannel();
34213434

3422-
OobChannel oobChannel = (OobChannel) helper.createOobChannel(addressGroup, "oobauthority");
3435+
OobChannel oobChannel = (OobChannel) helper.createOobChannel(
3436+
Collections.singletonList(addressGroup), "oobauthority");
34233437
assertEquals(IDLE, getStats(oobChannel).state);
34243438

34253439
oobChannel.getSubchannel().requestConnection();

‎grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,26 +75,28 @@ class GrpclbLoadBalancer extends LoadBalancer {
7575
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
7676
Attributes attributes = resolvedAddresses.getAttributes();
7777
List<EquivalentAddressGroup> newLbAddresses = attributes.get(GrpclbConstants.ATTR_LB_ADDRS);
78-
if ((newLbAddresses == null || newLbAddresses.isEmpty())
79-
&& resolvedAddresses.getAddresses().isEmpty()) {
78+
if (newLbAddresses == null) {
79+
newLbAddresses = Collections.emptyList();
80+
}
81+
if (newLbAddresses.isEmpty() && resolvedAddresses.getAddresses().isEmpty()) {
8082
handleNameResolutionError(
8183
Status.UNAVAILABLE.withDescription("No backend or balancer addresses found"));
8284
return;
8385
}
84-
List<LbAddressGroup> newLbAddressGroups = new ArrayList<>();
85-
86-
if (newLbAddresses != null) {
87-
for (EquivalentAddressGroup lbAddr : newLbAddresses) {
88-
String lbAddrAuthority = lbAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY);
89-
if (lbAddrAuthority == null) {
90-
throw new AssertionError(
91-
"This is a bug: LB address " + lbAddr + " does not have an authority.");
92-
}
93-
newLbAddressGroups.add(new LbAddressGroup(lbAddr, lbAddrAuthority));
86+
List<EquivalentAddressGroup> overrideAuthorityLbAddresses =
87+
new ArrayList<>(newLbAddresses.size());
88+
for (EquivalentAddressGroup lbAddr : newLbAddresses) {
89+
String lbAddrAuthority = lbAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY);
90+
if (lbAddrAuthority == null) {
91+
throw new AssertionError(
92+
"This is a bug: LB address " + lbAddr + " does not have an authority.");
9493
}
94+
Attributes attrs = lbAddr.getAttributes().toBuilder()
95+
.set(EquivalentAddressGroup.ATTR_AUTHORITY_OVERRIDE, lbAddrAuthority)
96+
.build();
97+
overrideAuthorityLbAddresses.add(new EquivalentAddressGroup(lbAddr.getAddresses(), attrs));
9598
}
9699

97-
newLbAddressGroups = Collections.unmodifiableList(newLbAddressGroups);
98100
List<EquivalentAddressGroup> newBackendServers =
99101
Collections.unmodifiableList(resolvedAddresses.getAddresses());
100102
GrpclbConfig newConfig = (GrpclbConfig) resolvedAddresses.getLoadBalancingPolicyConfig();
@@ -106,7 +108,8 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
106108
helper.getChannelLogger().log(ChannelLogLevel.INFO, "Config: " + newConfig);
107109
recreateStates();
108110
}
109-
grpclbState.handleAddresses(newLbAddressGroups, newBackendServers);
111+
grpclbState.handleAddresses(Collections.unmodifiableList(overrideAuthorityLbAddresses),
112+
newBackendServers);
110113
}
111114

112115
@Override

0 commit comments

Comments
 (0)