Skip to content

Commit

Permalink
Disable RRLB health-checking for DiscoveryStrategy.ON_NEW_CONNECTION (
Browse files Browse the repository at this point in the history
#2772)

Motivation:

When `DiscoveryStrategy.ON_NEW_CONNECTION` mode is used, LB will have a
single unresolved entry, the actual IP address will be resolved by a
`ConnectionFactory`. We should not run health-checking logic in this
case because it can lead to "all unhealthy" state. We should run a new
connection attempt all the time and fail with an exception from
transport.

Modifications:

- Configure a custom `loadBalancerFactory` on a client builder when
`DiscoveryStrategy.ON_NEW_CONNECTION` is used;

Result:

No health-checking for an unresolved address when
`DiscoveryStrategy.ON_NEW_CONNECTION` is used.
  • Loading branch information
idelpivnitskiy authored Dec 7, 2023
1 parent 112fb07 commit 3266749
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.servicetalk.concurrent.api.ListenableAsyncCloseable;
import io.servicetalk.concurrent.api.Publisher;
import io.servicetalk.http.api.DelegatingSingleAddressHttpClientBuilder;
import io.servicetalk.http.api.FilterableStreamingHttpLoadBalancedConnection;
import io.servicetalk.http.api.HttpClient;
import io.servicetalk.http.api.HttpHeaderNames;
import io.servicetalk.http.api.HttpProviders.MultiAddressHttpClientBuilderProvider;
Expand All @@ -39,6 +40,7 @@
import io.servicetalk.http.api.ProxyConfig;
import io.servicetalk.http.api.SingleAddressHttpClientBuilder;
import io.servicetalk.http.api.StreamingHttpRequest;
import io.servicetalk.loadbalancer.RoundRobinLoadBalancers;
import io.servicetalk.transport.api.HostAndPort;
import io.servicetalk.transport.api.TransportObserver;

Expand Down Expand Up @@ -451,6 +453,15 @@ private static <U, R> SingleAddressHttpClientBuilder<U, R> forSingleAddress(
// Apply after providers to let them see these customizations.
.serviceDiscoverer(usd)
.retryServiceDiscoveryErrors(NoRetriesStrategy.INSTANCE)
// Disable health-checking:
.loadBalancerFactory(DefaultHttpLoadBalancerFactory.Builder.from(
RoundRobinLoadBalancers.<R, FilterableStreamingHttpLoadBalancedConnection>builder(
// Use a different ID to let providers distinguish this LB from the default one
DefaultHttpLoadBalancerFactory.class.getSimpleName() + '-' +
DiscoveryStrategy.ON_NEW_CONNECTION.name())
.healthCheckFailedConnectionsThreshold(-1)
.build())
.build())
.appendConnectionFactoryFilter(resolvingConnectionFactory.get());
default:
throw new IllegalArgumentException("Unsupported strategy: " + discoveryStrategy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,18 @@ void attemptToOverrideServiceDiscovererThrows() {
assertThat(e.getMessage(), allOf(containsString(ON_NEW_CONNECTION.name()), containsString(otherSd.toString())));
}

@Test
void noHealthChecking() throws Exception {
try (BlockingHttpClient client = HttpClients.forSingleAddress(FailureCase.SERVICE_DISCOVERER_FAILED.customSd,
new UnresolvedAddress(null), ON_NEW_CONNECTION).buildBlocking()) {
// The default health-checking threshold is 5, validate that client doesn't start returning
// NoActiveHostException instead of DeliberateException after 5 attempts
for (int i = 0; i < 10; i++) {
assertThrows(DeliberateException.class, () -> client.request(client.get("/")));
}
}
}

@ParameterizedTest(name = "{displayName} [{index}]: failureCase={0}")
@EnumSource(FailureCase.class)
void failureCases(FailureCase failureCase) throws Exception {
Expand Down

0 comments on commit 3266749

Please sign in to comment.