Stop updating endpoints when HealthCheckedEndpointGroup is closing

[ikhoon]

Motivation:

When HealthCheckedEndpointGroup is closing, the internally managed endpoints are cleared.

armeria/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/DefaultHealthCheckerContext.java

Line 135 in 011741d

onUpdateHealth.accept(originalEndpoint.withAttrs(endpointAttributes), false);

In fact, the closing process is not a problem. However, when the endpoints change, the change is propagated to EndpointSelector. Some EndpointSelector implementations warn about the empty endpoints. The warning is not a real problem but a false positive alarm.

armeria/core/src/main/java/com/linecorp/armeria/client/endpoint/WeightedRoundRobinStrategy.java

Line 81 in 62da203

logger.warn("No valid endpoint with weight > 0. endpoints: {}", toShortString(endpoints));

For better compatibility with EndpointSelector implementations, I propose to stop additional endpoints from being updated when HealthCheckedEndpointGroup is closed. As a result, EndpointSelector will not updated with empty endpoints.

Modifications:

• Stop updating endpoints when HealthCheckedEndpointGroup is closing

Result:

You no loger see a false positive warning log when HealthCheckedEndpointGroup is closing.

[github-actions]

🔍 Build Scan® (commit: 6a88be8)

Job name	Status	Build Scan®
build-ubicloud-standard-8-jdk-8	✅	https://ge.armeria.dev/s/mfath3gcqpn5c
build-ubicloud-standard-8-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/zssixhmnz5u6q
build-ubicloud-standard-8-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/64lyyyjfblxw2
build-ubicloud-standard-8-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/zcmfuxuh5cvzg
build-ubicloud-standard-8-jdk-17-leak	✅	https://ge.armeria.dev/s/6srguzhyau63i
build-ubicloud-standard-8-jdk-11	✅	https://ge.armeria.dev/s/dignuykh7rnzy
build-macos-latest-jdk-21	❌ (failure)	https://ge.armeria.dev/s/rhw6eo5g3ye4u

[minwoox]

What do you think of not calling the updateHealth at all from the DefaultHealthCheckerContext?

private DefaultHealthCheckerContext newCheckerContext(Endpoint endpoint) {
    return new DefaultHealthCheckerContext(endpoint, port, protocol, clientOptions, retryBackoff,
                                           this::updateHealth, this);
}

DefaultHealthCheckerContext(Endpoint endpoint, int port, SessionProtocol protocol,
                            ClientOptions clientOptions, Backoff retryBackoff,
                            BiConsumer<Endpoint, Boolean> onUpdateHealth,
                            ListenableAsyncCloseable listenableAsyncCloseable) {
  ...
}

// Use listenableAsyncCloseable.isClosing( ) to check if the HealthCheckedEndpointGroup is closing or not.

[minwoox]

I realized that there's no heavy logic in the DefaultHealthCheckerContext.
So probably just check isClosing() in the updateHealth() method would be enough.