Skip to content

Commit 4a2c5d6

Browse files
authored
core: fix a bug in health check config propgation. (#6804)
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.
1 parent 7be75a0 commit 4a2c5d6

File tree

2 files changed

+36
-8
lines changed

2 files changed

+36
-8
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,14 +1399,12 @@ public void run() {
13991399
Attributes effectiveAttrs = resolutionResult.getAttributes();
14001400
// Call LB only if it's not shutdown. If LB is shutdown, lbHelper won't match.
14011401
if (NameResolverListener.this.helper == ManagedChannelImpl.this.lbHelper) {
1402-
if (effectiveServiceConfig != validServiceConfig) {
1403-
Map<String, ?> healthCheckingConfig =
1404-
effectiveServiceConfig.getHealthCheckingConfig();
1405-
if (healthCheckingConfig != null) {
1406-
effectiveAttrs = effectiveAttrs.toBuilder()
1407-
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
1408-
.build();
1409-
}
1402+
Map<String, ?> healthCheckingConfig =
1403+
effectiveServiceConfig.getHealthCheckingConfig();
1404+
if (healthCheckingConfig != null) {
1405+
effectiveAttrs = effectiveAttrs.toBuilder()
1406+
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
1407+
.build();
14101408
}
14111409

14121410
Status handleResult = helper.lb.tryHandleResolvedAddresses(

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3918,6 +3918,36 @@ public void notUseDefaultImmediatelyIfEnableLookUp() throws Exception {
39183918
.build());
39193919
}
39203920

3921+
@Test
3922+
public void healthCheckingConfigPropagated() throws Exception {
3923+
LoadBalancerRegistry.getDefaultRegistry().register(mockLoadBalancerProvider);
3924+
try {
3925+
FakeNameResolverFactory nameResolverFactory =
3926+
new FakeNameResolverFactory.Builder(expectedUri)
3927+
.setServers(Collections.singletonList(new EquivalentAddressGroup(socketAddress)))
3928+
.build();
3929+
channelBuilder.nameResolverFactory(nameResolverFactory);
3930+
3931+
Map<String, Object> rawServiceConfig =
3932+
parseConfig("{\"healthCheckConfig\": {\"serviceName\": \"service1\"}}");
3933+
ManagedChannelServiceConfig managedChannelServiceConfig =
3934+
createManagedChannelServiceConfig(rawServiceConfig, null);
3935+
nameResolverFactory.nextConfigOrError.set(
3936+
ConfigOrError.fromConfig(managedChannelServiceConfig));
3937+
3938+
createChannel();
3939+
3940+
ArgumentCaptor<ResolvedAddresses> resultCaptor =
3941+
ArgumentCaptor.forClass(ResolvedAddresses.class);
3942+
verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture());
3943+
assertThat(resultCaptor.getValue().getAttributes()
3944+
.get(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG))
3945+
.containsExactly("serviceName", "service1");
3946+
} finally {
3947+
LoadBalancerRegistry.getDefaultRegistry().deregister(mockLoadBalancerProvider);
3948+
}
3949+
}
3950+
39213951
private static final class ChannelBuilder
39223952
extends AbstractManagedChannelImplBuilder<ChannelBuilder> {
39233953

0 commit comments

Comments
 (0)