grpclb: force GRPCLB policy when NameResolver returns at least one balancer. (#3291)

Previously only when all addresses returned by NameResolver are
balancers would GRPCLB be forced.  This change is a follow-up of
https://github.com/grpc/grpc/pull/10258
This commit is contained in:
Kun Zhang 2017-07-31 11:07:11 -07:00 committed by GitHub
parent 9be41ba0e8
commit f54a2df67f
2 changed files with 12 additions and 16 deletions

View File

@ -184,14 +184,11 @@ class GrpclbLoadBalancer extends LoadBalancer implements WithLogId {
} }
} }
if (newBackendServers.isEmpty()) { if (!newLbAddressGroups.isEmpty()) {
// handleResolvedAddressGroups()'s javadoc has guaranteed updatedServers is never empty.
checkState(!newLbAddressGroups.isEmpty(),
"No backend address nor LB address. updatedServers=%s", updatedServers);
if (newLbPolicy != LbPolicy.GRPCLB) { if (newLbPolicy != LbPolicy.GRPCLB) {
newLbPolicy = LbPolicy.GRPCLB; newLbPolicy = LbPolicy.GRPCLB;
logger.log(Level.FINE, "[{0}] Switching to GRPCLB because all addresses are balancers", logger.log(
logId); Level.FINE, "[{0}] Switching to GRPCLB because there is at least one balancer", logId);
} }
} }
if (newLbPolicy == null) { if (newLbPolicy == null) {

View File

@ -755,7 +755,7 @@ public class GrpclbLoadBalancerTest {
// Switch to PICK_FIRST // Switch to PICK_FIRST
List<EquivalentAddressGroup> pickFirstResolutionList = List<EquivalentAddressGroup> pickFirstResolutionList =
createResolvedServerAddresses(true, false); createResolvedServerAddresses(false, false);
Attributes pickFirstResolutionAttrs = Attributes.newBuilder() Attributes pickFirstResolutionAttrs = Attributes.newBuilder()
.set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.PICK_FIRST).build(); .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.PICK_FIRST).build();
verify(pickFirstBalancerFactory, never()).newLoadBalancer(any(Helper.class)); verify(pickFirstBalancerFactory, never()).newLoadBalancer(any(Helper.class));
@ -769,7 +769,7 @@ public class GrpclbLoadBalancerTest {
verify(pickFirstBalancerFactory).newLoadBalancer(same(helper)); verify(pickFirstBalancerFactory).newLoadBalancer(same(helper));
// Only non-LB addresses are passed to the delegate // Only non-LB addresses are passed to the delegate
verify(pickFirstBalancer).handleResolvedAddressGroups( verify(pickFirstBalancer).handleResolvedAddressGroups(
eq(Arrays.asList(pickFirstResolutionList.get(1))), same(pickFirstResolutionAttrs)); eq(pickFirstResolutionList), same(pickFirstResolutionAttrs));
assertSame(LbPolicy.PICK_FIRST, balancer.getLbPolicy()); assertSame(LbPolicy.PICK_FIRST, balancer.getLbPolicy());
assertSame(pickFirstBalancer, balancer.getDelegate()); assertSame(pickFirstBalancer, balancer.getDelegate());
// GRPCLB connection is closed // GRPCLB connection is closed
@ -778,7 +778,7 @@ public class GrpclbLoadBalancerTest {
// Switch to ROUND_ROBIN // Switch to ROUND_ROBIN
List<EquivalentAddressGroup> roundRobinResolutionList = List<EquivalentAddressGroup> roundRobinResolutionList =
createResolvedServerAddresses(true, false, false); createResolvedServerAddresses(false, false, false);
Attributes roundRobinResolutionAttrs = Attributes.newBuilder() Attributes roundRobinResolutionAttrs = Attributes.newBuilder()
.set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.ROUND_ROBIN).build(); .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.ROUND_ROBIN).build();
verify(roundRobinBalancerFactory, never()).newLoadBalancer(any(Helper.class)); verify(roundRobinBalancerFactory, never()).newLoadBalancer(any(Helper.class));
@ -787,13 +787,13 @@ public class GrpclbLoadBalancerTest {
verify(roundRobinBalancerFactory).newLoadBalancer(same(helper)); verify(roundRobinBalancerFactory).newLoadBalancer(same(helper));
// Only non-LB addresses are passed to the delegate // Only non-LB addresses are passed to the delegate
verify(roundRobinBalancer).handleResolvedAddressGroups( verify(roundRobinBalancer).handleResolvedAddressGroups(
eq(roundRobinResolutionList.subList(1, 3)), same(roundRobinResolutionAttrs)); eq(roundRobinResolutionList), same(roundRobinResolutionAttrs));
assertSame(LbPolicy.ROUND_ROBIN, balancer.getLbPolicy()); assertSame(LbPolicy.ROUND_ROBIN, balancer.getLbPolicy());
assertSame(roundRobinBalancer, balancer.getDelegate()); assertSame(roundRobinBalancer, balancer.getDelegate());
// Special case: if all addresses are loadbalancers, use GRPCLB no matter what the NameResolver // Special case: if at least one address is loadbalancer, use GRPCLB no matter what the
// says. // NameResolver says.
grpclbResolutionList = createResolvedServerAddresses(true, true, true); grpclbResolutionList = createResolvedServerAddresses(true, false, true, false);
grpclbResolutionAttrs = Attributes.newBuilder() grpclbResolutionAttrs = Attributes.newBuilder()
.set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.PICK_FIRST).build(); .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.PICK_FIRST).build();
deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs); deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs);
@ -802,7 +802,6 @@ public class GrpclbLoadBalancerTest {
assertNull(balancer.getDelegate()); assertNull(balancer.getDelegate());
EquivalentAddressGroup combinedEag = new EquivalentAddressGroup(Arrays.asList( EquivalentAddressGroup combinedEag = new EquivalentAddressGroup(Arrays.asList(
grpclbResolutionList.get(0).getAddresses().get(0), grpclbResolutionList.get(0).getAddresses().get(0),
grpclbResolutionList.get(1).getAddresses().get(0),
grpclbResolutionList.get(2).getAddresses().get(0))); grpclbResolutionList.get(2).getAddresses().get(0)));
verify(helper).createOobChannel(eq(combinedEag), eq(lbAuthority(0))); verify(helper).createOobChannel(eq(combinedEag), eq(lbAuthority(0)));
assertEquals(1, fakeOobChannels.size()); assertEquals(1, fakeOobChannels.size());
@ -810,7 +809,7 @@ public class GrpclbLoadBalancerTest {
verify(mockLbService, times(2)).balanceLoad(lbResponseObserverCaptor.capture()); verify(mockLbService, times(2)).balanceLoad(lbResponseObserverCaptor.capture());
// Special case: PICK_FIRST is the default // Special case: PICK_FIRST is the default
pickFirstResolutionList = createResolvedServerAddresses(true, false, false); pickFirstResolutionList = createResolvedServerAddresses(false, false, false);
pickFirstResolutionAttrs = Attributes.EMPTY; pickFirstResolutionAttrs = Attributes.EMPTY;
verify(pickFirstBalancerFactory).newLoadBalancer(any(Helper.class)); verify(pickFirstBalancerFactory).newLoadBalancer(any(Helper.class));
assertFalse(oobChannel.isShutdown()); assertFalse(oobChannel.isShutdown());
@ -819,7 +818,7 @@ public class GrpclbLoadBalancerTest {
verify(pickFirstBalancerFactory, times(2)).newLoadBalancer(same(helper)); verify(pickFirstBalancerFactory, times(2)).newLoadBalancer(same(helper));
// Only non-LB addresses are passed to the delegate // Only non-LB addresses are passed to the delegate
verify(pickFirstBalancer).handleResolvedAddressGroups( verify(pickFirstBalancer).handleResolvedAddressGroups(
eq(pickFirstResolutionList.subList(1, 3)), same(pickFirstResolutionAttrs)); eq(pickFirstResolutionList), same(pickFirstResolutionAttrs));
assertSame(LbPolicy.PICK_FIRST, balancer.getLbPolicy()); assertSame(LbPolicy.PICK_FIRST, balancer.getLbPolicy());
assertSame(pickFirstBalancer, balancer.getDelegate()); assertSame(pickFirstBalancer, balancer.getDelegate());
// GRPCLB connection is closed // GRPCLB connection is closed