From dab097c727e05113571d00494d269078501aa79e Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 25 Apr 2023 10:50:24 +0200 Subject: Set HTTP request timeout in feed client based on given operation timeout --- .../java/ai/vespa/feed/client/impl/ApacheCluster.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java index 3192bb4f225..c240018df11 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java @@ -24,9 +24,11 @@ import javax.net.ssl.SSLContext; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.URI; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -38,6 +40,7 @@ import java.util.zip.GZIPOutputStream; import static ai.vespa.feed.client.FeedClientBuilder.Compression.auto; import static ai.vespa.feed.client.FeedClientBuilder.Compression.gzip; +import static java.util.Objects.requireNonNullElse; import static org.apache.hc.core5.http.ssl.TlsCiphers.excludeH2Blacklisted; import static org.apache.hc.core5.http.ssl.TlsCiphers.excludeWeak; @@ -50,7 +53,7 @@ class ApacheCluster implements Cluster { private final List defaultHeaders = Arrays.asList(new BasicHeader(HttpHeaders.USER_AGENT, String.format("vespa-feed-client/%s", Vespa.VERSION)), new BasicHeader("Vespa-Client-Version", Vespa.VERSION)); private final Header gzipEncodingHeader = new BasicHeader(HttpHeaders.CONTENT_ENCODING, "gzip"); - private final RequestConfig requestConfig; + private final RequestConfig.Builder requestConfig; private final Compression compression; private int someNumber = 0; @@ -86,7 +89,8 @@ class ApacheCluster implements Cluster { SimpleHttpRequest request = new SimpleHttpRequest(wrapped.method(), wrapped.path()); request.setScheme(endpoint.url.getScheme()); request.setAuthority(new URIAuthority(endpoint.url.getHost(), portOf(endpoint.url))); - request.setConfig(requestConfig); + long timeoutMillis = wrapped.timeout() == null ? 190_000 : wrapped.timeout().toMillis() * 11 / 10 + 1_000; + request.setConfig(requestConfig.setResponseTimeout(Timeout.ofMilliseconds(timeoutMillis)).build()); defaultHeaders.forEach(request::setHeader); wrapped.headers().forEach((name, value) -> request.setHeader(name, value.get())); if (wrapped.body() != null) { @@ -104,11 +108,10 @@ class ApacheCluster implements Cluster { @Override public void failed(Exception ex) { vessel.completeExceptionally(ex); } @Override public void cancelled() { vessel.cancel(false); } }); - long timeoutMillis = wrapped.timeout() == null ? 200_000 : wrapped.timeout().toMillis() * 11 / 10 + 1_000; Future cancellation = timeoutExecutor.schedule(() -> { future.cancel(true); vessel.cancel(true); - }, timeoutMillis, TimeUnit.MILLISECONDS); + }, timeoutMillis + 10_000, TimeUnit.MILLISECONDS); vessel.whenComplete((__, ___) -> cancellation.cancel(true)); } catch (Throwable thrown) { @@ -193,13 +196,12 @@ class ApacheCluster implements Cluster { } @SuppressWarnings("deprecation") - private static RequestConfig createRequestConfig(FeedClientBuilderImpl b) { + private static RequestConfig.Builder createRequestConfig(FeedClientBuilderImpl b) { RequestConfig.Builder builder = RequestConfig.custom() .setConnectTimeout(Timeout.ofSeconds(10)) - .setConnectionRequestTimeout(Timeout.DISABLED) - .setResponseTimeout(Timeout.ofSeconds(190)); + .setConnectionRequestTimeout(Timeout.DISABLED); if (b.proxy != null) builder.setProxy(new HttpHost(b.proxy.getScheme(), b.proxy.getHost(), b.proxy.getPort())); - return builder.build(); + return builder; } private static class ApacheHttpResponse implements HttpResponse { -- cgit v1.2.3 From 6bf90f4877890d6781dba66c877b117da527a8f1 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 25 Apr 2023 13:52:23 +0200 Subject: Copy request config to mutate --- .../java/ai/vespa/feed/client/impl/ApacheCluster.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java index c240018df11..c188e7936c6 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java @@ -53,7 +53,7 @@ class ApacheCluster implements Cluster { private final List defaultHeaders = Arrays.asList(new BasicHeader(HttpHeaders.USER_AGENT, String.format("vespa-feed-client/%s", Vespa.VERSION)), new BasicHeader("Vespa-Client-Version", Vespa.VERSION)); private final Header gzipEncodingHeader = new BasicHeader(HttpHeaders.CONTENT_ENCODING, "gzip"); - private final RequestConfig.Builder requestConfig; + private final RequestConfig requestConfig; private final Compression compression; private int someNumber = 0; @@ -90,7 +90,7 @@ class ApacheCluster implements Cluster { request.setScheme(endpoint.url.getScheme()); request.setAuthority(new URIAuthority(endpoint.url.getHost(), portOf(endpoint.url))); long timeoutMillis = wrapped.timeout() == null ? 190_000 : wrapped.timeout().toMillis() * 11 / 10 + 1_000; - request.setConfig(requestConfig.setResponseTimeout(Timeout.ofMilliseconds(timeoutMillis)).build()); + request.setConfig(RequestConfig.copy(requestConfig).setResponseTimeout(Timeout.ofMilliseconds(timeoutMillis)).build()); defaultHeaders.forEach(request::setHeader); wrapped.headers().forEach((name, value) -> request.setHeader(name, value.get())); if (wrapped.body() != null) { @@ -108,10 +108,11 @@ class ApacheCluster implements Cluster { @Override public void failed(Exception ex) { vessel.completeExceptionally(ex); } @Override public void cancelled() { vessel.cancel(false); } }); - Future cancellation = timeoutExecutor.schedule(() -> { - future.cancel(true); - vessel.cancel(true); - }, timeoutMillis + 10_000, TimeUnit.MILLISECONDS); + // We've seen some requests time out, even with a response timeout, + // so we schedule this to be absolutely sure we don't hang (for ever). + Future cancellation = timeoutExecutor.schedule(() -> { future.cancel(true); vessel.cancel(true); }, + timeoutMillis + 10_000, + TimeUnit.MILLISECONDS); vessel.whenComplete((__, ___) -> cancellation.cancel(true)); } catch (Throwable thrown) { @@ -196,12 +197,12 @@ class ApacheCluster implements Cluster { } @SuppressWarnings("deprecation") - private static RequestConfig.Builder createRequestConfig(FeedClientBuilderImpl b) { + private static RequestConfig createRequestConfig(FeedClientBuilderImpl b) { RequestConfig.Builder builder = RequestConfig.custom() .setConnectTimeout(Timeout.ofSeconds(10)) .setConnectionRequestTimeout(Timeout.DISABLED); if (b.proxy != null) builder.setProxy(new HttpHost(b.proxy.getScheme(), b.proxy.getHost(), b.proxy.getPort())); - return builder; + return builder.build(); } private static class ApacheHttpResponse implements HttpResponse { -- cgit v1.2.3 From cec5d1f76d9f5a20d2139cc564517744f1630794 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 25 Apr 2023 13:54:34 +0200 Subject: Fix imports --- .../src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java index c188e7936c6..8e7bf59cd0f 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java @@ -24,11 +24,9 @@ import javax.net.ssl.SSLContext; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.URI; -import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -40,7 +38,6 @@ import java.util.zip.GZIPOutputStream; import static ai.vespa.feed.client.FeedClientBuilder.Compression.auto; import static ai.vespa.feed.client.FeedClientBuilder.Compression.gzip; -import static java.util.Objects.requireNonNullElse; import static org.apache.hc.core5.http.ssl.TlsCiphers.excludeH2Blacklisted; import static org.apache.hc.core5.http.ssl.TlsCiphers.excludeWeak; -- cgit v1.2.3