Skip to content

Commit

Permalink
core: fix a bug in health check config propgation. (#6804)
Browse files Browse the repository at this point in the history
The condition "effectiveServiceConfig != validServiceConfig" should
have been deleted in commit 2162ad0.

The condition was there before that commit because
NAME_RESOLVER_SERVICE_CONFIG was already in "attrs", thus it needed to
be re-added only if "effectiveServiceConfig" differs from the original
"validServiceConfig".

In contrast, ATTR_HEALTH_CHECKING_CONFIG is not in the original
"attrs" and always needs to be added.
  • Loading branch information
zhangkun83 committed Mar 5, 2020
1 parent 7be75a0 commit 4a2c5d6
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 8 deletions.
14 changes: 6 additions & 8 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Expand Up @@ -1399,14 +1399,12 @@ public void run() {
Attributes effectiveAttrs = resolutionResult.getAttributes();
// Call LB only if it's not shutdown. If LB is shutdown, lbHelper won't match.
if (NameResolverListener.this.helper == ManagedChannelImpl.this.lbHelper) {
if (effectiveServiceConfig != validServiceConfig) {
Map<String, ?> healthCheckingConfig =
effectiveServiceConfig.getHealthCheckingConfig();
if (healthCheckingConfig != null) {
effectiveAttrs = effectiveAttrs.toBuilder()
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
.build();
}
Map<String, ?> healthCheckingConfig =
effectiveServiceConfig.getHealthCheckingConfig();
if (healthCheckingConfig != null) {
effectiveAttrs = effectiveAttrs.toBuilder()
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
.build();
}

Status handleResult = helper.lb.tryHandleResolvedAddresses(
Expand Down
30 changes: 30 additions & 0 deletions core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java
Expand Up @@ -3918,6 +3918,36 @@ public void notUseDefaultImmediatelyIfEnableLookUp() throws Exception {
.build());
}

@Test
public void healthCheckingConfigPropagated() throws Exception {
LoadBalancerRegistry.getDefaultRegistry().register(mockLoadBalancerProvider);
try {
FakeNameResolverFactory nameResolverFactory =
new FakeNameResolverFactory.Builder(expectedUri)
.setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
.build();
channelBuilder.nameResolverFactory(nameResolverFactory);

Map<String, Object> rawServiceConfig =
parseConfig("{\"healthCheckConfig\": {\"serviceName\": \"service1\"}}");
ManagedChannelServiceConfig managedChannelServiceConfig =
createManagedChannelServiceConfig(rawServiceConfig, null);
nameResolverFactory.nextConfigOrError.set(
ConfigOrError.fromConfig(managedChannelServiceConfig));

createChannel();

ArgumentCaptor<ResolvedAddresses> resultCaptor =
ArgumentCaptor.forClass(ResolvedAddresses.class);
verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture());
assertThat(resultCaptor.getValue().getAttributes()
.get(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG))
.containsExactly("serviceName", "service1");
} finally {
LoadBalancerRegistry.getDefaultRegistry().deregister(mockLoadBalancerProvider);
}
}

private static final class ChannelBuilder
extends AbstractManagedChannelImplBuilder<ChannelBuilder> {

Expand Down

0 comments on commit 4a2c5d6

Please sign in to comment.