diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-06-10 09:47:05 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2021-06-10 09:47:05 +0200 |
commit | 3019f32f0ccdcfa13b63ba0ad157df584f535758 (patch) | |
tree | a110a6130ce5af4ab89d569b821a4363fcc207a3 | |
parent | 5be6949b581dc9c4b78c8d270a1868eae2bdda0b (diff) |
Prefer document API timeout in /doc/v1
2 files changed, 30 insertions, 22 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index 789a2ab537f..9db296e33cd 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -122,7 +122,7 @@ import static java.util.stream.Collectors.toUnmodifiableMap; */ public class DocumentV1ApiHandler extends AbstractRequestHandler { - private static final Duration defaultTimeout = Duration.ofSeconds(175); + private static final Duration defaultTimeout = Duration.ofSeconds(180); // Match document API default timeout. private static final Logger log = Logger.getLogger(DocumentV1ApiHandler.class.getName()); private static final Parser<Integer> integerParser = Integer::parseInt; @@ -160,6 +160,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { private static final String TRACELEVEL = "tracelevel"; private final Clock clock; + private final Duration handlerTimeout; private final Metric metric; private final DocumentApiMetrics metrics; private final DocumentOperationParser parser; @@ -184,14 +185,15 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { ClusterListConfig clusterListConfig, AllClustersBucketSpacesConfig bucketSpacesConfig, DocumentOperationExecutorConfig executorConfig) { - this(Clock.systemUTC(), metric, metricReceiver, documentAccess, + this(Clock.systemUTC(), Duration.ofSeconds(5), metric, metricReceiver, documentAccess, documentManagerConfig, executorConfig, clusterListConfig, bucketSpacesConfig); } - DocumentV1ApiHandler(Clock clock, Metric metric, MetricReceiver metricReceiver, DocumentAccess access, + DocumentV1ApiHandler(Clock clock, Duration handlerTimeout, Metric metric, MetricReceiver metricReceiver, DocumentAccess access, DocumentmanagerConfig documentmanagerConfig, DocumentOperationExecutorConfig executorConfig, ClusterListConfig clusterListConfig, AllClustersBucketSpacesConfig bucketSpacesConfig) { this.clock = clock; + this.handlerTimeout = handlerTimeout; this.parser = new DocumentOperationParser(documentmanagerConfig); this.metric = metric; this.metrics = new DocumentApiMetrics(metricReceiver, "documentV1"); @@ -222,8 +224,9 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { HttpRequest request = (HttpRequest) rawRequest; try { - request.setTimeout(getProperty(request, TIMEOUT, timeoutMillisParser) - .orElse(defaultTimeout.toMillis()), + // Set a higher HTTP layer timeout than the document API timeout, to prefer triggering the latter. + request.setTimeout( getProperty(request, TIMEOUT, timeoutMillisParser).orElse(defaultTimeout.toMillis()) + + handlerTimeout.toMillis(), TimeUnit.MILLISECONDS); Path requestPath = new Path(request.getUri()); @@ -251,7 +254,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { @Override public void handleTimeout(Request request, ResponseHandler responseHandler) { - timeout((HttpRequest) request, "Request timeout after " + request.getTimeout(TimeUnit.MILLISECONDS) + "ms", responseHandler); + timeout((HttpRequest) request, "Timeout after " + (request.getTimeout(TimeUnit.MILLISECONDS) - handlerTimeout.toMillis()) + "ms", responseHandler); } @Override @@ -970,7 +973,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { parameters.setMaxTotalHits(wantedDocumentCount); parameters.setThrottlePolicy(new StaticThrottlePolicy().setMaxPendingCount(concurrency)); parameters.visitInconsistentBuckets(true); - parameters.setSessionTimeoutMs(Math.max(1, request.getTimeout(TimeUnit.MILLISECONDS) - 5000)); + parameters.setSessionTimeoutMs(Math.max(1, request.getTimeout(TimeUnit.MILLISECONDS) - handlerTimeout.toMillis())); return parameters; } @@ -980,7 +983,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { VisitorParameters parameters = parseCommonParameters(request, path, Optional.of(requireProperty(request, CLUSTER))); parameters.setThrottlePolicy(new DynamicThrottlePolicy().setMinWindowSize(1).setWindowSizeIncrement(1)); long timeChunk = getProperty(request, TIME_CHUNK, timeoutMillisParser).orElse(60_000L); - parameters.setSessionTimeoutMs(Math.max(1, Math.min(timeChunk, request.getTimeout(TimeUnit.MILLISECONDS) - 5000L))); + parameters.setSessionTimeoutMs(Math.max(1, Math.min(timeChunk, request.getTimeout(TimeUnit.MILLISECONDS) - handlerTimeout.toMillis()))); return parameters; } diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java index 49fa15849ce..29ae7f52265 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java @@ -60,6 +60,7 @@ import org.junit.Test; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.UncheckedIOException; +import java.time.Duration; import java.util.Collection; import java.util.List; import java.util.Map; @@ -133,7 +134,7 @@ public class DocumentV1ApiTest { access = new MockDocumentAccess(docConfig); metric = new NullMetric(); metrics = new MetricReceiver.MockReceiver(); - handler = new DocumentV1ApiHandler(clock, metric, metrics, access, docConfig, executorConfig, clusterConfig, bucketConfig); + handler = new DocumentV1ApiHandler(clock, Duration.ofMillis(1), metric, metrics, access, docConfig, executorConfig, clusterConfig, bucketConfig); } @After @@ -179,7 +180,7 @@ public class DocumentV1ApiTest { } @Test - public void testResponses() { + public void testResponses() throws InterruptedException { RequestHandlerTestDriver driver = new RequestHandlerTestDriver(handler); List<AckToken> tokens = List.of(new AckToken(null), new AckToken(null), new AckToken(null)); // GET at non-existent path returns 404 with available paths @@ -207,7 +208,7 @@ public class DocumentV1ApiTest { assertEquals(100, ((StaticThrottlePolicy) parameters.getThrottlePolicy()).getMaxPendingCount()); assertEquals("[id]", parameters.getFieldSet()); assertEquals("(all the things)", parameters.getDocumentSelection()); - assertEquals(1000, parameters.getSessionTimeoutMs()); + assertEquals(6000, parameters.getSessionTimeoutMs()); // Put some documents in the response parameters.getLocalDataHandler().onMessage(new PutDocumentMessage(new DocumentPut(doc1)), tokens.get(0)); parameters.getLocalDataHandler().onMessage(new PutDocumentMessage(new DocumentPut(doc2)), tokens.get(1)); @@ -272,7 +273,7 @@ public class DocumentV1ApiTest { access.expect(parameters -> { assertEquals("[Content:cluster=content]", parameters.getRemoteDataHandler()); assertEquals("[all]", parameters.fieldSet()); - assertEquals(55_000L, parameters.getSessionTimeoutMs()); + assertEquals(60_000L, parameters.getSessionTimeoutMs()); parameters.getControlHandler().onDone(VisitorControlHandler.CompletionCode.SUCCESS, "We made it!"); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid?destinationCluster=content&selection=true&cluster=content&timeout=60", POST); @@ -666,14 +667,18 @@ public class DocumentV1ApiTest { handler.set(parameters.responseHandler().get()); return new Result(Result.ResultType.SUCCESS, null); }); - var response4 = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?timeout=1ms"); - assertSameJson("{" + - " \"pathId\": \"/document/v1/space/music/docid/one\"," + - " \"message\": \"Request timeout after 1ms\"" + - "}", response4.readAll()); - assertEquals(504, response4.getStatus()); - if (handler.get() != null) // Timeout may have occurred before dispatch, or ... - handler.get().handleResponse(new Response(0)); // response may eventually arrive, but too late. + try { + var response4 = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?timeout=1ms"); + assertSameJson("{" + + " \"pathId\": \"/document/v1/space/music/docid/one\"," + + " \"message\": \"Timeout after 1ms\"" + + "}", response4.readAll()); + assertEquals(504, response4.getStatus()); + } + finally { + if (handler.get() != null) // Timeout may have occurred before dispatch, or ... + handler.get().handleResponse(new Response(0)); // response may eventually arrive, but too late. + } driver.close(); } @@ -681,7 +686,7 @@ public class DocumentV1ApiTest { @Test public void testThroughput() throws InterruptedException { DocumentOperationExecutorConfig executorConfig = new DocumentOperationExecutorConfig.Builder().build(); - handler = new DocumentV1ApiHandler(clock, metric, metrics, access, docConfig, executorConfig, clusterConfig, bucketConfig); + handler = new DocumentV1ApiHandler(clock, Duration.ofMillis(1), metric, metrics, access, docConfig, executorConfig, clusterConfig, bucketConfig); int writers = 4; int queueFill = executorConfig.maxThrottled() - writers; @@ -742,7 +747,7 @@ public class DocumentV1ApiTest { }); } latch.await(); - System.err.println((System.nanoTime() - startNanos) * 1e-9 + " seconds total"); + System.err.println(docs + " requests in " + (System.nanoTime() - startNanos) * 1e-9 + " seconds"); assertNull(failed.get()); driver.close(); |