api: deprecate Helper.updateSubchannelAddresses() and add equivalent on Subchannel (#5802)

Resolves #5676
This commit is contained in:
Kun Zhang 2019-05-30 09:16:38 -07:00 committed by GitHub
parent 9b4c958201
commit af2c16d301
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 66 additions and 11 deletions

View File

@ -963,7 +963,9 @@ public abstract class LoadBalancer {
* href="https://github.com/grpc/grpc-java/issues/5015">#5015</a> for the background.
*
* @since 1.4.0
* @deprecated use {@link Subchannel#updateAddresses} instead
*/
@Deprecated
public final void updateSubchannelAddresses(
Subchannel subchannel, EquivalentAddressGroup addrs) {
checkNotNull(addrs, "addrs");
@ -982,7 +984,9 @@ public abstract class LoadBalancer {
* @throws IllegalArgumentException if {@code subchannel} was not returned from {@link
* #createSubchannel} or {@code addrs} is empty
* @since 1.14.0
* @deprecated use {@link Subchannel#updateAddresses} instead
*/
@Deprecated
public void updateSubchannelAddresses(
Subchannel subchannel, List<EquivalentAddressGroup> addrs) {
throw new UnsupportedOperationException();
@ -1301,6 +1305,19 @@ public abstract class LoadBalancer {
throw new UnsupportedOperationException();
}
/**
* Replaces the existing addresses used with this {@code Subchannel}. If the new and old
* addresses overlap, the Subchannel can continue using an existing connection.
*
* <p>It must be called from the Synchronization Context or will throw.
*
* @throws IllegalArgumentException if {@code addrs} is empty
* @since 1.22.0
*/
public void updateAddresses(List<EquivalentAddressGroup> addrs) {
throw new UnsupportedOperationException();
}
/**
* (Internal use only) returns an object that represents the underlying subchannel that is used
* by the Channel for sending RPCs when this {@link Subchannel} is picked. This is an opaque

View File

@ -167,6 +167,7 @@ public class LoadBalancerTest {
}
}
@Deprecated
@Test
public void helper_updateSubchannelAddresses_delegates() {
class OverrideUpdateSubchannel extends NoopHelper {
@ -187,6 +188,7 @@ public class LoadBalancerTest {
assertThat(helper.ran).isTrue();
}
@Deprecated
@Test(expected = UnsupportedOperationException.class)
public void helper_updateSubchannelAddressesList_throws() {
new NoopHelper().updateSubchannelAddresses(null, Arrays.asList(eag));

View File

@ -1127,6 +1127,7 @@ final class ManagedChannelImpl extends ManagedChannel implements
syncContext.execute(new LoadBalancerRefreshNameResolution());
}
@Deprecated
@Override
public void updateSubchannelAddresses(
LoadBalancer.Subchannel subchannel, List<EquivalentAddressGroup> addrs) {
@ -1621,6 +1622,12 @@ final class ManagedChannelImpl extends ManagedChannel implements
public ChannelLogger getChannelLogger() {
return subchannelLogger;
}
@Override
public void updateAddresses(List<EquivalentAddressGroup> addrs) {
syncContext.throwIfNotInThisSynchronizationContext();
subchannel.updateAddresses(addrs);
}
}
@Override

View File

@ -64,7 +64,7 @@ final class PickFirstLoadBalancer extends LoadBalancer {
helper.updateBalancingState(CONNECTING, new Picker(PickResult.withSubchannel(subchannel)));
subchannel.requestConnection();
} else {
helper.updateSubchannelAddresses(subchannel, servers);
subchannel.updateAddresses(servers);
}
}

View File

@ -51,6 +51,7 @@ public abstract class ForwardingLoadBalancerHelper extends LoadBalancer.Helper {
return delegate().createSubchannel(args);
}
@Deprecated
@Override
public void updateSubchannelAddresses(
Subchannel subchannel, List<EquivalentAddressGroup> addrs) {

View File

@ -74,6 +74,11 @@ public abstract class ForwardingSubchannel extends LoadBalancer.Subchannel {
return delegate().getInternalSubchannel();
}
@Override
public void updateAddresses(List<EquivalentAddressGroup> addrs) {
delegate().updateAddresses(addrs);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this).add("delegate", delegate()).toString();

View File

@ -549,7 +549,7 @@ public class ManagedChannelImplIdlenessTest {
new Runnable() {
@Override
public void run() {
helper.updateSubchannelAddresses(subchannel, addrs);
subchannel.updateAddresses(Collections.singletonList(addrs));
}
});
}

View File

@ -184,7 +184,16 @@ public class ManagedChannelImplTest {
return "test-addr";
}
};
private final SocketAddress socketAddress2 =
new SocketAddress() {
@Override
public String toString() {
return "test-addr";
}
};
private final EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(socketAddress);
private final EquivalentAddressGroup addressGroup2 =
new EquivalentAddressGroup(Arrays.asList(socketAddress, socketAddress2));
private final FakeClock timer = new FakeClock();
private final FakeClock executor = new FakeClock();
private final FakeClock balancerRpcExecutor = new FakeClock();
@ -1322,6 +1331,11 @@ public class ManagedChannelImplTest {
.newClientTransport(
eq(socketAddress), eq(clientTransportOptions), isA(TransportLogger.class));
// updateAddresses()
updateAddressesSafely(helper, sub1, Collections.singletonList(addressGroup2));
assertThat(((InternalSubchannel) sub1.getInternalSubchannel()).getAddressGroups())
.isEqualTo(Collections.singletonList(addressGroup2));
// shutdown() has a delay
shutdownSafely(helper, sub1);
timer.forwardTime(ManagedChannelImpl.SUBCHANNEL_SHUTDOWN_DELAY_SECONDS - 1, TimeUnit.SECONDS);
@ -4116,6 +4130,17 @@ public class ManagedChannelImplTest {
});
}
private static void updateAddressesSafely(
Helper helper, final Subchannel subchannel, final List<EquivalentAddressGroup> addrs) {
helper.getSynchronizationContext().execute(
new Runnable() {
@Override
public void run() {
subchannel.updateAddresses(addrs);
}
});
}
private static void shutdownSafely(
final Helper helper, final Subchannel subchannel) {
helper.getSynchronizationContext().execute(

View File

@ -172,6 +172,7 @@ public class PickFirstLoadBalancerTest {
verify(mockSubchannel).requestConnection();
loadBalancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build());
verify(mockSubchannel).updateAddresses(eq(servers));
verifyNoMoreInteractions(mockSubchannel);
verify(mockHelper).createSubchannel(createArgsCaptor.capture());
@ -179,8 +180,7 @@ public class PickFirstLoadBalancerTest {
verify(mockHelper)
.updateBalancingState(isA(ConnectivityState.class), isA(SubchannelPicker.class));
// Updating the subchannel addresses is unnecessary, but doesn't hurt anything
verify(mockHelper).updateSubchannelAddresses(
eq(mockSubchannel), ArgumentMatchers.<EquivalentAddressGroup>anyList());
verify(mockSubchannel).updateAddresses(ArgumentMatchers.<EquivalentAddressGroup>anyList());
verifyNoMoreInteractions(mockHelper);
}
@ -191,7 +191,7 @@ public class PickFirstLoadBalancerTest {
List<EquivalentAddressGroup> newServers =
Lists.newArrayList(new EquivalentAddressGroup(socketAddr));
InOrder inOrder = inOrder(mockHelper);
InOrder inOrder = inOrder(mockHelper, mockSubchannel);
loadBalancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build());
@ -205,7 +205,7 @@ public class PickFirstLoadBalancerTest {
loadBalancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(newServers).setAttributes(affinity).build());
inOrder.verify(mockHelper).updateSubchannelAddresses(eq(mockSubchannel), eq(newServers));
inOrder.verify(mockSubchannel).updateAddresses(eq(newServers));
verifyNoMoreInteractions(mockSubchannel);
verifyNoMoreInteractions(mockHelper);

View File

@ -438,7 +438,7 @@ final class GrpclbState {
} else {
checkState(subchannels.size() == 1, "Unexpected Subchannel count: %s", subchannels);
subchannel = subchannels.values().iterator().next();
helper.updateSubchannelAddresses(subchannel, eagList);
subchannel.updateAddresses(eagList);
}
subchannels = Collections.singletonMap(eagList, subchannel);
newBackendList.add(

View File

@ -1776,8 +1776,7 @@ public class GrpclbLoadBalancerTest {
// createSubchannel() has ever been called only once
verify(helper, times(1)).createSubchannel(any(List.class), any(Attributes.class));
assertThat(mockSubchannels).isEmpty();
inOrder.verify(helper).updateSubchannelAddresses(
same(subchannel),
verify(subchannel).updateAddresses(
eq(Arrays.asList(
new EquivalentAddressGroup(backends2.get(0).addr, eagAttrsWithToken("token0001")),
new EquivalentAddressGroup(backends2.get(2).addr,
@ -1874,8 +1873,7 @@ public class GrpclbLoadBalancerTest {
// createSubchannel() has ever been called only once
verify(helper, times(1)).createSubchannel(any(List.class), any(Attributes.class));
assertThat(mockSubchannels).isEmpty();
inOrder.verify(helper).updateSubchannelAddresses(
same(subchannel),
verify(subchannel).updateAddresses(
eq(Arrays.asList(
new EquivalentAddressGroup(backends1.get(0).addr, eagAttrsWithToken("token0001")),
new EquivalentAddressGroup(backends1.get(1).addr,