diff --git a/java-client/src/main/java/co/elastic/clients/transport/BackoffPolicy.java b/java-client/src/main/java/co/elastic/clients/transport/BackoffPolicy.java index 798af4c063..0b09d684f7 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/BackoffPolicy.java +++ b/java-client/src/main/java/co/elastic/clients/transport/BackoffPolicy.java @@ -55,7 +55,7 @@ public static BackoffPolicy noBackoff() { } /** - * Creates an new constant backoff policy with the provided configuration. + * Creates a new constant backoff policy with the provided configuration. * * @param delay The delay defines how long to wait between retry attempts. Must not be null. * Must be <= Integer.MAX_VALUE ms. @@ -68,7 +68,7 @@ public static BackoffPolicy constantBackoff(Long delay, int maxNumberOfRetries) } /** - * Creates an new exponential backoff policy with a default configuration of 50 ms initial wait period and 8 retries taking + * Creates a new exponential backoff policy with a default configuration of 50 ms initial wait period and 8 retries taking * roughly 5.1 seconds in total. * * @return A backoff policy with an exponential increase in wait time for retries. The returned instance is thread safe but each @@ -79,7 +79,7 @@ public static BackoffPolicy exponentialBackoff() { } /** - * Creates an new exponential backoff policy with the provided configuration. + * Creates a new exponential backoff policy with the provided configuration. * * @param initialDelay The initial delay defines how long to wait for the first retry attempt. Must not be null. * Must be <= Integer.MAX_VALUE ms. diff --git a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java index 9714b863c8..589fcc8baa 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java @@ -38,6 +38,7 @@ public class DefaultTransportOptions implements TransportOptions { private final Map parameters; private final Function, Boolean> onWarnings; private boolean keepResponseBodyOnException; + private RetryConfig retryConfig; public static final DefaultTransportOptions EMPTY = new DefaultTransportOptions(); @@ -51,8 +52,19 @@ public DefaultTransportOptions( @Nullable Function, Boolean> onWarnings, boolean keepResponseBodyOnException ) { - this(headers,parameters,onWarnings); + this(headers, parameters, onWarnings, keepResponseBodyOnException, RetryConfig.disabled()); + } + + public DefaultTransportOptions( + @Nullable HeaderMap headers, + @Nullable Map parameters, + @Nullable Function, Boolean> onWarnings, + boolean keepResponseBodyOnException, + @Nullable RetryConfig retryConfig + ) { + this(headers, parameters, onWarnings); this.keepResponseBodyOnException = keepResponseBodyOnException; + this.retryConfig = retryConfig == null ? RetryConfig.disabled() : retryConfig; } public DefaultTransportOptions( @@ -65,10 +77,11 @@ public DefaultTransportOptions( Collections.emptyMap() : Collections.unmodifiableMap(parameters); this.onWarnings = onWarnings; this.keepResponseBodyOnException = false; + this.retryConfig = RetryConfig.disabled(); } protected DefaultTransportOptions(AbstractBuilder builder) { - this(builder.headers, builder.parameters, builder.onWarnings, builder.keepResponseBodyOnException); + this(builder.headers, builder.parameters, builder.onWarnings, builder.keepResponseBodyOnException, builder.retryConfig); } public static DefaultTransportOptions of(@Nullable TransportOptions options) { @@ -111,6 +124,11 @@ public boolean keepResponseBodyOnException() { return keepResponseBodyOnException; } + @Override + public RetryConfig retryConfig() { + return retryConfig; + } + @Override public Builder toBuilder() { return new Builder(this); @@ -135,6 +153,7 @@ public abstract static class AbstractBuilder parameters; private Function, Boolean> onWarnings; private boolean keepResponseBodyOnException; + private RetryConfig retryConfig = RetryConfig.disabled(); public AbstractBuilder() { } @@ -144,6 +163,7 @@ public AbstractBuilder(DefaultTransportOptions options) { this.parameters = copyOrNull(options.parameters); this.onWarnings = options.onWarnings; this.keepResponseBodyOnException = options.keepResponseBodyOnException; + this.retryConfig = options.retryConfig == null ? RetryConfig.disabled() : options.retryConfig; } protected abstract BuilderT self(); @@ -154,6 +174,12 @@ public BuilderT keepResponseBodyOnException(boolean value) { return self(); } + @Override + public BuilderT retryConfig(RetryConfig config) { + this.retryConfig = config == null ? RetryConfig.disabled() : config; + return self(); + } + @Override public BuilderT addHeader(String name, String value) { if (name.equalsIgnoreCase(HeaderMap.CLIENT_META)) { diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 51e4d8ef50..66f682a2f9 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -32,6 +32,7 @@ import co.elastic.clients.transport.endpoints.TextEndpoint; import co.elastic.clients.transport.http.HeaderMap; import co.elastic.clients.transport.http.RepeatableBodyResponse; +import co.elastic.clients.transport.http.RetryingHttpClient; import co.elastic.clients.transport.http.TransportHttpClient; import co.elastic.clients.transport.instrumentation.Instrumentation; import co.elastic.clients.transport.instrumentation.NoopInstrumentation; @@ -80,6 +81,14 @@ public abstract class ElasticsearchTransportBase implements ElasticsearchTranspo } protected final TransportHttpClient httpClient; + // Single retry wrapper, shared by every request that uses retries (whether configured on the client or per + // request) and reading the effective RetryConfig from each call's TransportOptions. Created lazily on first + // use, so clients that never use retries keep the exact same object graph as before and never allocate a + // retry scheduler. Creation and close() are serialized on `this` so a wrapper can't be created after close + // and leak its scheduler. + private volatile RetryingHttpClient retryingHttpClient; + // Guarded by `this` + private boolean closed; protected final Instrumentation instrumentation; protected final JsonpMapper mapper; protected final TransportOptions transportOptions; @@ -120,7 +129,18 @@ protected ElasticsearchTransportBase cloneWith( @Override public void close() throws IOException { - httpClient.close(); + // The retry wrapper, if it was ever created, owns the delegate and the retry scheduler, so closing it + // also closes httpClient and shuts down the scheduler. Otherwise close httpClient directly. + RetryingHttpClient retrying; + synchronized (this) { + closed = true; + retrying = retryingHttpClient; + } + if (retrying != null) { + retrying.close(); + } else { + httpClient.close(); + } } @Override @@ -137,6 +157,38 @@ public TransportHttpClient httpClient() { return httpClient; } + // Returns the shared retry wrapper when this request uses retries, the bare http client otherwise. The + // wrapper is created lazily on first use, so clients that never retry keep the non-retry path untouched. + private TransportHttpClient httpClientFor(TransportOptions options) { + RetryConfig retryConfig = options.retryConfig(); + if (retryConfig == null || !retryConfig.isEnabled()) { + return httpClient; + } + // If the user supplied a client that already retries, don't wrap it a second time: both layers would + // read the same RetryConfig and multiply attempts. + if (httpClient instanceof RetryingHttpClient) { + return httpClient; + } + return retryingHttpClient(); + } + + private RetryingHttpClient retryingHttpClient() { + RetryingHttpClient client = retryingHttpClient; + if (client == null) { + synchronized (this) { + if (closed) { + throw new IllegalStateException("Transport has been closed"); + } + client = retryingHttpClient; + if (client == null) { + client = new RetryingHttpClient(httpClient); + retryingHttpClient = client; + } + } + } + return client; + } + @Override public final ResponseT performRequest( RequestT request, @@ -150,7 +202,7 @@ public final ResponseT performRequest( TransportHttpClient.Request req = prepareTransportRequest(request, endpoint); ctx.beforeSendingHttpRequest(req, options); - TransportHttpClient.Response resp = httpClient.performRequest(endpoint.id(), null, req, opts); + TransportHttpClient.Response resp = httpClientFor(opts).performRequest(endpoint.id(), null, req, opts); ctx.afterReceivingHttpResponse(resp); ResponseT apiResponse = getApiResponse(resp, endpoint); @@ -189,7 +241,7 @@ public final CompletableFuture performR // Propagate required property checks to the thread that will decode the response boolean disableRequiredChecks = ApiTypeHelper.requiredPropertiesCheckDisabled(); - CompletableFuture clientFuture = httpClient.performRequestAsync( + CompletableFuture clientFuture = httpClientFor(opts).performRequestAsync( endpoint.id(), null, clientReq, opts ); diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportConfig.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportConfig.java index 2d7b9ccb38..07d60de922 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportConfig.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportConfig.java @@ -30,6 +30,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.List; +import java.util.function.Consumer; import java.util.function.Function; public abstract class ElasticsearchTransportConfig { @@ -326,6 +327,20 @@ public BuilderT transportOptions(Function o.retryConfig(retryConfig)); + } + + /** + * Configure transport-level retries with a builder lambda. + */ + public BuilderT retryConfig(Consumer fn) { + return transportOptions(o -> o.retryConfig(fn)); + } + protected void checkNull(Object value, String name, String other) { if (value != null) { throw new IllegalArgumentException("Cannot set both " + other + " and " + name + "."); diff --git a/java-client/src/main/java/co/elastic/clients/transport/RetryConfig.java b/java-client/src/main/java/co/elastic/clients/transport/RetryConfig.java new file mode 100644 index 0000000000..16a2d23eab --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/transport/RetryConfig.java @@ -0,0 +1,194 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.transport; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.util.Objects; +import java.util.Set; +import java.util.function.Consumer; + +/** + * Configuration for transport-level retries. A request is retried when: + *
    + *
  • it results in a response — even one carried by an exception, such as the low-level clients' + * {@code ResponseException} — whose status code is in {@link #retryableStatuses()}, or
  • + *
  • it fails with an exception that matches one of {@link #retryableExceptions()}.
  • + *
+ * Either set may be empty to disable that retry route. + *

+ * Delays and the maximum number of attempts are controlled by {@link #backoffPolicy()}, which must be set for + * retries to happen: retries are always opt-in. + *

+ * Scope: retries reissue the same logical request to the underlying transport. Node selection is delegated + * to that transport (e.g. {@code Rest5Client}, {@code RestClient}), which manages its own node-rotation and + * dead-node tracking independently. Configuring retries here does not control which node a retried + * request is sent to. Note that for some failures (e.g. 502/503/504 responses and connection errors) the + * underlying client already tries every configured node before reporting the failure, so the total number of + * attempts on the cluster can be up to {@code nodes × (retries + 1)}. + *

+ * Idempotency: a retried request may have already been received and processed by the server (e.g. when the + * connection drops after the request was sent), giving at-least-once semantics. Most Elasticsearch APIs are + * idempotent, but retrying operations that are not (e.g. bulk indexing without user-provided document ids) can + * duplicate their effects. Choose {@link #retryableExceptions()} accordingly. + *

+ * Memory: the request body stays referenced for the whole retry sequence, including backoff waits. With + * large requests (e.g. big bulks) and long backoffs, this pins the corresponding buffers in memory until the + * request completes or fails definitively. + */ +public final class RetryConfig { + + /** + * Default set of HTTP status codes that trigger a retry: {@code 429, 500, 502, 503, 504}. + */ + public static final Set DEFAULT_RETRYABLE_STATUSES = Set.of(429, 500, 502, 503, 504); + + /** + * Default set of exceptions that trigger a retry: {@link IOException} (and all subclasses). + */ + public static final Set> DEFAULT_RETRYABLE_EXCEPTIONS = Set.of(IOException.class); + + private static final RetryConfig DISABLED = new RetryConfig( + BackoffPolicy.noBackoff(), DEFAULT_RETRYABLE_STATUSES, DEFAULT_RETRYABLE_EXCEPTIONS + ); + + private final BackoffPolicy backoffPolicy; + private final Set retryableStatuses; + private final Set> retryableExceptions; + + private RetryConfig(BackoffPolicy backoffPolicy, Set retryableStatuses, + Set> retryableExceptions) { + this.backoffPolicy = Objects.requireNonNull(backoffPolicy, "backoffPolicy"); + this.retryableStatuses = retryableStatuses; + this.retryableExceptions = retryableExceptions; + } + + private RetryConfig(Builder builder) { + this(builder.backoffPolicy, builder.retryableStatuses, builder.retryableExceptions); + } + + /** + * Disabled retry configuration, equivalent to {@link BackoffPolicy#noBackoff()} with the default status set. + * No retries will happen. + */ + public static RetryConfig disabled() { + return DISABLED; + } + + public static Builder builder() { + return new Builder(); + } + + public static RetryConfig of(Consumer fn) { + Builder b = new Builder(); + fn.accept(b); + return b.build(); + } + + /** + * The backoff policy controlling delays and the maximum number of retry attempts. {@link BackoffPolicy#noBackoff()} + * means no retries are performed. + */ + public BackoffPolicy backoffPolicy() { + return backoffPolicy; + } + + public boolean isEnabled() { + return backoffPolicy != BackoffPolicy.noBackoff(); + } + + /** + * The HTTP status codes that should trigger a retry. Defaults to {@link #DEFAULT_RETRYABLE_STATUSES}. + */ + public Set retryableStatuses() { + return retryableStatuses; + } + + /** + * The exception types that should trigger a retry. Wrapped exceptions are also considered. + * Defaults to {@link #DEFAULT_RETRYABLE_EXCEPTIONS}. + */ + public Set> retryableExceptions() { + return retryableExceptions; + } + + + public Builder toBuilder() { + return new Builder() + .backoffPolicy(backoffPolicy) + .retryableStatuses(retryableStatuses) + .retryableExceptions(retryableExceptions); + } + + public static final class Builder { + // Null means "not set", which build() rejects: enabling retries requires an explicit backoff policy, + // and a config accidentally built without one must fail fast rather than silently never retry. + @Nullable + private BackoffPolicy backoffPolicy; + private Set retryableStatuses = DEFAULT_RETRYABLE_STATUSES; + private Set> retryableExceptions = DEFAULT_RETRYABLE_EXCEPTIONS; + + /** + * Sets the backoff policy controlling delays and the maximum number of retry attempts. Required. + * Passing {@link BackoffPolicy#noBackoff()} (or {@code null}) explicitly disables retries, like + * {@link RetryConfig#disabled()}. + */ + public Builder backoffPolicy(@Nullable BackoffPolicy policy) { + this.backoffPolicy = policy == null ? BackoffPolicy.noBackoff() : policy; + return this; + } + + /** + * Sets the HTTP status codes that should trigger a retry. Pass an empty set to disable status-based retries. + */ + public Builder retryableStatuses(Set statuses) { + this.retryableStatuses = Set.copyOf(Objects.requireNonNull(statuses, "retryableStatuses")); + return this; + } + + public Builder retryableStatuses(Integer... statuses) { + return retryableStatuses(Set.of(statuses)); + } + + /** + * Sets the exception types that should trigger a retry. Pass an empty set to disable exception-based retries. + */ + public Builder retryableExceptions(Set> exceptions) { + this.retryableExceptions = Set.copyOf(Objects.requireNonNull(exceptions, "retryableExceptions")); + return this; + } + + @SafeVarargs + public final Builder retryableExceptions(Class... exceptions) { + return retryableExceptions(Set.of(exceptions)); + } + + public RetryConfig build() { + if (backoffPolicy == null) { + throw new IllegalStateException( + "A backoff policy is required to enable retries: set RetryConfig.Builder#backoffPolicy," + + " e.g. BackoffPolicy.exponentialBackoff(). To explicitly disable retries, use" + + " RetryConfig.disabled() or BackoffPolicy.noBackoff()." + ); + } + return new RetryConfig(this); + } + } +} diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java index f5603b9b3d..8fab860828 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java @@ -47,6 +47,14 @@ public interface TransportOptions { */ boolean keepResponseBodyOnException(); + /** + * Configuration for transport-level retries (I/O exceptions and configurable HTTP status codes). + * Defaults to {@link RetryConfig#disabled()}, i.e. no retries. + */ + default RetryConfig retryConfig() { + return RetryConfig.disabled(); + } + Builder toBuilder(); default TransportOptions with(Consumer fn) { @@ -75,5 +83,19 @@ interface Builder extends ObjectBuilder { * streamed by the http library. */ Builder keepResponseBodyOnException(boolean value); + + /** + * Set the configuration for transport-level retries. + */ + default Builder retryConfig(RetryConfig config) { + throw new UnsupportedOperationException("This implementation does not support setting a retry configuration"); + } + + /** + * Convenience overload to configure transport-level retries with a builder lambda. + */ + default Builder retryConfig(Consumer fn) { + return retryConfig(RetryConfig.of(fn)); + } } } diff --git a/java-client/src/main/java/co/elastic/clients/transport/http/RetryingHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/http/RetryingHttpClient.java new file mode 100644 index 0000000000..f9a248274f --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/transport/http/RetryingHttpClient.java @@ -0,0 +1,360 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.transport.http; + +import co.elastic.clients.transport.RetryConfig; +import co.elastic.clients.transport.TransportOptions; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.util.Iterator; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * A {@link TransportHttpClient} wrapper that retries failed requests according to the {@link RetryConfig} + * carried by each request's {@link TransportOptions}. + *

+ * The retry configuration is read from the {@link TransportOptions} passed to every call (see + * {@link TransportOptions#retryConfig()}), so it can be set globally on the client or overridden per request, + * exactly like headers or query parameters. When the effective configuration is disabled + * ({@link RetryConfig#isEnabled()} returns {@code false}), the call is forwarded to the delegate unchanged. + *

+ * A request is retried when: + *

    + *
  • the delegate returns a response — or fails with an exception that the delegate recognizes as carrying + * a response, see {@link TransportHttpClient#responseStatusCode(Throwable)} — whose status code is in + * {@link RetryConfig#retryableStatuses()}, or
  • + *
  • the delegate fails with an exception matching {@link RetryConfig#retryableExceptions()}. Exceptions + * carrying a response are always classified by their status code, never by exception type.
  • + *
+ *

+ * Notes: + *

    + *
  • Retries assume the underlying request is safe to repeat. Most Elasticsearch APIs are idempotent at the user + * level, but users issuing custom non-idempotent operations should think twice before enabling retries.
  • + *
  • Retries reissue the same logical request to the {@code delegate}. Node selection (and any node-rotation / + * dead-node tracking) is the delegate's responsibility: this wrapper does not influence which node a retried + * request is sent to.
  • + *
  • When retries are enabled, blocking requests are executed through the delegate's asynchronous path, so + * exception stack traces reflect the async machinery rather than the calling thread.
  • + *
  • Backoff delays are scheduled on a {@link ScheduledExecutorService}. A single daemon-thread + * scheduler is created with this client and shut down by {@link #close()}; its thread is started lazily on + * the first retry and stopped while idle. A scheduler can also be supplied via + * {@link #RetryingHttpClient(TransportHttpClient, ScheduledExecutorService)}, in which case its lifecycle is + * the caller's responsibility.
  • + *
+ */ +public final class RetryingHttpClient implements TransportHttpClient { + + private static final Log logger = LogFactory.getLog(RetryingHttpClient.class); + + // Instance counter, to name the retry thread + private static final AtomicInteger idCounter = new AtomicInteger(); + + // Idle time (ms) after which the retry worker thread is stopped. During waits longer than this the worker + // wakes up once per interval, which is cheap; there is no need to track the backoff delays in use. + private static final long RETRY_SCHEDULER_KEEP_ALIVE_MS = 10_000L; + + private final TransportHttpClient delegate; + + // The scheduler used to defer retries. Its (single, daemon) worker thread is only started on the first + // scheduled retry and is stopped while idle (see defaultRetryScheduler), so a client that isn't actively + // retrying holds no retry thread. + private final ScheduledExecutorService retryScheduler; + // Same instance as retryScheduler when we created it ourselves, null for a user-supplied scheduler, whose + // lifecycle is the caller's responsibility. Used to shut down a scheduler we own on close(). + @Nullable + private final ScheduledThreadPoolExecutor managedScheduler; + + public RetryingHttpClient(TransportHttpClient delegate) { + this.delegate = Objects.requireNonNull(delegate, "delegate"); + ScheduledThreadPoolExecutor scheduler = defaultRetryScheduler(); + this.retryScheduler = scheduler; + this.managedScheduler = scheduler; + } + + /** + * Build a retrying client using a user-provided scheduler. The external scheduler won't be shut down or + * reconfigured by this client. + */ + public RetryingHttpClient(TransportHttpClient delegate, ScheduledExecutorService scheduler) { + this.delegate = Objects.requireNonNull(delegate, "delegate"); + this.retryScheduler = Objects.requireNonNull(scheduler, "scheduler"); + this.managedScheduler = null; + } + + @Override + public TransportOptions createOptions(@Nullable TransportOptions options) { + return delegate.createOptions(options); + } + + @Nullable + @Override + public Integer responseStatusCode(Throwable exception) { + return delegate.responseStatusCode(exception); + } + + @Override + public Response performRequest(String endpointId, @Nullable Node node, Request request, + TransportOptions options) throws IOException { + // No retries configured for this request: forward to the delegate unchanged (no async indirection). + if (!configOf(options).isEnabled()) { + return delegate.performRequest(endpointId, node, request, options); + } + CompletableFuture future = performRequestAsync(endpointId, node, request, options); + try { + return future.get(); + } catch (InterruptedException ie) { + // The caller has given up: stop the in-flight attempt and any pending retries, so they don't + // keep running (and holding the request body and a connection) with nobody waiting. + future.cancel(true); + Thread.currentThread().interrupt(); + throw new RuntimeException("thread waiting for the response was interrupted", ie); + } catch (ExecutionException ee) { + Throwable cause = ee.getCause(); + if (cause instanceof IOException) { + throw (IOException) cause; + } + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; + } + if (cause instanceof Error) { + throw (Error) cause; + } + throw new IOException(cause); + } + } + + @Override + public CompletableFuture performRequestAsync(String endpointId, @Nullable Node node, Request request, + TransportOptions options) { + RetryConfig config = configOf(options); + if (!config.isEnabled()) { + return delegate.performRequestAsync(endpointId, node, request, options); + } + RetryFuture result = new RetryFuture(); + attemptAsync(endpointId, node, request, options, config, config.backoffPolicy().iterator(), result); + return result; + } + + private void attemptAsync(String endpointId, @Nullable Node node, Request request, TransportOptions options, + RetryConfig config, Iterator backoffIter, RetryFuture result) { + if (result.isDone()) { + return; + } + + CompletableFuture attempt = delegate.performRequestAsync(endpointId, node, request, options); + result.setInFlight(attempt); + + attempt.whenComplete((resp, err) -> { + // Early return if request was cancelled + if (result.isCancelled()) { + if (resp != null) { + closeQuietly(resp); + } + return; + } + result.clearInFlight(); + + // Classify the outcome + Throwable cause = err != null ? unwrap(err) : null; + boolean retryable = cause != null + ? isRetryableFailure(config, cause) + : isRetryableStatus(config, resp); + + // Not retryable, or no retries left: complete with whatever we got + if (!retryable || !backoffIter.hasNext()) { + if (retryable) { + logger.warn("Retries exhausted for [" + endpointId + "]"); + } + if (cause != null) { + result.completeExceptionally(cause); + } else if (!result.complete(resp)) { + // The future was already completed/cancelled elsewhere: avoid leaking the response + closeQuietly(resp); + } + return; + } + + // Retryable with budget left: close any response before triggering a retry + if (resp != null) { + closeQuietly(resp); + } + + long delayMs = backoffIter.next(); + if (logger.isDebugEnabled()) { + logger.debug("Retrying [" + endpointId + "] in " + delayMs + " ms"); + } + + try { + ScheduledFuture scheduled = retryScheduler.schedule( + () -> attemptAsync(endpointId, node, request, options, config, backoffIter, result), + delayMs, TimeUnit.MILLISECONDS + ); + result.setScheduledRetry(scheduled); + } catch (RejectedExecutionException rex) { + result.completeExceptionally(cause != null ? cause : rex); + } + }); + } + + @Override + public void close() throws IOException { + try { + delegate.close(); + } finally { + // Only shut down a scheduler we created; a user-supplied one is the caller's responsibility. + if (managedScheduler != null) { + managedScheduler.shutdownNow(); + } + } + } + + /** + * Decides whether a failed attempt is retryable. Failures that the delegate recognizes as carrying an http + * response (see {@link TransportHttpClient#responseStatusCode(Throwable)}) are classified by their status + * code only: the server did answer, so the exception type must not make an otherwise non-retryable status + * retryable. Other failures are classified by exception type. + */ + boolean isRetryableFailure(RetryConfig config, Throwable err) { + Integer status = delegate.responseStatusCode(err); + if (status == null && err.getCause() != null) { + status = delegate.responseStatusCode(err.getCause()); + } + if (status != null) { + return config.retryableStatuses().contains(status); + } + return isRetryableException(config, err); + } + + boolean isRetryableException(RetryConfig config, Throwable err) { + return matchesRetryable(config, err) || matchesRetryable(config, err.getCause()); + } + + private boolean matchesRetryable(RetryConfig config, @Nullable Throwable t) { + if (t == null) { + return false; + } + for (Class ex : config.retryableExceptions()) { + if (ex.isInstance(t)) { + return true; + } + } + return false; + } + + boolean isRetryableStatus(RetryConfig config, Response resp) { + return config.retryableStatuses().contains(resp.statusCode()); + } + + private static RetryConfig configOf(@Nullable TransportOptions options) { + RetryConfig config = options == null ? null : options.retryConfig(); + return config == null ? RetryConfig.disabled() : config; + } + + private static Throwable unwrap(Throwable t) { + while (t instanceof CompletionException && t.getCause() != null && t.getCause() != t) { + t = t.getCause(); + } + return t; + } + + private static void closeQuietly(Response resp) { + try { + resp.close(); + } catch (IOException ignored) { + // ignored + } + } + + private static ScheduledThreadPoolExecutor defaultRetryScheduler() { + int clientId = idCounter.incrementAndGet(); + ScheduledThreadPoolExecutor scheduler = new ScheduledThreadPoolExecutor(1, r -> { + Thread t = Executors.defaultThreadFactory().newThread(r); + t.setName("elasticsearch-java-retry#" + clientId + "#" + t.getId()); + t.setDaemon(true); + return t; + }); + // Don't keep an idle worker parked between (potentially infrequent) retry sequences: the thread is started + // on the first scheduled retry, stopped once it has been idle for the keep-alive window, and recreated on + // demand. A pending scheduled retry always keeps the worker alive (the pool never stops its last worker + // while the queue is non-empty), so an in-progress sequence is never affected. + scheduler.setKeepAliveTime(RETRY_SCHEDULER_KEEP_ALIVE_MS, TimeUnit.MILLISECONDS); + scheduler.allowCoreThreadTimeOut(true); + // Cancelled retries (e.g. after request cancellation) are removed from the queue immediately so they + // don't keep the worker alive or delay it being stopped. + scheduler.setRemoveOnCancelPolicy(true); + return scheduler; + } + + /** + * Result future that propagates cancellation to both the in-flight delegate request and any pending scheduled retry. + */ + private static final class RetryFuture extends CompletableFuture { + private volatile CompletableFuture inFlight; + private volatile ScheduledFuture scheduledRetry; + + void setInFlight(CompletableFuture future) { + this.inFlight = future; + if (isCancelled()) { + future.cancel(true); + } + } + + void clearInFlight() { + this.inFlight = null; + } + + void setScheduledRetry(ScheduledFuture scheduledFuture) { + this.scheduledRetry = scheduledFuture; + if (isCancelled()) { + scheduledFuture.cancel(false); + } + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + boolean cancelled = super.cancel(mayInterruptIfRunning); + if (cancelled) { + CompletableFuture future = inFlight; + if (future != null) { + future.cancel(mayInterruptIfRunning); + } + ScheduledFuture scheduledFuture = scheduledRetry; + if (scheduledFuture != null) { + scheduledFuture.cancel(false); + } + } + return cancelled; + } + } +} diff --git a/java-client/src/main/java/co/elastic/clients/transport/http/TransportHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/http/TransportHttpClient.java index 4a8b3ffa33..3f587e08d0 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/http/TransportHttpClient.java +++ b/java-client/src/main/java/co/elastic/clients/transport/http/TransportHttpClient.java @@ -50,6 +50,20 @@ default TransportOptions createOptions(@Nullable TransportOptions options) { return options == null ? DefaultTransportOptions.EMPTY : options; } + /** + * If the given exception carries an http response — like the low-level clients' {@code ResponseException}, + * which reports non-2xx statuses as an exception rather than as a response — returns that response's status + * code, otherwise {@code null}. + *

+ * This allows implementation-agnostic wrappers (e.g. {@link RetryingHttpClient}) to classify such failures + * by status code without knowing the underlying http library. Implementations should override this for + * their own response-carrying exception types. + */ + @Nullable + default Integer responseStatusCode(Throwable exception) { + return null; + } + /** * Perform a blocking request. * diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientHttpClient.java index a8649fe65c..e401822749 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientHttpClient.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientHttpClient.java @@ -23,6 +23,7 @@ import co.elastic.clients.transport.http.HeaderMap; import co.elastic.clients.transport.http.TransportHttpClient; import co.elastic.clients.transport.rest5_client.low_level.Cancellable; +import co.elastic.clients.transport.rest5_client.low_level.ResponseException; import co.elastic.clients.transport.rest5_client.low_level.ResponseListener; import co.elastic.clients.transport.rest5_client.low_level.Rest5Client; import co.elastic.clients.util.BinaryData; @@ -85,6 +86,15 @@ public Rest5ClientOptions createOptions(@Nullable TransportOptions options) { return Rest5ClientOptions.of(options); } + @Nullable + @Override + public Integer responseStatusCode(Throwable exception) { + if (exception instanceof ResponseException) { + return ((ResponseException) exception).getResponse().getStatusCode(); + } + return null; + } + @Override public Response performRequest(String endpointId, @Nullable Node node, Request request, TransportOptions options) throws IOException { diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientOptions.java b/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientOptions.java index 2520b16bb7..6f4f745297 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest5_client/Rest5ClientOptions.java @@ -19,6 +19,7 @@ package co.elastic.clients.transport.rest5_client; +import co.elastic.clients.transport.RetryConfig; import co.elastic.clients.transport.TransportOptions; import co.elastic.clients.transport.Version; import co.elastic.clients.transport.http.HeaderMap; @@ -44,6 +45,8 @@ public class Rest5ClientOptions implements TransportOptions { boolean keepResponseBodyOnException; + private final RetryConfig retryConfig; + @VisibleForTesting static final String CLIENT_META_VALUE = getClientMeta(); @VisibleForTesting @@ -55,18 +58,26 @@ public static Rest5ClientOptions of(@Nullable TransportOptions options) { } if (options instanceof Rest5ClientOptions) { - return (Rest5ClientOptions)options; + return (Rest5ClientOptions) options; } final Builder builder = new Builder(RequestOptions.DEFAULT.toBuilder()); options.headers().forEach(h -> builder.addHeader(h.getKey(), h.getValue())); options.queryParameters().forEach(builder::setParameter); builder.onWarnings(options.onWarnings()); + builder.keepResponseBodyOnException(options.keepResponseBodyOnException()); + builder.retryConfig(options.retryConfig()); return builder.build(); } public Rest5ClientOptions(RequestOptions options, boolean keepResponseBodyOnException) { + this(options, keepResponseBodyOnException, RetryConfig.disabled()); + } + + public Rest5ClientOptions(RequestOptions options, boolean keepResponseBodyOnException, + RetryConfig retryConfig) { this.keepResponseBodyOnException = keepResponseBodyOnException; + this.retryConfig = retryConfig == null ? RetryConfig.disabled() : retryConfig; this.options = addBuiltinHeaders(options.toBuilder()).build(); } @@ -112,9 +123,17 @@ public boolean keepResponseBodyOnException() { return this.keepResponseBodyOnException; } + @Override + public RetryConfig retryConfig() { + return retryConfig; + } + @Override public Builder toBuilder() { - return new Builder(options.toBuilder()); + Builder b = new Builder(options.toBuilder()); + b.keepResponseBodyOnException(keepResponseBodyOnException); + b.retryConfig(retryConfig); + return b; } public static class Builder implements TransportOptions.Builder { @@ -123,6 +142,8 @@ public static class Builder implements TransportOptions.Builder { private boolean keepResponseBodyOnException; + private RetryConfig retryConfig = RetryConfig.disabled(); + public Builder(RequestOptions.Builder builder) { this.builder = builder; } @@ -141,7 +162,8 @@ public TransportOptions.Builder addHeader(String name, String value) { return this; } if (name.equalsIgnoreCase(HeaderMap.USER_AGENT)) { - // We must remove our own user-agent from the options, or we'll end up with multiple values for the header + // We must remove our own user-agent from the options, or we'll end up with multiple values + // for the header builder.removeHeader(HeaderMap.USER_AGENT); } builder.addHeader(name, value); @@ -173,7 +195,8 @@ public TransportOptions.Builder setParameter(String name, String value) { @Override public TransportOptions.Builder removeParameter(String name) { - throw new UnsupportedOperationException("This implementation does not support removing parameters"); + throw new UnsupportedOperationException("This implementation does not support removing " + + "parameters"); } /** @@ -202,9 +225,16 @@ public TransportOptions.Builder keepResponseBodyOnException(boolean value) { return this; } + @Override + public TransportOptions.Builder retryConfig(RetryConfig config) { + this.retryConfig = config == null ? RetryConfig.disabled() : config; + return this; + } + @Override public Rest5ClientOptions build() { - return new Rest5ClientOptions(addBuiltinHeaders(builder).build(), keepResponseBodyOnException); + return new Rest5ClientOptions(addBuiltinHeaders(builder).build(), keepResponseBodyOnException, + retryConfig); } } diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java index 85b92c1070..7d2165c5d6 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java @@ -30,6 +30,7 @@ import org.apache.http.entity.ContentType; import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Cancellable; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.ResponseListener; import org.elasticsearch.client.RestClient; @@ -84,6 +85,15 @@ public RestClientOptions createOptions(@Nullable TransportOptions options) { return RestClientOptions.of(options); } + @Nullable + @Override + public Integer responseStatusCode(Throwable exception) { + if (exception instanceof ResponseException) { + return ((ResponseException) exception).getResponse().getStatusLine().getStatusCode(); + } + return null; + } + @Override public Response performRequest(String endpointId, @Nullable Node node, Request request, TransportOptions options) throws IOException { diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java index de1658e26a..431a9cbd44 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java @@ -19,6 +19,7 @@ package co.elastic.clients.transport.rest_client; +import co.elastic.clients.transport.RetryConfig; import co.elastic.clients.transport.TransportOptions; import co.elastic.clients.transport.Version; import co.elastic.clients.transport.http.HeaderMap; @@ -44,6 +45,8 @@ public class RestClientOptions implements TransportOptions { boolean keepResponseBodyOnException; + private final RetryConfig retryConfig; + @VisibleForTesting static final String CLIENT_META_VALUE = getClientMeta(); @VisibleForTesting @@ -62,11 +65,18 @@ public static RestClientOptions of(@Nullable TransportOptions options) { options.headers().forEach(h -> builder.addHeader(h.getKey(), h.getValue())); options.queryParameters().forEach(builder::setParameter); builder.onWarnings(options.onWarnings()); + builder.keepResponseBodyOnException(options.keepResponseBodyOnException()); + builder.retryConfig(options.retryConfig()); return builder.build(); } public RestClientOptions(RequestOptions options, boolean keepResponseBodyOnException) { + this(options, keepResponseBodyOnException, RetryConfig.disabled()); + } + + public RestClientOptions(RequestOptions options, boolean keepResponseBodyOnException, RetryConfig retryConfig) { this.keepResponseBodyOnException = keepResponseBodyOnException; + this.retryConfig = retryConfig == null ? RetryConfig.disabled() : retryConfig; this.options = addBuiltinHeaders(options.toBuilder()).build(); } @@ -112,9 +122,17 @@ public boolean keepResponseBodyOnException() { return this.keepResponseBodyOnException; } + @Override + public RetryConfig retryConfig() { + return retryConfig; + } + @Override public Builder toBuilder() { - return new Builder(options.toBuilder()); + Builder b = new Builder(options.toBuilder()); + b.keepResponseBodyOnException(keepResponseBodyOnException); + b.retryConfig(retryConfig); + return b; } public static class Builder implements TransportOptions.Builder { @@ -123,6 +141,8 @@ public static class Builder implements TransportOptions.Builder { private boolean keepResponseBodyOnException; + private RetryConfig retryConfig = RetryConfig.disabled(); + public Builder(RequestOptions.Builder builder) { this.builder = builder; } @@ -202,9 +222,15 @@ public TransportOptions.Builder keepResponseBodyOnException(boolean value) { return this; } + @Override + public TransportOptions.Builder retryConfig(RetryConfig config) { + this.retryConfig = config == null ? RetryConfig.disabled() : config; + return this; + } + @Override public RestClientOptions build() { - return new RestClientOptions(addBuiltinHeaders(builder).build(), keepResponseBodyOnException); + return new RestClientOptions(addBuiltinHeaders(builder).build(), keepResponseBodyOnException, retryConfig); } } diff --git a/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportConfigTest.java b/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportConfigTest.java index d4a3f48e0f..13ece3fd2d 100644 --- a/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportConfigTest.java +++ b/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportConfigTest.java @@ -130,6 +130,35 @@ public void credentialCombinations() { ); } + @Test + public void retryConfigShortcut() { + // Top-level shortcut on the main builder + ElasticsearchClient client = ElasticsearchClient.of(b -> b + .host("http://example.com") + .retryConfig(r -> r + .backoffPolicy(BackoffPolicy.constantBackoff(50L, 3)) + .retryableStatuses(429, 503)) + ); + + RetryConfig cfg = client._transportOptions().retryConfig(); + assertNotEquals(BackoffPolicy.noBackoff(), cfg.backoffPolicy()); + assertEquals(BackoffPolicy.constantBackoff(50L, 3).toString(), cfg.backoffPolicy().toString()); + assertEquals(java.util.Set.of(429, 503), cfg.retryableStatuses()); + } + + @Test + public void retryConfigShortcutComposesWithTransportOptions() { + // Mix the top-level shortcut with other transportOptions calls + ElasticsearchClient client = ElasticsearchClient.of(b -> b + .host("http://example.com") + .transportOptions(o -> o.keepResponseBodyOnException(true)) + .retryConfig(r -> r.backoffPolicy(BackoffPolicy.constantBackoff(10L, 2))) + ); + + assertTrue(client._transportOptions().keepResponseBodyOnException()); + assertNotEquals(BackoffPolicy.noBackoff(), client._transportOptions().retryConfig().backoffPolicy()); + } + @Test public void checkDefaultConfig() throws Exception { ElasticsearchTransportConfig.Default config = new ElasticsearchTransportConfig.Builder() diff --git a/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportRetryTest.java b/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportRetryTest.java new file mode 100644 index 0000000000..5204d72b30 --- /dev/null +++ b/java-client/src/test/java/co/elastic/clients/transport/ElasticsearchTransportRetryTest.java @@ -0,0 +1,277 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.transport; + +import co.elastic.clients.elasticsearch.core.CountRequest; +import co.elastic.clients.elasticsearch.core.CountResponse; +import co.elastic.clients.testkit.MockHttpClient; +import co.elastic.clients.testkit.ModelTestCase; +import co.elastic.clients.transport.http.RetryingHttpClient; +import co.elastic.clients.transport.http.TransportHttpClient; +import co.elastic.clients.util.BinaryData; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Verifies how {@link ElasticsearchTransportBase} routes requests depending on where retries are configured: + *

    + *
  • enabled at the client level, and
  • + *
  • enabled only for a single request on an otherwise non-retrying client.
  • + *
+ * In both cases requests go through a single, lazily-created, shared retry wrapper; requests without retries go + * straight to the bare http client. + */ +public class ElasticsearchTransportRetryTest extends ModelTestCase { + + private static final BackoffPolicy FAST = BackoffPolicy.constantBackoff(1L, 3); + + private static TransportOptions retryOptions() { + return new DefaultTransportOptions.Builder() + .retryConfig(RetryConfig.of(r -> r.backoffPolicy(FAST))) + .build(); + } + + private static TransportOptions disabledRetryOptions() { + return new DefaultTransportOptions.Builder() + .retryConfig(RetryConfig.disabled()) + .build(); + } + + // An http client that returns `failuresBeforeSuccess` retryable 503 responses, then a valid 200 _count response. + private ScriptedHttpClient scriptedClient(int failuresBeforeSuccess) { + ScriptedHttpClient client = new ScriptedHttpClient(failuresBeforeSuccess); + client.add("/_count", "application/json", toJson(CountResponse.of(c -> c + .count(1) + .shards(s -> s.successful(1).failed(0).total(1)) + ))); + return client; + } + + private ElasticsearchTransportBase transport(MockHttpClient httpClient, TransportOptions clientOptions) { + return new ElasticsearchTransportBase(httpClient, clientOptions, mapper) {}; + } + + @Test + public void clientLevelRetryRetriesAndSucceeds() throws IOException { + ScriptedHttpClient http = scriptedClient(2); + ElasticsearchTransportBase transport = transport(http, retryOptions()); + + // No per-request options: the client-level RetryConfig (carried by transportOptions) applies. + CountResponse resp = transport.performRequest(CountRequest.of(c -> c), CountRequest._ENDPOINT, null); + + assertEquals(1L, resp.count()); + assertEquals(3, http.calls.get()); + } + + @Test + public void perRequestRetryRetriesAndSucceedsSync() throws IOException { + ScriptedHttpClient http = scriptedClient(1); + // Client has NO retries: the retry is requested only for this call. + ElasticsearchTransportBase transport = transport(http, null); + + CountResponse resp = transport.performRequest(CountRequest.of(c -> c), CountRequest._ENDPOINT, retryOptions()); + + assertEquals(1L, resp.count()); + assertEquals(2, http.calls.get()); + } + + @Test + public void perRequestRetryRetriesAndSucceedsAsync() throws Exception { + ScriptedHttpClient http = scriptedClient(1); + ElasticsearchTransportBase transport = transport(http, null); + + CompletableFuture future = + transport.performRequestAsync(CountRequest.of(c -> c), CountRequest._ENDPOINT, retryOptions()); + CountResponse resp = future.get(5, TimeUnit.SECONDS); + + assertEquals(1L, resp.count()); + assertEquals(2, http.calls.get()); + } + + @Test + public void noRetryConfiguredMakesSingleAttempt() { + ScriptedHttpClient http = scriptedClient(5); + ElasticsearchTransportBase transport = transport(http, null); + + // Neither client nor request enable retries: the 503 surfaces immediately. + assertThrows(TransportException.class, + () -> transport.performRequest(CountRequest.of(c -> c), CountRequest._ENDPOINT, null)); + assertEquals(1, http.calls.get()); + } + + /** + * Cancelling a per-request retrying call must stop pending retries and not call the delegate again, + * without surfacing spurious exceptions. + */ + @Test + public void perRequestRetryAsyncCancellationStopsRetries() throws Exception { + AtomicInteger calls = new AtomicInteger(); + CountDownLatch firstCall = new CountDownLatch(1); + + TransportHttpClient delegate = new TransportHttpClient() { + @Override + public Response performRequest(String endpointId, Node node, Request request, TransportOptions options) { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture performRequestAsync(String endpointId, Node node, Request request, + TransportOptions options) { + calls.incrementAndGet(); + CompletableFuture f = new CompletableFuture<>(); + f.complete(new StatusResponse(503)); + firstCall.countDown(); + return f; + } + + @Override + public void close() { + } + }; + + ElasticsearchTransportBase transport = new ElasticsearchTransportBase(delegate, null, mapper) {}; + + // Long backoff so the retry stays pending until we cancel. + TransportOptions longRetry = new DefaultTransportOptions.Builder() + .retryConfig(RetryConfig.of(r -> r.backoffPolicy(BackoffPolicy.constantBackoff(3_600_000L, 5)))) + .build(); + + CompletableFuture future = + transport.performRequestAsync(CountRequest.of(c -> c), CountRequest._ENDPOINT, longRetry); + + assertTrue(firstCall.await(5, TimeUnit.SECONDS)); + // Give the retry a moment to be scheduled before we cancel. + Thread.sleep(50); + future.cancel(true); + + assertThrows(CancellationException.class, () -> future.get(1, TimeUnit.SECONDS)); + + // No further attempts after cancellation. + Thread.sleep(100); + assertEquals(1, calls.get(), "delegate must not be called again after cancellation"); + } + + @Test + public void alreadyRetryingHttpClientIsNotWrappedAgain() throws IOException { + // If the http client handed to the transport already retries, the transport must not add a second + // retry layer: both layers would read the same RetryConfig and multiply attempts. + ScriptedHttpClient http = scriptedClient(100); + ElasticsearchTransportBase transport = + new ElasticsearchTransportBase(new RetryingHttpClient(http), retryOptions(), mapper) {}; + + assertThrows(TransportException.class, + () -> transport.performRequest(CountRequest.of(c -> c), CountRequest._ENDPOINT, null)); + + // FAST allows 3 retries: 4 attempts with a single retry layer, 16 with an accidental double layer. + assertEquals(4, http.calls.get()); + transport.close(); + } + + @Test + public void perRequestDisableOverridesClientRetry() { + ScriptedHttpClient http = scriptedClient(5); + // Client enables retries, but this specific request opts out. + ElasticsearchTransportBase transport = transport(http, retryOptions()); + + assertThrows(TransportException.class, + () -> transport.performRequest(CountRequest.of(c -> c), CountRequest._ENDPOINT, disabledRetryOptions())); + assertEquals(1, http.calls.get()); + } + + static class ScriptedHttpClient extends MockHttpClient { + final AtomicInteger calls = new AtomicInteger(); + private final int failuresBeforeSuccess; + + ScriptedHttpClient(int failuresBeforeSuccess) { + this.failuresBeforeSuccess = failuresBeforeSuccess; + } + + @Override + public Response performRequest(String endpointId, Node node, Request request, TransportOptions option) + throws IOException { + int i = calls.getAndIncrement(); + if (i < failuresBeforeSuccess) { + return new StatusResponse(503); + } + return super.performRequest(endpointId, node, request, option); + } + + @Override + public CompletableFuture performRequestAsync(String endpointId, Node node, Request request, + TransportOptions options) { + CompletableFuture f = new CompletableFuture<>(); + try { + f.complete(performRequest(endpointId, node, request, options)); + } catch (Exception e) { + f.completeExceptionally(e); + } + return f; + } + } + + static final class StatusResponse implements TransportHttpClient.Response { + private final int status; + + StatusResponse(int status) { + this.status = status; + } + + @Override + public TransportHttpClient.Node node() { + return null; + } + + @Override + public int statusCode() { + return status; + } + + @Override + public String header(String name) { + return null; + } + + @Override + public List headers(String name) { + return null; + } + + @Override + public BinaryData body() { + return null; + } + + @Override + public Object originalResponse() { + return null; + } + + @Override + public void close() { + } + } +} diff --git a/java-client/src/test/java/co/elastic/clients/transport/TransportRetryStatusTest.java b/java-client/src/test/java/co/elastic/clients/transport/TransportRetryStatusTest.java new file mode 100644 index 0000000000..dc0ad08e70 --- /dev/null +++ b/java-client/src/test/java/co/elastic/clients/transport/TransportRetryStatusTest.java @@ -0,0 +1,138 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.transport; + +import co.elastic.clients.elasticsearch.ElasticsearchClient; +import co.elastic.clients.transport.rest5_client.Rest5ClientTransport; +import co.elastic.clients.transport.rest_client.RestClientTransport; +import com.sun.net.httpserver.HttpServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.SocketException; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; + +/** + * Verifies status-based retries through the real http clients. Both low-level clients surface + * retryable statuses as {@code ResponseException} rather than as a plain response (the rest5 client for 5xx, + * the legacy client for every non-ignored error status), so these tests ensure the retry classification + * extracts status codes from those exceptions instead of matching them as generic {@link IOException}s. + * Exception-based retries are deliberately narrowed or disabled in these tests: they must not be the reason + * a request is retried. + */ +public class TransportRetryStatusTest extends Assertions { + + private HttpServer server; + private final AtomicInteger hits = new AtomicInteger(); + + @BeforeEach + public void setUp() throws IOException { + server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + server.start(); + } + + @AfterEach + public void tearDown() { + server.stop(0); + } + + // Responds to ping (HEAD /) with the scripted statuses, then 200 for any further request. + private String scriptedPingServer(int... statuses) { + server.createContext("/", exchange -> { + int i = hits.getAndIncrement(); + int status = i < statuses.length ? statuses[i] : 200; + if (status == 200) { + exchange.getResponseHeaders().set("X-Elastic-Product", "Elasticsearch"); + } + exchange.sendResponseHeaders(status, -1); + exchange.close(); + }); + var address = server.getAddress(); + return "http://" + address.getHostString() + ":" + address.getPort(); + } + + private ElasticsearchClient client( + String url, + Function transportFactory, + RetryConfig retryConfig + ) { + return ElasticsearchClient.of(b -> b + .host(url) + .transportFactory(transportFactory) + .retryConfig(retryConfig) + ); + } + + @Test + public void rest5RetriesOn5xxStatusWithNarrowedExceptions() throws IOException { + // The rest5 client reports a 503 as a ResponseException. With exception retries narrowed to + // SocketException, only the status route can trigger the retries. + String url = scriptedPingServer(503, 503); + + RetryConfig config = RetryConfig.of(r -> r + .backoffPolicy(BackoffPolicy.constantBackoff(1L, 5)) + .retryableStatuses(503) + .retryableExceptions(SocketException.class)); + + try (ElasticsearchClient client = client(url, Rest5ClientTransport::new, config)) { + assertTrue(client.ping().value()); + } + assertEquals(3, hits.get()); + } + + @Test + public void rest5DoesNotRetryStatusesOutsideTheConfiguredSet() throws IOException { + // A 501 also surfaces as a ResponseException (an IOException). It must be classified by its status + // code - not in the default retryable set - and not retried as a generic IOException. + String url = scriptedPingServer(501); + + RetryConfig config = RetryConfig.of(r -> r.backoffPolicy(BackoffPolicy.constantBackoff(1L, 5))); + + try (ElasticsearchClient client = client(url, Rest5ClientTransport::new, config)) { + assertThrows(IOException.class, () -> client.ping()); + } + assertEquals(1, hits.get()); + } + + @Test + public void legacyRetriesOnStatusWithExceptionRetriesDisabled() throws IOException { + // The legacy client reports every non-ignored error status (429 included) as a ResponseException, + // which is accessed reflectively. Exception-based retries are fully disabled here: only the status + // route can trigger the retries. + String url = scriptedPingServer(429, 429); + + RetryConfig config = RetryConfig.of(r -> r + .backoffPolicy(BackoffPolicy.constantBackoff(1L, 5)) + .retryableStatuses(429) + .retryableExceptions(Set.of())); + + try (ElasticsearchClient client = client(url, RestClientTransport::new, config)) { + assertTrue(client.ping().value()); + } + assertEquals(3, hits.get()); + } +} diff --git a/java-client/src/test/java/co/elastic/clients/transport/http/RetryingHttpClientTest.java b/java-client/src/test/java/co/elastic/clients/transport/http/RetryingHttpClientTest.java new file mode 100644 index 0000000000..36c79257c2 --- /dev/null +++ b/java-client/src/test/java/co/elastic/clients/transport/http/RetryingHttpClientTest.java @@ -0,0 +1,546 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.transport.http; + +import co.elastic.clients.transport.BackoffPolicy; +import co.elastic.clients.transport.DefaultTransportOptions; +import co.elastic.clients.transport.RetryConfig; +import co.elastic.clients.transport.TransportOptions; +import co.elastic.clients.util.BinaryData; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.net.SocketException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; + +class RetryingHttpClientTest extends Assertions { + + private ScheduledExecutorService scheduler; + + @BeforeEach + void setUp() { + scheduler = Executors.newSingleThreadScheduledExecutor(r -> { + Thread t = new Thread(r, "retry-test-scheduler"); + t.setDaemon(true); + return t; + }); + } + + @AfterEach + void tearDown() { + scheduler.shutdownNow(); + } + + private static final TransportHttpClient.Request REQ = new TransportHttpClient.Request( + "GET", "/", Collections.emptyMap(), Collections.emptyMap(), null + ); + + private RetryingHttpClient wrap(TransportHttpClient delegate) { + return new RetryingHttpClient(delegate, scheduler); + } + + // The retry configuration is now read from each request's TransportOptions, so tests inject it there. + private static TransportOptions opts(RetryConfig config) { + return new DefaultTransportOptions.Builder().retryConfig(config).build(); + } + + private static TransportOptions opts(BackoffPolicy policy) { + return opts(RetryConfig.of(r -> r.backoffPolicy(policy))); + } + + private static BackoffPolicy fixed(long ms, int retries) { + return BackoffPolicy.constantBackoff(ms, retries); + } + + // A mock transport instance that returns the next scripted outcome on each call. + static class MockedClient implements TransportHttpClient { + final List> script = new ArrayList<>(); + final AtomicInteger calls = new AtomicInteger(); + + // Add a response outcome. + MockedClient andRespond(int statusCode) { + script.add(b -> new FakeResponse(statusCode, Collections.emptyMap())); + return this; + } + + MockedClient andThrow(Throwable t) { + script.add(b -> { throw new FakeException(t); }); + return this; + } + + @Override + public Response performRequest(String endpointId, @Nullable Node node, Request request, + TransportOptions options) throws IOException { + int i = calls.getAndIncrement(); + Function step = script.get(Math.min(i, script.size() - 1)); + try { + Object out = step.apply(true); + return (Response) out; + } catch (FakeException se) { + if (se.cause instanceof IOException) throw (IOException) se.cause; + if (se.cause instanceof RuntimeException) throw (RuntimeException) se.cause; + throw new RuntimeException(se.cause); + } + } + + @Override + public CompletableFuture performRequestAsync(String endpointId, @Nullable Node node, Request request, + TransportOptions options) { + int i = calls.getAndIncrement(); + Function step = script.get(Math.min(i, script.size() - 1)); + CompletableFuture f = new CompletableFuture<>(); + try { + Object out = step.apply(true); + f.complete((Response) out); + } catch (FakeException se) { + f.completeExceptionally(se.cause); + } + return f; + } + + @Override + public void close() {} + } + + static final class FakeException extends RuntimeException { + final Throwable cause; + FakeException(Throwable c) { this.cause = c; } + } + + static final class FakeResponse implements TransportHttpClient.Response { + private final int status; + private final java.util.Map headers; + boolean closed; + + FakeResponse(int status, java.util.Map headers) { + this.status = status; + this.headers = headers; + } + @Override public TransportHttpClient.Node node() { return new TransportHttpClient.Node("http://localhost/"); } + @Override public int statusCode() { return status; } + @Override public String header(String name) { return headers.get(name); } + @Override public List headers(String name) { + String v = headers.get(name); + return v == null ? Collections.emptyList() : Collections.singletonList(v); + } + @Override public BinaryData body() { return null; } + @Override public Object originalResponse() { return this; } + @Override public void close() { closed = true; } + } + + @Test + void syncRetriesOn5xxThenSucceeds() throws IOException { + MockedClient client = new MockedClient() + .andRespond(503).andRespond(502).andRespond(500).andRespond(200); + + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(fixed(1L, 5))); + + assertEquals(200, resp.statusCode()); + assertEquals(4, client.calls.get()); + } + + @Test + void syncRetriesOn429() throws IOException { + MockedClient client = new MockedClient().andRespond(429).andRespond(200); + + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(fixed(1L, 3))); + + assertEquals(200, resp.statusCode()); + assertEquals(2, client.calls.get()); + } + + @Test + void syncDoesNotRetryOn501() throws IOException { + MockedClient client = new MockedClient().andRespond(501).andRespond(200); + + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(fixed(1L, 3))); + + assertEquals(501, resp.statusCode()); + assertEquals(1, client.calls.get()); + } + + @Test + void customRetryableStatusesOnly() throws IOException { + // Configure to retry only on 418 - a 503 should pass through + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 3)) + .retryableStatuses(418)); + + MockedClient client = new MockedClient().andRespond(503).andRespond(200); + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(cfg)); + + assertEquals(503, resp.statusCode()); + assertEquals(1, client.calls.get()); + } + + @Test + void customRetryableStatusesIncludingNonStandard() throws IOException { + // Configure to retry on 408 (Request Timeout) - not in the default set + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 3)) + .retryableStatuses(408, 503)); + + MockedClient client = new MockedClient().andRespond(408).andRespond(200); + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(cfg)); + + assertEquals(200, resp.statusCode()); + assertEquals(2, client.calls.get()); + } + + @Test + void customRetryableExceptionsOnly() throws IOException { + // Narrow exception retries to SocketTimeoutException - a plain SocketException must not retry + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 3)) + .retryableExceptions(java.net.SocketTimeoutException.class)); + + MockedClient client = new MockedClient().andThrow(new SocketException("not a timeout")).andRespond(200); + IOException thrown = assertThrows(IOException.class, + () -> wrap(client).performRequest("ep", null, REQ, opts(cfg))); + assertEquals("not a timeout", thrown.getMessage()); + assertEquals(1, client.calls.get()); + } + + @Test + void emptyRetryableStatusesDisablesStatusRetries() throws IOException { + // Exception-based retries only - a 503 response should NOT trigger a retry + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 3)) + .retryableStatuses(new HashSet<>())); + + MockedClient client = new MockedClient().andRespond(503).andRespond(200); + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(cfg)); + + assertEquals(503, resp.statusCode()); + assertEquals(1, client.calls.get()); + } + + @Test + void emptyRetryableExceptionsDisablesExceptionRetries() { + // Status-based retries only - a SocketException should NOT trigger a retry + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 3)) + .retryableExceptions(new HashSet<>())); + + MockedClient client = new MockedClient().andThrow(new SocketException("io")).andRespond(200); + assertThrows(IOException.class, () -> wrap(client).performRequest("ep", null, REQ, opts(cfg))); + assertEquals(1, client.calls.get()); + } + + @Test + void retryConfigDefensivelyCopiesStatusSet() { + HashSet mutable = new HashSet<>(); + mutable.add(429); + mutable.add(503); + RetryConfig cfg = RetryConfig.of(r -> r.backoffPolicy(fixed(1L, 1)).retryableStatuses(mutable)); + + // Mutating the original set after building must not affect the config. + mutable.add(418); + assertFalse(cfg.retryableStatuses().contains(418)); + + // The returned set itself must be immutable. + assertThrows(UnsupportedOperationException.class, + () -> cfg.retryableStatuses().add(999)); + } + + @Test + void retryConfigRequiresABackoffPolicy() { + // Customizing retry conditions without choosing a backoff policy must fail fast instead of silently + // building a config that never retries. + assertThrows(IllegalStateException.class, + () -> RetryConfig.of(r -> r.retryableStatuses(429, 503))); + assertThrows(IllegalStateException.class, () -> RetryConfig.builder().build()); + + // Explicitly choosing no backoff remains a valid way to build a disabled config (per-request opt-out). + RetryConfig disabled = RetryConfig.of(r -> r.backoffPolicy(BackoffPolicy.noBackoff())); + assertFalse(disabled.isEnabled()); + assertFalse(RetryConfig.disabled().isEnabled()); + } + + @Test + void syncDoesNotRetryOn4xx() throws IOException { + MockedClient client = new MockedClient().andRespond(404).andRespond(200); + + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(fixed(1L, 3))); + + assertEquals(404, resp.statusCode()); + assertEquals(1, client.calls.get()); + } + + @Test + void syncRetriesOnIOExceptionButNotOnRuntimeException() { + MockedClient client1 = new MockedClient() + .andThrow(new SocketException("boom")).andRespond(200); + try { + TransportHttpClient.Response r = wrap(client1).performRequest("ep", null, REQ, opts(fixed(1L, 3))); + assertEquals(200, r.statusCode()); + assertEquals(2, client1.calls.get()); + } catch (IOException e) { + fail("did not expect exception", e); + } + + MockedClient client2 = new MockedClient() + .andThrow(new IllegalStateException("bad")).andRespond(200); + assertThrows(IllegalStateException.class, + () -> wrap(client2).performRequest("ep", null, REQ, opts(fixed(1L, 3)))); + assertEquals(1, client2.calls.get()); + } + + @Test + void syncReturnsLastResponseWhenRetriesExhausted() throws IOException { + MockedClient client = new MockedClient() + .andRespond(503).andRespond(503).andRespond(503); + + TransportHttpClient.Response resp = wrap(client).performRequest("ep", null, REQ, opts(fixed(1L, 2))); + + assertEquals(503, resp.statusCode()); + assertEquals(3, client.calls.get()); + } + + @Test + void syncThrowsLastIOExceptionWhenRetriesExhausted() { + MockedClient client = new MockedClient() + .andThrow(new SocketException("first")) + .andThrow(new SocketException("second")) + .andThrow(new SocketException("third")); + + IOException e = assertThrows(IOException.class, + () -> wrap(client).performRequest("ep", null, REQ, opts(fixed(1L, 2)))); + assertEquals("third", e.getMessage()); + assertEquals(3, client.calls.get()); + } + + @Test + void asyncRetriesOn5xxThenSucceeds() throws Exception { + MockedClient client = new MockedClient() + .andRespond(503).andRespond(502).andRespond(200); + + TransportHttpClient.Response resp = wrap(client).performRequestAsync("ep", null, REQ, opts(fixed(1L, 5))) + .get(5, TimeUnit.SECONDS); + + assertEquals(200, resp.statusCode()); + assertEquals(3, client.calls.get()); + } + + @Test + void asyncRetriesOnIOExceptionThenSucceeds() throws Exception { + MockedClient client = new MockedClient() + .andThrow(new SocketException("nope")).andRespond(200); + + TransportHttpClient.Response resp = wrap(client).performRequestAsync("ep", null, REQ, opts(fixed(1L, 3))) + .get(5, TimeUnit.SECONDS); + + assertEquals(200, resp.statusCode()); + assertEquals(2, client.calls.get()); + } + + @Test + void asyncDoesNotRetryOnRuntimeException() { + MockedClient client = new MockedClient() + .andThrow(new IllegalArgumentException("nope")).andRespond(200); + + CompletableFuture f = + wrap(client).performRequestAsync("ep", null, REQ, opts(fixed(1L, 3))); + CompletionException ex = assertThrows(CompletionException.class, f::join); + assertTrue(ex.getCause() instanceof IllegalArgumentException); + assertEquals(1, client.calls.get()); + } + + @Test + void asyncReturnsLastResponseWhenRetriesExhausted() throws Exception { + MockedClient client = new MockedClient() + .andRespond(503).andRespond(503).andRespond(503); + + TransportHttpClient.Response resp = wrap(client).performRequestAsync("ep", null, REQ, opts(fixed(1L, 2))) + .get(5, TimeUnit.SECONDS); + + assertEquals(503, resp.statusCode()); + assertEquals(3, client.calls.get()); + } + + @Test + void disabledRetryConfigForwardsToDelegate() throws IOException { + // A disabled RetryConfig must not retry: a 503 is returned as-is on the first attempt. + MockedClient client = new MockedClient().andRespond(503).andRespond(200); + + TransportHttpClient.Response resp = wrap(client) + .performRequest("ep", null, REQ, opts(RetryConfig.disabled())); + + assertEquals(503, resp.statusCode()); + assertEquals(1, client.calls.get()); + } + + /** + * Verifies that cancelling the result future stops pending retries: with a long backoff (1 hour), + * after we cancel during the first delay, the delegate must not be called again. + */ + @Test + void asyncCancellationStopsPendingRetries() throws Exception { + CountDownLatch firstCallReturned = new CountDownLatch(1); + AtomicInteger calls = new AtomicInteger(); + TransportHttpClient delegate = new TransportHttpClient() { + @Override + public Response performRequest(String endpointId, @Nullable Node node, Request request, TransportOptions options) { + throw new UnsupportedOperationException(); + } + + @Override + public CompletableFuture performRequestAsync(String endpointId, @Nullable Node node, + Request request, TransportOptions options) { + calls.incrementAndGet(); + CompletableFuture f = new CompletableFuture<>(); + f.complete(new FakeResponse(503, Collections.emptyMap())); + firstCallReturned.countDown(); + return f; + } + + @Override public void close() {} + }; + + RetryingHttpClient retry = wrap(delegate); + + // 1h delay so the retry stays pending until we cancel + CompletableFuture result = retry.performRequestAsync( + "ep", null, REQ, opts(BackoffPolicy.constantBackoff(3_600_000L, 5))); + + // Wait for the first attempt to complete (we know it returned 503, so a retry is scheduled) + assertTrue(firstCallReturned.await(5, TimeUnit.SECONDS)); + + // Give the whenComplete handler a moment to schedule the retry + Thread.sleep(50); + + result.cancel(true); + + // The cancellation should be observable + assertThrows(CancellationException.class, () -> result.get(1, TimeUnit.SECONDS)); + + // No further calls should happen + Thread.sleep(100); + assertEquals(1, calls.get(), "delegate should not be called again after cancellation"); + } + + // ---------- retryability classifiers ---------- + + @Test + void exceptionClassifierUsesConfiguredTypes() { + RetryingHttpClient client = wrap(new MockedClient()); + + // Default behaviour: any IOException (and its subclasses) is retryable + RetryConfig defaultCfg = RetryConfig.of(r -> r.backoffPolicy(fixed(1L, 1))); + assertTrue(client.isRetryableException(defaultCfg, new SocketException("io"))); + assertTrue(client.isRetryableException(defaultCfg, new RuntimeException(new IOException("wrapped")))); + assertFalse(client.isRetryableException(defaultCfg, new IllegalArgumentException("bad"))); + assertFalse(client.isRetryableException(defaultCfg, new NullPointerException())); + + // Narrowed to a specific subclass: only that subclass is retryable + RetryConfig narrowCfg = RetryConfig.of(r -> r.backoffPolicy(fixed(1L, 1)).retryableExceptions(SocketException.class)); + assertTrue(client.isRetryableException(narrowCfg, new SocketException("io"))); + assertFalse(client.isRetryableException(narrowCfg, new IOException("plain io"))); + } + + /** An exception carrying an http status, like the low-level clients' ResponseException. */ + static final class StatusException extends IOException { + final int status; + StatusException(int status) { + super("status " + status); + this.status = status; + } + } + + /** A delegate that recognizes StatusException, like the real adapters recognize ResponseException. */ + static class StatusAwareClient extends MockedClient { + @Override + public Integer responseStatusCode(Throwable exception) { + return exception instanceof StatusException ? ((StatusException) exception).status : null; + } + } + + @Test + void failureClassifierUsesDelegateProvidedStatusOverExceptionType() { + RetryingHttpClient client = wrap(new StatusAwareClient()); + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 1)) + .retryableStatuses(429, 503)); + + // Exceptions recognized by the delegate as carrying a response are classified by status code only: + // even though StatusException is an IOException (the default retryable exception), a non-retryable + // status must not be retried. + assertTrue(client.isRetryableFailure(cfg, new StatusException(503))); + assertFalse(client.isRetryableFailure(cfg, new StatusException(501))); + // Also when the status-carrying exception is wrapped + assertFalse(client.isRetryableFailure(cfg, new RuntimeException(new StatusException(501)))); + + // Ordinary exceptions don't carry a status code and are classified by type. + assertTrue(client.isRetryableFailure(cfg, new SocketException("io"))); + assertFalse(client.isRetryableFailure(cfg, new IllegalArgumentException("bad"))); + } + + @Test + void asyncRetriesOnStatusCarryingException() throws Exception { + // End-to-end: a 503 surfacing as an exception is retried, then the request succeeds. Exception-based + // retries are disabled, so only the status extracted from the exception can trigger the retry. + MockedClient client = new StatusAwareClient(); + client.andThrow(new StatusException(503)).andRespond(200); + + RetryConfig cfg = RetryConfig.of(r -> r + .backoffPolicy(fixed(1L, 3)) + .retryableExceptions(new HashSet<>())); + + TransportHttpClient.Response resp = wrap(client) + .performRequestAsync("ep", null, REQ, opts(cfg)) + .get(5, TimeUnit.SECONDS); + + assertEquals(200, resp.statusCode()); + assertEquals(2, client.calls.get()); + } + + @Test + void responseClassifierUsesConfiguredStatuses() { + RetryingHttpClient client = wrap(new MockedClient()); + + RetryConfig defaultCfg = RetryConfig.of(r -> r.backoffPolicy(fixed(1L, 1))); + assertTrue(client.isRetryableStatus(defaultCfg, new FakeResponse(500, Collections.emptyMap()))); + assertTrue(client.isRetryableStatus(defaultCfg, new FakeResponse(429, Collections.emptyMap()))); + assertTrue(client.isRetryableStatus(defaultCfg, new FakeResponse(503, Collections.emptyMap()))); + assertFalse(client.isRetryableStatus(defaultCfg, new FakeResponse(501, Collections.emptyMap()))); + assertFalse(client.isRetryableStatus(defaultCfg, new FakeResponse(404, Collections.emptyMap()))); + assertFalse(client.isRetryableStatus(defaultCfg, new FakeResponse(200, Collections.emptyMap()))); + + RetryConfig customCfg = RetryConfig.of(r -> r.backoffPolicy(fixed(1L, 1)).retryableStatuses(418)); + assertTrue(client.isRetryableStatus(customCfg, new FakeResponse(418, Collections.emptyMap()))); + assertFalse(client.isRetryableStatus(customCfg, new FakeResponse(503, Collections.emptyMap()))); + } +}