diff options
41 files changed, 388 insertions, 325 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index 413a7dbc62e..8ccd1e67ba1 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -120,7 +120,7 @@ public interface ModelContext { default Duration jdiscHealthCheckProxyClientTimeout() { return Duration.ofMillis(100); } // TODO(bjorncs): Temporary feature flag - default double feedCoreThreadPoolSizeFactor() { return 1.0; } + default double feedCoreThreadPoolSizeFactor() { return 4.0; } default Quota quota() { return Quota.empty(); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java index a4737c9f54c..9012cc2aba0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java @@ -64,10 +64,9 @@ public class ContainerDocumentApi { } private static ThreadPoolExecutorComponent newExecutorComponent(String name, ContainerCluster<?> cluster, Options options) { - int maxPoolSize = maxPoolSize(cluster); return new ThreadPoolExecutorComponent.Builder(name) - .maxPoolSize(maxPoolSize) - .corePoolSize(corePoolSize(maxPoolSize, options)) + .maxPoolSize(maxPoolSize(cluster, options)) + .corePoolSize(corePoolSize(cluster, options)) .queueSize(500) .build(); } @@ -93,29 +92,36 @@ public class ContainerDocumentApi { return handler; } - private static int maxPoolSize(ContainerCluster<?> cluster) { + private static int maxPoolSize(ContainerCluster<?> cluster, Options options) { + double vcpu = vcpu(cluster); + if (vcpu == 0) return FALLBACK_MAX_POOL_SIZE; + return Math.max(2, (int)Math.ceil(vcpu * options.feedThreadPoolSizeFactor)); + } + + private static int corePoolSize(ContainerCluster<?> cluster, Options options) { + double vcpu = vcpu(cluster); + if (vcpu == 0) return FALLBACK_CORE_POOL_SIZE; + return Math.max(1, (int)Math.ceil(vcpu * options.feedThreadPoolSizeFactor * 0.5)); + } + + private static double vcpu(ContainerCluster<?> cluster) { List<Double> vcpus = cluster.getContainers().stream() .filter(c -> c.getHostResource() != null && c.getHostResource().realResources() != null) .map(c -> c.getHostResource().realResources().vcpu()) .distinct() .collect(Collectors.toList()); // We can only use host resource for calculation if all container nodes in the cluster are homogeneous (in terms of vcpu) - if (vcpus.size() != 1 || vcpus.get(0) == 0) return FALLBACK_MAX_POOL_SIZE; - return Math.max(2, (int)Math.ceil(vcpus.get(0))); - } - - private static int corePoolSize(int maxPoolSize, Options options) { - if (maxPoolSize == FALLBACK_MAX_POOL_SIZE) return FALLBACK_CORE_POOL_SIZE; - return Math.max(1, (int)Math.ceil(options.feedCoreThreadPoolSizeFactor * maxPoolSize)); + if (vcpus.size() != 1 || vcpus.get(0) == 0) return 0; + return vcpus.get(0); } public static final class Options { private final Collection<String> bindings; - private final double feedCoreThreadPoolSizeFactor; + private final double feedThreadPoolSizeFactor; - public Options(Collection<String> bindings, double feedCoreThreadPoolSizeFactor) { + public Options(Collection<String> bindings, double feedThreadPoolSizeFactor) { this.bindings = Collections.unmodifiableCollection(bindings); - this.feedCoreThreadPoolSizeFactor = feedCoreThreadPoolSizeFactor; + this.feedThreadPoolSizeFactor = feedThreadPoolSizeFactor; } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java index 37e5dc21346..ab73309bb7d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java @@ -116,8 +116,8 @@ public class ContainerDocumentApiBuilderTest extends ContainerModelBuilderTestBa assertThat(injectedComponentIds, hasItem("threadpool@feedapi-handler")); ThreadpoolConfig config = root.getConfig(ThreadpoolConfig.class, "cluster1/component/com.yahoo.vespa.http.server.FeedHandler/threadpool@feedapi-handler"); - assertEquals(4, config.maxthreads()); - assertEquals(4, config.corePoolSize()); + assertEquals(16, config.maxthreads()); + assertEquals(8, config.corePoolSize()); } private static class HostProvisionerWithCustomRealResource implements HostProvisioner { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionContentReadResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionContentReadResponse.java index 07d27ffd5de..a1a41cc0472 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionContentReadResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionContentReadResponse.java @@ -14,9 +14,9 @@ import static com.yahoo.jdisc.http.HttpResponse.Status.OK; * Represents a response for a request to read contents of a file. * * @author Ulf Lilleengen - * @since 5.1 */ public class SessionContentReadResponse extends HttpResponse { + private final ApplicationFile file; public SessionContentReadResponse(ApplicationFile file) { @@ -35,4 +35,5 @@ public class SessionContentReadResponse extends HttpResponse { public String getContentType() { return HttpResponse.DEFAULT_MIME_TYPE; } + } diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index 9292a946e82..244087e0271 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -464,7 +464,9 @@ "public java.lang.String getCharacterEncoding()", "public void populateAccessLogEntry(com.yahoo.container.logging.AccessLogEntry)", "public void complete()", - "public java.lang.Iterable getLogValues()" + "public java.lang.Iterable getLogValues()", + "public void setRequestType(com.yahoo.jdisc.Request$RequestType)", + "public com.yahoo.jdisc.Request$RequestType getRequestType()" ], "fields": [ "public static final java.lang.String DEFAULT_MIME_TYPE", diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/HttpResponse.java b/container-core/src/main/java/com/yahoo/container/jdisc/HttpResponse.java index b4fcd044e50..dd03d72d97d 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/HttpResponse.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/HttpResponse.java @@ -3,6 +3,7 @@ package com.yahoo.container.jdisc; import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.jdisc.HeaderFields; +import com.yahoo.jdisc.Request; import com.yahoo.jdisc.Response; import com.yahoo.processing.execution.Execution.Trace.LogValue; @@ -18,21 +19,18 @@ import java.util.Collections; */ public abstract class HttpResponse { - /** - * Default response content type; text/plain. - */ + /** Default response content type; text/plain. */ public static final String DEFAULT_MIME_TYPE = "text/plain"; - /** - * Default encoding/character set of a HTTP response; UTF-8. - */ + /** Default encoding/character set of a HTTP response; UTF-8. */ public static final String DEFAULT_CHARACTER_ENCODING = "UTF-8"; - private final Response parentResponse; + private Request.RequestType requestType; + /** - * Create a new HTTP response. + * Creates a new HTTP response * * @param status the HTTP status code to return with this response (may be changed later) * @see Response @@ -41,13 +39,11 @@ public abstract class HttpResponse { parentResponse = com.yahoo.jdisc.http.HttpResponse.newInstance(status); } - /** - * Marshal this response to the network layer. The caller is responsible for flushing and closing outputStream. - */ + /** Marshals this response to the network layer. The caller is responsible for flushing and closing outputStream. */ public abstract void render(OutputStream outputStream) throws IOException; /** - * The numeric HTTP status code, e.g. 200, 404 and so on. + * Returns the numeric HTTP status code, e.g. 200, 404 and so on. * * @return the numeric HTTP status code */ @@ -129,4 +125,13 @@ public abstract class HttpResponse { return Collections::emptyIterator; } + /** Sets the type classification of this request for metric collection purposes */ + public void setRequestType(Request.RequestType requestType) { this.requestType = requestType; } + + /** + * Returns the type classification of this request for metric collection purposes, or null if not set. + * When not set, the request type will be "read" for GET requests and "write" for other request methods. + */ + public Request.RequestType getRequestType() { return requestType; } + } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java index 3a99ee7d0c6..ac1aa533201 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java @@ -78,6 +78,7 @@ public abstract class ThreadedHttpRequestHandler extends ThreadedRequestHandler try { channel = new LazyContentChannel(httpRequest, responseHandler, metric, log); HttpResponse httpResponse = handle(httpRequest, channel); + request.setRequestType(httpResponse.getRequestType()); channel.setHttpResponse(httpResponse); // may or may not have already been done render(httpRequest, httpResponse, channel, jdiscRequest.creationTime(TimeUnit.MILLISECONDS)); } catch (Exception e) { diff --git a/container-di/pom.xml b/container-di/pom.xml index 8e89910e247..f143d5afd19 100644 --- a/container-di/pom.xml +++ b/container-di/pom.xml @@ -119,6 +119,10 @@ <groupId>com.yahoo.vespa</groupId> <artifactId>abi-check-plugin</artifactId> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-compiler-plugin</artifactId> + </plugin> </plugins> </build> </project> diff --git a/container-search-and-docproc/src/main/java/com/yahoo/container/handler/observability/ApplicationStatusHandler.java b/container-search-and-docproc/src/main/java/com/yahoo/container/handler/observability/ApplicationStatusHandler.java index 05d14e6e1d6..a58aabfce17 100644 --- a/container-search-and-docproc/src/main/java/com/yahoo/container/handler/observability/ApplicationStatusHandler.java +++ b/container-search-and-docproc/src/main/java/com/yahoo/container/handler/observability/ApplicationStatusHandler.java @@ -332,4 +332,5 @@ public class ApplicationStatusHandler extends AbstractRequestHandler { handler.completed(); } } + } diff --git a/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java b/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java index bafe1dbd43f..9ddcc7a7e69 100644 --- a/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java +++ b/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java @@ -40,7 +40,6 @@ import java.util.logging.Level; * * @author Henrik Høiness */ - public class GUIHandler extends LoggingRequestHandler { private final IndexModel indexModel; diff --git a/container-search/src/main/java/com/yahoo/search/Result.java b/container-search/src/main/java/com/yahoo/search/Result.java index ab48d5797b2..6a875851ca9 100644 --- a/container-search/src/main/java/com/yahoo/search/Result.java +++ b/container-search/src/main/java/com/yahoo/search/Result.java @@ -51,7 +51,7 @@ public final class Result extends com.yahoo.processing.Response implements Clone * Headers containing "envelope" meta information to be returned with this result. * Used for HTTP getHeaders when the return protocol is HTTP. */ - private ListMap<String,String> headers = null; + private ListMap<String, String> headers = null; /** Creates a new Result where the top level hit group has id "toplevel" */ public Result(Query query) { @@ -66,7 +66,6 @@ public final class Result extends com.yahoo.processing.Response implements Clone * @param query the query which produced this result * @param hits the hit container which this will return from {@link #hits()} */ - @SuppressWarnings("deprecation") public Result(Query query, HitGroup hits) { super(query); if (query==null) throw new NullPointerException("The query reference in a result cannot be null"); diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index b0c8fbba059..bb4df325762 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -17,6 +17,7 @@ import com.yahoo.container.jdisc.VespaHeaders; import com.yahoo.container.logging.AccessLog; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.Request; import com.yahoo.language.Linguistics; import java.util.logging.Level; import com.yahoo.net.HostName; @@ -314,6 +315,7 @@ public class SearchHandler extends LoggingRequestHandler { HttpSearchResponse response = new HttpSearchResponse(getHttpResponseStatus(request, result), result, query, renderer, extractTraceNode(query)); + response.setRequestType(Request.RequestType.READ); if (hostResponseHeaderKey.isPresent()) response.headers().add(hostResponseHeaderKey.get(), selfHostname); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index d8abc77ed06..7135cbe77c9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -725,7 +725,8 @@ public class ApplicationController { // ok; already gone } finally { controller.routing().policies().refresh(application.get().id().instance(instanceName), application.get().deploymentSpec(), zone); - applicationStore.putMetaTombstone(id, clock.instant()); + if (zone.environment().isManuallyDeployed()) + applicationStore.putMetaTombstone(id, clock.instant()); } return application.with(instanceName, instance -> instance.withoutDeploymentIn(zone)); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 0290c72b9eb..942c9ac037b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -48,6 +48,7 @@ import java.time.Duration; import java.time.Instant; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -60,6 +61,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionUsWest1; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -181,6 +183,27 @@ public class ControllerTest { assertNull("Zone was removed", context.instance().deployments().get(productionUsWest1.zone(main))); assertNull("Deployment job was removed", context.instanceJobs().get(productionUsWest1)); + + // Submission has stored application meta. + assertArrayEquals(applicationPackage.metaDataZip(), + tester.controllerTester().serviceRegistry().applicationStore() + .getMeta(context.instanceId()) + .get(tester.clock().instant())); + + // Meta data tombstone placed on delete + tester.clock().advance(Duration.ofSeconds(1)); + context.submit(ApplicationPackage.deploymentRemoval()); + tester.clock().advance(Duration.ofSeconds(1)); + context.submit(ApplicationPackage.deploymentRemoval()); + tester.applications().deleteApplication(context.application().id(), + tester.controllerTester().credentialsFor(context.instanceId().tenant())); + assertArrayEquals(new byte[0], + tester.controllerTester().serviceRegistry().applicationStore() + .getMeta(context.instanceId()) + .get(tester.clock().instant())); + + assertNull(tester.controllerTester().serviceRegistry().applicationStore() + .getMeta(context.deploymentIdIn(productionUsWest1.zone(main)))); } @Test @@ -635,7 +658,7 @@ public class ControllerTest { tester.configServer().application(context.instanceId(), zone).get().activated()); assertTrue("No job status added", context.instanceJobs().isEmpty()); - assertEquals("DeploymentSpec is not persisted", DeploymentSpec.empty, context.application().deploymentSpec()); + assertEquals("DeploymentSpec is not stored", DeploymentSpec.empty, context.application().deploymentSpec()); // Verify zone supports shared layer 4 and shared routing methods Set<RoutingMethod> routingMethods = tester.controller().routing().endpointsOf(context.deploymentIdIn(zone)) @@ -644,6 +667,20 @@ public class ControllerTest { .map(Endpoint::routingMethod) .collect(Collectors.toSet()); assertEquals(routingMethods, Set.of(RoutingMethod.shared, RoutingMethod.sharedLayer4)); + + // Deployment has stored application meta. + assertArrayEquals(applicationPackage.metaDataZip(), + tester.controllerTester().serviceRegistry().applicationStore() + .getMeta(new DeploymentId(context.instanceId(), zone)) + .get(tester.clock().instant())); + + // Meta data tombstone placed on delete + tester.clock().advance(Duration.ofSeconds(1)); + tester.controller().applications().deactivate(context.instanceId(), zone); + assertArrayEquals(new byte[0], + tester.controllerTester().serviceRegistry().applicationStore() + .getMeta(new DeploymentId(context.instanceId(), zone)) + .get(tester.clock().instant())); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java index 81bda23146e..59e2b6c04d8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationStoreMock.java @@ -141,4 +141,8 @@ public class ApplicationStoreMock implements ApplicationStore { } } + public NavigableMap<Instant, byte[]> getMeta(ApplicationId id) { return meta.get(id); } + + public NavigableMap<Instant, byte[]> getMeta(DeploymentId id) { return metaManual.get(id); } + } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index f33ca549cf7..b7c7fc5ff15 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -346,8 +346,8 @@ public class Flags { ); public static final UnboundDoubleFlag FEED_CORE_THREAD_POOL_SIZE_FACTOR = defineDoubleFlag( - "feed-core-thread-pool-size-factor", 1.0, - "Number of core threads in threadpool for feeding APIs as factor of max pool size", + "feed-core-thread-pool-size-factor", 4.0, + "Max threads in threadpool for feeding APIs as a factor of vcpu", "Takes effect on next internal redeployment", APPLICATION_ID); diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java index e0292bbe026..bce6f72c1fc 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java @@ -48,6 +48,11 @@ public class Request extends AbstractResource { private boolean serverRequest; private Long timeout; private URI uri; + private RequestType requestType; + + public enum RequestType { + READ, WRITE, MONITORING + } /** * <p>Creates a new instance of this class. As a {@link ServerProvider} you need to inject a {@link @@ -326,6 +331,12 @@ public class Request extends AbstractResource { return unit.convert(creationTime, TimeUnit.MILLISECONDS); } + /** Sets the type classification of this request for metric collection purposes */ + public void setRequestType(RequestType requestType) { this.requestType = requestType; } + + /** Returns the type classification of this request for metric collection purposes, or null if not set */ + public RequestType getRequestType() { return requestType; } + /** * <p>Returns whether or not this Request has been cancelled. This can be thought of as the {@link * Thread#isInterrupted()} of Requests - it does not enforce anything in ways of blocking the Request, it is simply diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index b9d686c1d6b..81577561c5b 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -67,12 +67,11 @@ class HttpRequestDispatch { this.jettyRequest = (Request) servletRequest; this.metricReporter = new MetricReporter(jDiscContext.metric, metricContext, jettyRequest.getTimeStamp()); - this.servletResponseController = new ServletResponseController( - servletRequest, - servletResponse, - jDiscContext.janitor, - metricReporter, - jDiscContext.developerMode()); + this.servletResponseController = new ServletResponseController(servletRequest, + servletResponse, + jDiscContext.janitor, + metricReporter, + jDiscContext.developerMode()); markConnectionAsNonPersistentIfThresholdReached(servletRequest); this.async = servletRequest.startAsync(); async.setTimeout(0); @@ -86,17 +85,13 @@ class HttpRequestDispatch { } catch (Throwable throwable) { servletResponseController.trySendError(throwable); servletResponseController.finishedFuture().whenComplete((result, exception) -> - completeRequestCallback.accept(null, throwable)); + completeRequestCallback.accept(null, throwable)); return; } try { - onError(servletRequestReader.finishedFuture, - servletResponseController::trySendError); - - onError(servletResponseController.finishedFuture(), - servletRequestReader::onError); - + onError(servletRequestReader.finishedFuture, servletResponseController::trySendError); + onError(servletResponseController.finishedFuture(), servletRequestReader::onError); CompletableFuture.allOf(servletRequestReader.finishedFuture, servletResponseController.finishedFuture()) .whenComplete(completeRequestCallback); } catch (Throwable throwable) { @@ -104,7 +99,7 @@ class HttpRequestDispatch { } } - private BiConsumer<Void, Throwable> completeRequestCallback; + private final BiConsumer<Void, Throwable> completeRequestCallback; { AtomicBoolean completeRequestCalled = new AtomicBoolean(false); HttpRequestDispatch parent = this; //used to avoid binding uninitialized variables @@ -139,7 +134,7 @@ class HttpRequestDispatch { log.finest(() -> "Request completed successfully: " + parent.jettyRequest.getRequestURI()); } catch (Throwable throwable) { Level level = reportedError ? Level.FINE: Level.WARNING; - log.log(level, "async.complete failed", throwable); + log.log(level, "Async.complete failed", throwable); } }; } @@ -180,16 +175,17 @@ class HttpRequestDispatch { try (ResourceReference ref = References.fromResource(jdiscRequest)) { HttpRequestFactory.copyHeaders(jettyRequest, jdiscRequest); requestContentChannel = requestHandler.handleRequest(jdiscRequest, servletResponseController.responseHandler); + if (jdiscRequest.getRequestType() != null) + jettyRequest.setAttribute(HttpResponseStatisticsCollector.requestTypeAttribute, + jdiscRequest.getRequestType()); } ServletInputStream servletInputStream = jettyRequest.getInputStream(); - ServletRequestReader servletRequestReader = - new ServletRequestReader( - servletInputStream, - requestContentChannel, - jDiscContext.janitor, - metricReporter); + ServletRequestReader servletRequestReader = new ServletRequestReader(servletInputStream, + requestContentChannel, + jDiscContext.janitor, + metricReporter); servletInputStream.setReadListener(servletRequestReader); return servletRequestReader; diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java index 13abb8ddd4d..d2ac0cb7f6a 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; +import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.AsyncContextEvent; @@ -26,18 +27,22 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.LongAdder; /** - * HttpResponseStatisticsCollector collects statistics about HTTP response types aggregated by category (1xx, 2xx, etc). It is similar to - * {@link org.eclipse.jetty.server.handler.StatisticsHandler} with the distinction that this class collects response type statistics grouped + * HttpResponseStatisticsCollector collects statistics about HTTP response types aggregated by category + * (1xx, 2xx, etc). It is similar to {@link org.eclipse.jetty.server.handler.StatisticsHandler} + * with the distinction that this class collects response type statistics grouped * by HTTP method and only collects the numbers that are reported as metrics from Vespa. * * @author ollivir */ public class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful { + + static final String requestTypeAttribute = "requestType"; + private final AtomicReference<FutureCallback> shutdown = new AtomicReference<>(); private final List<String> monitoringHandlerPaths; private final List<String> searchHandlerPaths; - public static enum HttpMethod { + public enum HttpMethod { GET, PATCH, POST, PUT, DELETE, OPTIONS, HEAD, OTHER } @@ -45,18 +50,20 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G HTTP, HTTPS, OTHER } - public enum RequestType { - READ, WRITE, MONITORING - } - - private static final String[] HTTP_RESPONSE_GROUPS = { Metrics.RESPONSES_1XX, Metrics.RESPONSES_2XX, Metrics.RESPONSES_3XX, - Metrics.RESPONSES_4XX, Metrics.RESPONSES_5XX, Metrics.RESPONSES_401, Metrics.RESPONSES_403}; + private static final String[] HTTP_RESPONSE_GROUPS = { + Metrics.RESPONSES_1XX, + Metrics.RESPONSES_2XX, + Metrics.RESPONSES_3XX, + Metrics.RESPONSES_4XX, + Metrics.RESPONSES_5XX, + Metrics.RESPONSES_401, + Metrics.RESPONSES_403 + }; private final AtomicLong inFlight = new AtomicLong(); - private final LongAdder statistics[][][][]; + private final LongAdder[][][][] statistics; public HttpResponseStatisticsCollector(List<String> monitoringHandlerPaths, List<String> searchHandlerPaths) { - super(); this.monitoringHandlerPaths = monitoringHandlerPaths; this.searchHandlerPaths = searchHandlerPaths; statistics = new LongAdder[HttpScheme.values().length][HttpMethod.values().length][][]; @@ -64,8 +71,8 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G for (int method = 0; method < HttpMethod.values().length; method++) { statistics[scheme][method] = new LongAdder[HTTP_RESPONSE_GROUPS.length][]; for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) { - statistics[scheme][method][group] = new LongAdder[RequestType.values().length]; - for (int requestType = 0 ; requestType < RequestType.values().length; requestType++) { + statistics[scheme][method][group] = new LongAdder[HttpRequest.RequestType.values().length]; + for (int requestType = 0; requestType < HttpRequest.RequestType.values().length; requestType++) { statistics[scheme][method][group][requestType] = new LongAdder(); } } @@ -74,18 +81,17 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G } private final AsyncListener completionWatcher = new AsyncListener() { + @Override - public void onTimeout(AsyncEvent event) throws IOException { - } + public void onTimeout(AsyncEvent event) { } @Override - public void onStartAsync(AsyncEvent event) throws IOException { + public void onStartAsync(AsyncEvent event) { event.getAsyncContext().addListener(this); } @Override - public void onError(AsyncEvent event) throws IOException { - } + public void onError(AsyncEvent event) { } @Override public void onComplete(AsyncEvent event) throws IOException { @@ -101,18 +107,16 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G throws IOException, ServletException { inFlight.incrementAndGet(); - /* The control flow logic here is mostly a copy from org.eclipse.jetty.server.handler.StatisticsHandler.handle(..) */ try { Handler handler = getHandler(); if (handler != null && shutdown.get() == null && isStarted()) { handler.handle(path, baseRequest, request, response); - } else if (!baseRequest.isHandled()) { + } else if ( ! baseRequest.isHandled()) { baseRequest.setHandled(true); response.sendError(HttpStatus.SERVICE_UNAVAILABLE_503); } } finally { HttpChannelState state = baseRequest.getHttpChannelState(); - if (state.isSuspended()) { if (state.isInitial()) { state.addListener(completionWatcher); @@ -128,7 +132,7 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G if (group >= 0) { HttpScheme scheme = getScheme(request); HttpMethod method = getMethod(request); - RequestType requestType = getRequestType(request); + HttpRequest.RequestType requestType = getRequestType(request); statistics[scheme.ordinal()][method.ordinal()][group][requestType.ordinal()].increment(); if (group == 5 || group == 6) { // if 401/403, also increment 4xx @@ -197,18 +201,22 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G } } - private RequestType getRequestType(Request request) { + private HttpRequest.RequestType getRequestType(Request request) { + HttpRequest.RequestType requestType = (HttpRequest.RequestType)request.getAttribute(requestTypeAttribute); + if (requestType != null) return requestType; + + // Deduce from path and method: String path = request.getRequestURI(); for (String monitoringHandlerPath : monitoringHandlerPaths) { - if (path.startsWith(monitoringHandlerPath)) return RequestType.MONITORING; + if (path.startsWith(monitoringHandlerPath)) return HttpRequest.RequestType.MONITORING; } for (String searchHandlerPath : searchHandlerPaths) { - if (path.startsWith(searchHandlerPath)) return RequestType.READ; + if (path.startsWith(searchHandlerPath)) return HttpRequest.RequestType.READ; } if ("GET".equals(request.getMethod())) { - return RequestType.READ; + return HttpRequest.RequestType.READ; } else { - return RequestType.WRITE; + return HttpRequest.RequestType.WRITE; } } @@ -219,7 +227,7 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G for (HttpMethod method : HttpMethod.values()) { int methodIndex = method.ordinal(); for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) { - for (RequestType type : RequestType.values()) { + for (HttpRequest.RequestType type : HttpRequest.RequestType.values()) { long value = statistics[schemeIndex][methodIndex][group][type.ordinal()].sumThenReset(); if (value > 0) { ret.add(new StatisticsEntry(scheme.name().toLowerCase(), method.name(), HTTP_RESPONSE_GROUPS[group], type.name().toLowerCase(), value)); @@ -241,15 +249,13 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G protected void doStop() throws Exception { super.doStop(); FutureCallback shutdownCb = shutdown.get(); - if (shutdown != null && !shutdownCb.isDone()) { + if ( ! shutdownCb.isDone()) { shutdownCb.failed(new TimeoutException()); } } @Override public Future<Void> shutdown() { - /* This shutdown callback logic is a copy from org.eclipse.jetty.server.handler.StatisticsHandler */ - FutureCallback shutdownCb = new FutureCallback(false); shutdown.compareAndSet(null, shutdownCb); shutdownCb = shutdown.get(); @@ -266,13 +272,13 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G } public static class StatisticsEntry { + public final String scheme; public final String method; public final String name; public final String requestType; public final long value; - public StatisticsEntry(String scheme, String method, String name, String requestType, long value) { this.scheme = scheme; this.method = method; @@ -280,5 +286,7 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G this.requestType = requestType; this.value = value; } + } + } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java index a6b2deb4681..dfbcfb741f5 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java @@ -33,49 +33,47 @@ class JDiscHttpServlet extends HttpServlet { private final static Logger log = Logger.getLogger(JDiscHttpServlet.class.getName()); private final JDiscContext context; + private static final Set<String> servletSupportedMethods = + Stream.of(Method.OPTIONS, Method.GET, Method.HEAD, Method.POST, Method.PUT, Method.DELETE, Method.TRACE) + .map(Method::name) + .collect(Collectors.toSet()); + public JDiscHttpServlet(JDiscContext context) { this.context = context; } @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @Override - protected void doPost(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @Override - protected void doHead(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doHead(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @Override - protected void doPut(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doPut(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @Override - protected void doDelete(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doDelete(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @Override - protected void doOptions(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doOptions(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @Override - protected void doTrace(HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { + protected void doTrace(HttpServletRequest request, HttpServletResponse response) throws IOException { dispatchHttpRequest(request, response); } @@ -92,11 +90,6 @@ class JDiscHttpServlet extends HttpServlet { context.metric.add(JettyHttpServer.Metrics.NUM_REQUESTS, 1, metricContext); context.metric.add(JettyHttpServer.Metrics.JDISC_HTTP_REQUESTS, 1, metricContext); - - Set<String> servletSupportedMethods = - Stream.of(Method.OPTIONS, Method.GET, Method.HEAD, Method.POST, Method.PUT, Method.DELETE, Method.TRACE) - .map(Method::name) - .collect(Collectors.toSet()); String method = request.getMethod().toUpperCase(); if (servletSupportedMethods.contains(method)) { super.service(request, response); @@ -109,8 +102,6 @@ class JDiscHttpServlet extends HttpServlet { } } - - static JDiscServerConnector getConnector(HttpServletRequest request) { return (JDiscServerConnector)getConnection(request).getConnector(); } @@ -121,8 +112,7 @@ class JDiscHttpServlet extends HttpServlet { try { switch (request.getDispatcherType()) { case REQUEST: - new HttpRequestDispatch(context, accessLogEntry, getMetricContext(request), request, response) - .dispatch(); + new HttpRequestDispatch(context, accessLogEntry, getMetricContext(request), request, response).dispatch(); break; default: if (log.isLoggable(Level.INFO)) { diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java index 51bcb892591..f480c659578 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java @@ -21,6 +21,7 @@ import java.util.concurrent.ConcurrentHashMap; * @author bjorncs */ class JDiscServerConnector extends ServerConnector { + public static final String REQUEST_ATTRIBUTE = JDiscServerConnector.class.getName(); private final Metric.Context metricCtx; private final Map<RequestDimensions, Metric.Context> requestMetricContextCache = new ConcurrentHashMap<>(); diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 386704a5cc2..ba477f9d32f 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -128,17 +128,16 @@ public class JettyHttpServer extends AbstractServerProvider { private final List<Integer> listenedPorts = new ArrayList<>(); @Inject - public JettyHttpServer( - final CurrentContainer container, - final Metric metric, - final ServerConfig serverConfig, - final ServletPathsConfig servletPathsConfig, - final ThreadFactory threadFactory, - final FilterBindings filterBindings, - final ComponentRegistry<ConnectorFactory> connectorFactories, - final ComponentRegistry<ServletHolder> servletHolders, - final FilterInvoker filterInvoker, - final AccessLog accessLog) { + public JettyHttpServer(CurrentContainer container, + Metric metric, + ServerConfig serverConfig, + ServletPathsConfig servletPathsConfig, + ThreadFactory threadFactory, + FilterBindings filterBindings, + ComponentRegistry<ConnectorFactory> connectorFactories, + ComponentRegistry<ServletHolder> servletHolders, + FilterInvoker filterInvoker, + AccessLog accessLog) { super(container); if (connectorFactories.allComponents().isEmpty()) throw new IllegalArgumentException("No connectors configured."); @@ -160,44 +159,40 @@ public class JettyHttpServer extends AbstractServerProvider { janitor = newJanitor(threadFactory); - JDiscContext jDiscContext = new JDiscContext( - filterBindings.getRequestFilters().activate(), - filterBindings.getResponseFilters().activate(), - container, - janitor, - metric, - serverConfig); + JDiscContext jDiscContext = new JDiscContext(filterBindings.getRequestFilters().activate(), + filterBindings.getResponseFilters().activate(), + container, + janitor, + metric, + serverConfig); ServletHolder jdiscServlet = new ServletHolder(new JDiscHttpServlet(jDiscContext)); FilterHolder jDiscFilterInvokerFilter = new FilterHolder(new JDiscFilterInvokerFilter(jDiscContext, filterInvoker)); List<JDiscServerConnector> connectors = Arrays.stream(server.getConnectors()) - .map(JDiscServerConnector.class::cast) - .collect(toList()); - - server.setHandler( - getHandlerCollection( - serverConfig, - servletPathsConfig, - connectors, - jdiscServlet, - servletHolders, - jDiscFilterInvokerFilter)); + .map(JDiscServerConnector.class::cast) + .collect(toList()); + + server.setHandler(getHandlerCollection(serverConfig, + servletPathsConfig, + connectors, + jdiscServlet, + servletHolders, + jDiscFilterInvokerFilter)); int numMetricReporterThreads = 1; - metricReporterExecutor = Executors.newScheduledThreadPool( - numMetricReporterThreads, - new ThreadFactoryBuilder() - .setDaemon(true) - .setNameFormat(JettyHttpServer.class.getName() + "-MetricReporter-%d") - .setThreadFactory(threadFactory) - .build() - ); + metricReporterExecutor = + Executors.newScheduledThreadPool(numMetricReporterThreads, + new ThreadFactoryBuilder() + .setDaemon(true) + .setNameFormat(JettyHttpServer.class.getName() + "-MetricReporter-%d") + .setThreadFactory(threadFactory) + .build()); metricReporterExecutor.scheduleAtFixedRate(new MetricTask(), 0, 2, TimeUnit.SECONDS); } private static void initializeJettyLogging() { - // Note: Jetty is logging stderr if no logger is explicitly configured. + // Note: Jetty is logging stderr if no logger is explicitly configured try { Log.setLog(new JavaUtilLog()); } catch (Exception e) { @@ -208,32 +203,26 @@ public class JettyHttpServer extends AbstractServerProvider { private static void setupJmx(Server server, ServerConfig serverConfig) { if (serverConfig.jmx().enabled()) { System.setProperty("java.rmi.server.hostname", "localhost"); - server.addBean( - new MBeanContainer(ManagementFactory.getPlatformMBeanServer())); - server.addBean( - new ConnectorServer( - createJmxLoopbackOnlyServiceUrl(serverConfig.jmx().listenPort()), - "org.eclipse.jetty.jmx:name=rmiconnectorserver")); + server.addBean(new MBeanContainer(ManagementFactory.getPlatformMBeanServer())); + server.addBean(new ConnectorServer(createJmxLoopbackOnlyServiceUrl(serverConfig.jmx().listenPort()), + "org.eclipse.jetty.jmx:name=rmiconnectorserver")); } } private static JMXServiceURL createJmxLoopbackOnlyServiceUrl(int port) { try { - return new JMXServiceURL( - "rmi", "localhost", port, "/jndi/rmi://localhost:" + port + "/jmxrmi"); + return new JMXServiceURL("rmi", "localhost", port, "/jndi/rmi://localhost:" + port + "/jmxrmi"); } catch (MalformedURLException e) { throw new RuntimeException(e); } } - private HandlerCollection getHandlerCollection( - ServerConfig serverConfig, - ServletPathsConfig servletPathsConfig, - List<JDiscServerConnector> connectors, - ServletHolder jdiscServlet, - ComponentRegistry<ServletHolder> servletHolders, - FilterHolder jDiscFilterInvokerFilter) { - + private HandlerCollection getHandlerCollection(ServerConfig serverConfig, + ServletPathsConfig servletPathsConfig, + List<JDiscServerConnector> connectors, + ServletHolder jdiscServlet, + ComponentRegistry<ServletHolder> servletHolders, + FilterHolder jDiscFilterInvokerFilter) { ServletContextHandler servletContextHandler = createServletContextHandler(); servletHolders.allComponentsById().forEach((id, servlet) -> { @@ -257,7 +246,9 @@ public class JettyHttpServer extends AbstractServerProvider { GzipHandler gzipHandler = newGzipHandler(serverConfig); gzipHandler.setHandler(authEnforcer); - HttpResponseStatisticsCollector statisticsCollector = new HttpResponseStatisticsCollector(serverConfig.metric().monitoringHandlerPaths(), serverConfig.metric().searchHandlerPaths()); + HttpResponseStatisticsCollector statisticsCollector = + new HttpResponseStatisticsCollector(serverConfig.metric().monitoringHandlerPaths(), + serverConfig.metric().searchHandlerPaths()); statisticsCollector.setHandler(gzipHandler); StatisticsHandler statisticsHandler = newStatisticsHandler(); diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java index 9314247b83b..fd1f84f7d49 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java @@ -28,6 +28,7 @@ import java.util.logging.Logger; * it's important that errors are delivered synchronously. */ class ServletRequestReader implements ReadListener { + private enum State { READING, ALL_DATA_READ, REQUEST_CONTENT_CLOSED } @@ -136,7 +137,7 @@ class ServletRequestReader implements ReadListener { requestContentChannel.write(buf, writeCompletionHandler); metricReporter.successfulRead(bytesReceived); bytesRead += bytesReceived; - } catch (final Throwable t) { + } catch (Throwable t) { finishedFuture.completeExceptionally(t); } finally { //decrease due to this method completing. @@ -145,7 +146,7 @@ class ServletRequestReader implements ReadListener { } private void decreaseOutstandingUserCallsAndCloseRequestContentChannelConditionally() { - final boolean shouldCloseRequestContentChannel; + boolean shouldCloseRequestContentChannel; synchronized (monitor) { assertStateNotEquals(state, State.REQUEST_CONTENT_CLOSED); @@ -154,7 +155,7 @@ class ServletRequestReader implements ReadListener { numberOfOutstandingUserCalls -= 1; shouldCloseRequestContentChannel = numberOfOutstandingUserCalls == 0 && - (finishedFuture.isDone() || state == State.ALL_DATA_READ); + (finishedFuture.isDone() || state == State.ALL_DATA_READ); if (shouldCloseRequestContentChannel) { state = State.REQUEST_CONTENT_CLOSED; diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java index 1a8aa23668b..4ae824e2b7a 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java @@ -34,6 +34,7 @@ import static org.hamcrest.Matchers.equalTo; * @author ollivir */ public class HttpResponseStatisticsCollectorTest { + private Connector connector; private List<String> monitoringPaths = List.of("/status.html"); private List<String> searchPaths = List.of("/search"); @@ -41,7 +42,7 @@ public class HttpResponseStatisticsCollectorTest { private int httpResponseCode = 500; @Test - public void statistics_are_aggregated_by_category() throws Exception { + public void statistics_are_aggregated_by_category() { testRequest("http", 300, "GET"); testRequest("http", 301, "GET"); testRequest("http", 200, "GET"); @@ -52,7 +53,7 @@ public class HttpResponseStatisticsCollectorTest { } @Test - public void statistics_are_grouped_by_http_method_and_scheme() throws Exception { + public void statistics_are_grouped_by_http_method_and_scheme() { testRequest("http", 200, "GET"); testRequest("http", 200, "PUT"); testRequest("http", 200, "POST"); @@ -74,7 +75,7 @@ public class HttpResponseStatisticsCollectorTest { } @Test - public void statistics_include_grouped_and_single_statuscodes() throws Exception { + public void statistics_include_grouped_and_single_statuscodes() { testRequest("http", 401, "GET"); testRequest("http", 404, "GET"); testRequest("http", 403, "GET"); @@ -87,7 +88,7 @@ public class HttpResponseStatisticsCollectorTest { } @Test - public void retrieving_statistics_resets_the_counters() throws Exception { + public void retrieving_statistics_resets_the_counters() { testRequest("http", 200, "GET"); testRequest("http", 200, "GET"); @@ -101,7 +102,7 @@ public class HttpResponseStatisticsCollectorTest { } @Test - public void statistics_include_request_type_dimension() throws Exception { + public void statistics_include_request_type_dimension() { testRequest("http", 200, "GET", "/search"); testRequest("http", 200, "POST", "/search"); testRequest("http", 200, "POST", "/feed"); @@ -117,7 +118,14 @@ public class HttpResponseStatisticsCollectorTest { stats = collector.takeStatistics(); assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L); + } + + @Test + public void request_type_can_be_set_explicitly() { + testRequest("http", 200, "GET", "/search", com.yahoo.jdisc.Request.RequestType.WRITE); + var stats = collector.takeStatistics(); + assertStatisticsEntryWithRequestTypePresent(stats, "http", "GET", Metrics.RESPONSES_2XX, "write", 1L); } @Before @@ -145,13 +153,19 @@ public class HttpResponseStatisticsCollectorTest { server.start(); } - private Request testRequest(String scheme, int responseCode, String httpMethod) throws Exception { + private Request testRequest(String scheme, int responseCode, String httpMethod) { return testRequest(scheme, responseCode, httpMethod, "foo/bar"); } - private Request testRequest(String scheme, int responseCode, String httpMethod, String path) throws Exception { + private Request testRequest(String scheme, int responseCode, String httpMethod, String path) { + return testRequest(scheme, responseCode, httpMethod, path, null); + } + private Request testRequest(String scheme, int responseCode, String httpMethod, String path, + com.yahoo.jdisc.Request.RequestType explicitRequestType) { HttpChannel channel = new HttpChannel(connector, new HttpConfiguration(), null, new DummyTransport()); MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI(scheme + "://" + path), HttpVersion.HTTP_1_1, new HttpFields()); Request req = channel.getRequest(); + if (explicitRequestType != null) + req.setAttribute("requestType", explicitRequestType); req.setMetaData(metaData); this.httpResponseCode = responseCode; diff --git a/processing/src/main/java/com/yahoo/processing/Response.java b/processing/src/main/java/com/yahoo/processing/Response.java index e8504eb5087..59e6c3d22ab 100644 --- a/processing/src/main/java/com/yahoo/processing/Response.java +++ b/processing/src/main/java/com/yahoo/processing/Response.java @@ -39,16 +39,12 @@ public class Response extends ListenableFreezableClass { private final DataList<?> data; - /** - * Creates a request containing an empty array data list - */ + /** Creates a request containing an empty array data list */ public Response(Request request) { this(ArrayDataList.create(request)); } - /** - * Creates a response containing a list of data - */ + /** Creates a response containing a list of data */ public Response(DataList<?> data) { this.data = data; @@ -104,7 +100,7 @@ public class Response extends ListenableFreezableClass { return new CompleteAllOnGetFuture<D>(futures); } - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") private static <D extends Data> void collectCompletionFutures(DataList<D> dataList, List<ListenableFuture<DataList<D>>> futures) { futures.add(dataList.complete()); for (D data : dataList.asList()) { diff --git a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp index a1dc619b3e6..34a2d139498 100644 --- a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp +++ b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp @@ -28,7 +28,7 @@ #include <vespa/searchlib/attribute/attributevector.hpp> #include <vespa/searchlib/attribute/reference_attribute.h> #include <vespa/searchlib/tensor/dense_tensor_attribute.h> -#include <vespa/searchlib/tensor/generic_tensor_attribute.h> +#include <vespa/searchlib/tensor/serialized_tensor_attribute.h> #include <vespa/searchlib/test/weighted_type_test_utils.h> #include <vespa/vespalib/stllike/hash_map.hpp> #include <vespa/vespalib/testkit/testapp.h> @@ -48,7 +48,7 @@ using search::attribute::Reference; using search::attribute::ReferenceAttribute; using search::tensor::ITensorAttribute; using search::tensor::DenseTensorAttribute; -using search::tensor::GenericTensorAttribute; +using search::tensor::SerializedTensorAttribute; using search::tensor::TensorAttribute; using vespalib::eval::ValueType; using vespalib::eval::TensorSpec; @@ -457,7 +457,7 @@ TEST_F("require that tensor modify update is applied", } TEST_F("require that tensor add update is applied", - TensorFixture<GenericTensorAttribute>("tensor(x{})", "sparse_tensor")) + TensorFixture<SerializedTensorAttribute>("tensor(x{})", "sparse_tensor")) { f.setTensor(TensorSpec(f.type).add({{"x", "a"}}, 2)); f.applyValueUpdate(*f.attribute, 1, @@ -466,7 +466,7 @@ TEST_F("require that tensor add update is applied", } TEST_F("require that tensor add update to non-existing tensor creates empty tensor first", - TensorFixture<GenericTensorAttribute>("tensor(x{})", "sparse_tensor")) + TensorFixture<SerializedTensorAttribute>("tensor(x{})", "sparse_tensor")) { f.applyValueUpdate(*f.attribute, 1, TensorAddUpdate(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "a"}}, 3)))); @@ -474,7 +474,7 @@ TEST_F("require that tensor add update to non-existing tensor creates empty tens } TEST_F("require that tensor remove update is applied", - TensorFixture<GenericTensorAttribute>("tensor(x{})", "sparse_tensor")) + TensorFixture<SerializedTensorAttribute>("tensor(x{})", "sparse_tensor")) { f.setTensor(TensorSpec(f.type).add({{"x", "a"}}, 2).add({{"x", "b"}}, 3)); f.applyValueUpdate(*f.attribute, 1, diff --git a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp index efd17f773f3..1a342a92b3d 100644 --- a/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp +++ b/searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp @@ -12,11 +12,11 @@ #include <vespa/searchlib/tensor/dense_tensor_attribute.h> #include <vespa/searchlib/tensor/direct_tensor_attribute.h> #include <vespa/searchlib/tensor/doc_vector_access.h> -#include <vespa/searchlib/tensor/generic_tensor_attribute.h> #include <vespa/searchlib/tensor/hnsw_index.h> #include <vespa/searchlib/tensor/nearest_neighbor_index.h> #include <vespa/searchlib/tensor/nearest_neighbor_index_factory.h> #include <vespa/searchlib/tensor/nearest_neighbor_index_saver.h> +#include <vespa/searchlib/tensor/serialized_tensor_attribute.h> #include <vespa/searchlib/tensor/tensor_attribute.h> #include <vespa/searchlib/test/directory_handler.h> #include <vespa/searchlib/util/fileutil.h> @@ -40,7 +40,7 @@ using search::tensor::DefaultNearestNeighborIndexFactory; using search::tensor::DenseTensorAttribute; using search::tensor::DirectTensorAttribute; using search::tensor::DocVectorAccess; -using search::tensor::GenericTensorAttribute; +using search::tensor::SerializedTensorAttribute; using search::tensor::HnswIndex; using search::tensor::HnswNode; using search::tensor::NearestNeighborIndex; @@ -361,7 +361,7 @@ struct Fixture { } else if (_traits.use_direct_tensor_attribute) { return std::make_shared<DirectTensorAttribute>(_name, _cfg); } else { - return std::make_shared<GenericTensorAttribute>(_name, _cfg); + return std::make_shared<SerializedTensorAttribute>(_name, _cfg); } } diff --git a/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp b/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp index a0cf47f64e0..148d18f79ff 100644 --- a/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp +++ b/searchlib/src/vespa/searchlib/attribute/createsinglestd.cpp @@ -7,8 +7,8 @@ #include "singlenumericattribute.hpp" #include "singlestringattribute.h" #include "singleboolattribute.h" -#include <vespa/searchlib/tensor/generic_tensor_attribute.h> #include <vespa/searchlib/tensor/dense_tensor_attribute.h> +#include <vespa/searchlib/tensor/serialized_tensor_attribute.h> namespace search { @@ -46,7 +46,7 @@ AttributeFactory::createSingleStd(stringref name, const Config & info) if (info.tensorType().is_dense()) { return std::make_shared<tensor::DenseTensorAttribute>(name, info); } else { - return std::make_shared<tensor::GenericTensorAttribute>(name, info); + return std::make_shared<tensor::SerializedTensorAttribute>(name, info); } case BasicType::REFERENCE: return std::make_shared<attribute::ReferenceAttribute>(name, info); diff --git a/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt b/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt index 55e83fc6147..fac6d015a5f 100644 --- a/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/tensor/CMakeLists.txt @@ -10,9 +10,6 @@ vespa_add_library(searchlib_tensor OBJECT direct_tensor_saver.cpp distance_function_factory.cpp distance_functions.cpp - generic_tensor_attribute.cpp - generic_tensor_attribute_saver.cpp - generic_tensor_store.cpp hnsw_graph.cpp hnsw_index.cpp hnsw_index_loader.cpp @@ -22,6 +19,9 @@ vespa_add_library(searchlib_tensor OBJECT inv_log_level_generator.cpp nearest_neighbor_index.cpp nearest_neighbor_index_saver.cpp + serialized_tensor_attribute.cpp + serialized_tensor_attribute_saver.cpp + serialized_tensor_store.cpp tensor_attribute.cpp tensor_deserialize.cpp tensor_store.cpp diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute_saver.h b/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute_saver.h deleted file mode 100644 index 92beef49136..00000000000 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute_saver.h +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/searchlib/attribute/attributesaver.h> -#include "tensor_attribute.h" - -namespace search { - -namespace tensor { - -class GenericTensorStore; - -/* - * Class for saving a tensor attribute. - */ -class GenericTensorAttributeSaver : public AttributeSaver -{ -public: - using RefCopyVector = TensorAttribute::RefCopyVector; -private: - RefCopyVector _refs; - const GenericTensorStore &_tensorStore; - using GenerationHandler = vespalib::GenerationHandler; - - virtual bool onSave(IAttributeSaveTarget &saveTarget) override; -public: - GenericTensorAttributeSaver(GenerationHandler::Guard &&guard, - const attribute::AttributeHeader &header, - RefCopyVector &&refs, - const GenericTensorStore &tensorStore); - - virtual ~GenericTensorAttributeSaver(); -}; - -} // namespace search::tensor - -} // namespace search diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute.cpp index 6864fb52120..d4a20abf2fd 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute.cpp @@ -1,9 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "generic_tensor_attribute.h" -#include "generic_tensor_attribute_saver.h" -#include "tensor_attribute.hpp" #include "blob_sequence_reader.h" +#include "serialized_tensor_attribute.h" +#include "serialized_tensor_attribute_saver.h" +#include "tensor_attribute.hpp" #include <vespa/eval/tensor/tensor.h> #include <vespa/fastlib/io/bufferedfile.h> #include <vespa/searchlib/attribute/readerbase.h> @@ -21,29 +21,29 @@ constexpr uint32_t TENSOR_ATTRIBUTE_VERSION = 0; } -GenericTensorAttribute::GenericTensorAttribute(stringref name, const Config &cfg) - : TensorAttribute(name, cfg, _genericTensorStore) +SerializedTensorAttribute::SerializedTensorAttribute(stringref name, const Config &cfg) + : TensorAttribute(name, cfg, _serializedTensorStore) { } -GenericTensorAttribute::~GenericTensorAttribute() +SerializedTensorAttribute::~SerializedTensorAttribute() { getGenerationHolder().clearHoldLists(); _tensorStore.clearHoldLists(); } void -GenericTensorAttribute::setTensor(DocId docId, const Tensor &tensor) +SerializedTensorAttribute::setTensor(DocId docId, const Tensor &tensor) { checkTensorType(tensor); - EntryRef ref = _genericTensorStore.setTensor(tensor); + EntryRef ref = _serializedTensorStore.setTensor(tensor); setTensorRef(docId, ref); } std::unique_ptr<Tensor> -GenericTensorAttribute::getTensor(DocId docId) const +SerializedTensorAttribute::getTensor(DocId docId) const { EntryRef ref; if (docId < getCommittedDocIdLimit()) { @@ -52,17 +52,17 @@ GenericTensorAttribute::getTensor(DocId docId) const if (!ref.valid()) { return std::unique_ptr<Tensor>(); } - return _genericTensorStore.getTensor(ref); + return _serializedTensorStore.getTensor(ref); } void -GenericTensorAttribute::getTensor(DocId, vespalib::tensor::MutableDenseTensorView &) const +SerializedTensorAttribute::getTensor(DocId, vespalib::tensor::MutableDenseTensorView &) const { notImplemented(); } bool -GenericTensorAttribute::onLoad() +SerializedTensorAttribute::onLoad() { BlobSequenceReader tensorReader(*this); if (!tensorReader.hasData()) { @@ -75,7 +75,7 @@ GenericTensorAttribute::onLoad() _refVector.unsafe_reserve(numDocs); for (uint32_t lid = 0; lid < numDocs; ++lid) { uint32_t tensorSize = tensorReader.getNextSize(); - auto raw = _genericTensorStore.allocRawBuffer(tensorSize); + auto raw = _serializedTensorStore.allocRawBuffer(tensorSize); if (tensorSize != 0) { tensorReader.readBlob(raw.data, tensorSize); } @@ -88,21 +88,21 @@ GenericTensorAttribute::onLoad() std::unique_ptr<AttributeSaver> -GenericTensorAttribute::onInitSave(vespalib::stringref fileName) +SerializedTensorAttribute::onInitSave(vespalib::stringref fileName) { vespalib::GenerationHandler::Guard guard(getGenerationHandler(). takeGuard()); - return std::make_unique<GenericTensorAttributeSaver> + return std::make_unique<SerializedTensorAttributeSaver> (std::move(guard), this->createAttributeHeader(fileName), getRefCopy(), - _genericTensorStore); + _serializedTensorStore); } void -GenericTensorAttribute::compactWorst() +SerializedTensorAttribute::compactWorst() { - doCompactWorst<GenericTensorStore::RefType>(); + doCompactWorst<SerializedTensorStore::RefType>(); } } diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute.h index 9dd3788511e..5596341a5b7 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute.h +++ b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute.h @@ -2,22 +2,19 @@ #pragma once +#include "serialized_tensor_store.h" #include "tensor_attribute.h" -#include "generic_tensor_store.h" -namespace search { - -namespace tensor { +namespace search::tensor { /** * Attribute vector class used to store tensors for all documents in memory. */ -class GenericTensorAttribute : public TensorAttribute -{ - GenericTensorStore _genericTensorStore; // data store for serialized tensors +class SerializedTensorAttribute : public TensorAttribute { + SerializedTensorStore _serializedTensorStore; // data store for serialized tensors public: - GenericTensorAttribute(vespalib::stringref baseFileName, const Config &cfg); - virtual ~GenericTensorAttribute(); + SerializedTensorAttribute(vespalib::stringref baseFileName, const Config &cfg); + virtual ~SerializedTensorAttribute(); virtual void setTensor(DocId docId, const Tensor &tensor) override; virtual std::unique_ptr<Tensor> getTensor(DocId docId) const override; virtual void getTensor(DocId docId, vespalib::tensor::MutableDenseTensorView &tensor) const override; @@ -26,7 +23,4 @@ public: virtual void compactWorst() override; }; - -} // namespace search::tensor - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute_saver.cpp b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute_saver.cpp index 81ec3a5218e..4c41c3a449e 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_attribute_saver.cpp +++ b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute_saver.cpp @@ -1,21 +1,19 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "generic_tensor_attribute_saver.h" -#include "generic_tensor_store.h" +#include "serialized_tensor_attribute_saver.h" +#include "serialized_tensor_store.h" #include <vespa/searchlib/util/bufferwriter.h> #include <vespa/searchlib/attribute/iattributesavetarget.h> using vespalib::GenerationHandler; -namespace search { +namespace search::tensor { -namespace tensor { - -GenericTensorAttributeSaver:: -GenericTensorAttributeSaver(GenerationHandler::Guard &&guard, - const attribute::AttributeHeader &header, - RefCopyVector &&refs, - const GenericTensorStore &tensorStore) +SerializedTensorAttributeSaver:: +SerializedTensorAttributeSaver(GenerationHandler::Guard &&guard, + const attribute::AttributeHeader &header, + RefCopyVector &&refs, + const SerializedTensorStore &tensorStore) : AttributeSaver(std::move(guard), header), _refs(std::move(refs)), _tensorStore(tensorStore) @@ -23,13 +21,13 @@ GenericTensorAttributeSaver(GenerationHandler::Guard &&guard, } -GenericTensorAttributeSaver::~GenericTensorAttributeSaver() +SerializedTensorAttributeSaver::~SerializedTensorAttributeSaver() { } bool -GenericTensorAttributeSaver::onSave(IAttributeSaveTarget &saveTarget) +SerializedTensorAttributeSaver::onSave(IAttributeSaveTarget &saveTarget) { std::unique_ptr<BufferWriter> datWriter(saveTarget.datWriter().allocBufferWriter()); @@ -45,7 +43,4 @@ GenericTensorAttributeSaver::onSave(IAttributeSaveTarget &saveTarget) return true; } - -} // namespace search::tensor - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute_saver.h b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute_saver.h new file mode 100644 index 00000000000..1ae2279b893 --- /dev/null +++ b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_attribute_saver.h @@ -0,0 +1,33 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/searchlib/attribute/attributesaver.h> +#include "tensor_attribute.h" + +namespace search::tensor { + +class SerializedTensorStore; + +/* + * Class for saving a tensor attribute. + */ +class SerializedTensorAttributeSaver : public AttributeSaver { +public: + using RefCopyVector = TensorAttribute::RefCopyVector; +private: + RefCopyVector _refs; + const SerializedTensorStore &_tensorStore; + using GenerationHandler = vespalib::GenerationHandler; + + virtual bool onSave(IAttributeSaveTarget &saveTarget) override; +public: + SerializedTensorAttributeSaver(GenerationHandler::Guard &&guard, + const attribute::AttributeHeader &header, + RefCopyVector &&refs, + const SerializedTensorStore &tensorStore); + + virtual ~SerializedTensorAttributeSaver(); +}; + +} diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.cpp b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_store.cpp index 8c695c32719..77903291e13 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.cpp +++ b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_store.cpp @@ -1,6 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "generic_tensor_store.h" +#include "serialized_tensor_store.h" #include "tensor_deserialize.h" #include <vespa/eval/tensor/tensor.h> #include <vespa/eval/tensor/serialization/typed_binary_format.h> @@ -17,7 +17,7 @@ namespace search::tensor { constexpr size_t MIN_BUFFER_ARRAYS = 1024; -GenericTensorStore::GenericTensorStore() +SerializedTensorStore::SerializedTensorStore() : TensorStore(_concreteStore), _concreteStore(), _bufferType(RefType::align(1), @@ -28,13 +28,13 @@ GenericTensorStore::GenericTensorStore() _store.initActiveBuffers(); } -GenericTensorStore::~GenericTensorStore() +SerializedTensorStore::~SerializedTensorStore() { _store.dropBuffers(); } std::pair<const void *, uint32_t> -GenericTensorStore::getRawBuffer(RefType ref) const +SerializedTensorStore::getRawBuffer(RefType ref) const { if (!ref.valid()) { return std::make_pair(nullptr, 0u); @@ -45,7 +45,7 @@ GenericTensorStore::getRawBuffer(RefType ref) const } Handle<char> -GenericTensorStore::allocRawBuffer(uint32_t size) +SerializedTensorStore::allocRawBuffer(uint32_t size) { if (size == 0) { return Handle<char>(); @@ -63,7 +63,7 @@ GenericTensorStore::allocRawBuffer(uint32_t size) } void -GenericTensorStore::holdTensor(EntryRef ref) +SerializedTensorStore::holdTensor(EntryRef ref) { if (!ref.valid()) { return; @@ -75,7 +75,7 @@ GenericTensorStore::holdTensor(EntryRef ref) } TensorStore::EntryRef -GenericTensorStore::move(EntryRef ref) +SerializedTensorStore::move(EntryRef ref) { if (!ref.valid()) { return RefType(); @@ -88,7 +88,7 @@ GenericTensorStore::move(EntryRef ref) } std::unique_ptr<Tensor> -GenericTensorStore::getTensor(EntryRef ref) const +SerializedTensorStore::getTensor(EntryRef ref) const { auto raw = getRawBuffer(ref); if (raw.second == 0u) { @@ -98,7 +98,7 @@ GenericTensorStore::getTensor(EntryRef ref) const } TensorStore::EntryRef -GenericTensorStore::setTensor(const Tensor &tensor) +SerializedTensorStore::setTensor(const Tensor &tensor) { vespalib::nbostream stream; TypedBinaryFormat::serialize(stream, tensor); diff --git a/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.h b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_store.h index 14b65e1ec5a..7c0a8e5ed16 100644 --- a/searchlib/src/vespa/searchlib/tensor/generic_tensor_store.h +++ b/searchlib/src/vespa/searchlib/tensor/serialized_tensor_store.h @@ -4,9 +4,7 @@ #include "tensor_store.h" -namespace search { - -namespace tensor { +namespace search::tensor { /** * Class for storing serialized tensors in memory, used by TensorAttribute. @@ -15,8 +13,7 @@ namespace tensor { * might also require corresponding changes to implemented optimized tensor * operations that use the serialized tensor as argument. */ -class GenericTensorStore : public TensorStore -{ +class SerializedTensorStore : public TensorStore { public: using RefType = vespalib::datastore::AlignedEntryRefT<22, 2>; using DataStoreType = vespalib::datastore::DataStoreT<RefType>; @@ -24,9 +21,9 @@ private: DataStoreType _concreteStore; vespalib::datastore::BufferType<char> _bufferType; public: - GenericTensorStore(); + SerializedTensorStore(); - virtual ~GenericTensorStore(); + virtual ~SerializedTensorStore(); std::pair<const void *, uint32_t> getRawBuffer(RefType ref) const; @@ -41,7 +38,4 @@ public: EntryRef setTensor(const Tensor &tensor); }; - -} // namespace search::tensor - -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/transactionlog/common.cpp b/searchlib/src/vespa/searchlib/transactionlog/common.cpp index 965f88d942f..40a065277be 100644 --- a/searchlib/src/vespa/searchlib/transactionlog/common.cpp +++ b/searchlib/src/vespa/searchlib/transactionlog/common.cpp @@ -3,6 +3,7 @@ #include "common.h" #include <vespa/vespalib/util/stringfmt.h> #include <vespa/fastos/file.h> +#include <stdexcept> namespace search::transactionlog { diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java index cf33b6033cd..6000026580a 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java @@ -3,11 +3,11 @@ package com.yahoo.document.restapi.resource; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.inject.Inject; - import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.inject.Inject; import com.yahoo.cloud.config.ClusterListConfig; import com.yahoo.container.handler.ThreadpoolConfig; +import com.yahoo.container.handler.threadpool.ContainerThreadPool; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; @@ -15,7 +15,6 @@ import com.yahoo.container.logging.AccessLog; import com.yahoo.document.DocumentTypeManager; import com.yahoo.document.TestAndSetCondition; import com.yahoo.document.config.DocumentmanagerConfig; - import com.yahoo.document.json.SingleDocumentParser; import com.yahoo.document.restapi.OperationHandler; import com.yahoo.document.restapi.OperationHandlerImpl; @@ -27,11 +26,11 @@ import com.yahoo.document.select.parser.ParseException; import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess; import com.yahoo.documentapi.messagebus.MessageBusParams; import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; -import java.util.logging.Level; +import com.yahoo.jdisc.Metric; import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.text.Text; -import com.yahoo.vespa.config.content.LoadTypeConfig; import com.yahoo.vespa.config.content.AllClustersBucketSpacesConfig; +import com.yahoo.vespa.config.content.LoadTypeConfig; import com.yahoo.vespaclient.ClusterDef; import com.yahoo.vespaclient.ClusterList; import com.yahoo.vespaxmlparser.DocumentFeedOperation; @@ -46,6 +45,7 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Level; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; @@ -77,11 +77,16 @@ public class RestApi extends LoggingRequestHandler { private final AtomicInteger threadsAvailableForApi; @Inject - public RestApi(LoggingRequestHandler.Context parentCtx, DocumentmanagerConfig documentManagerConfig, - LoadTypeConfig loadTypeConfig, ThreadpoolConfig threadpoolConfig, + public RestApi(ContainerThreadPool threadpool, + AccessLog accessLog, + Metric metric, + DocumentmanagerConfig documentManagerConfig, + LoadTypeConfig loadTypeConfig, + ThreadpoolConfig threadpoolConfig, AllClustersBucketSpacesConfig bucketSpacesConfig, - ClusterListConfig clusterListConfig, MetricReceiver metricReceiver) { - super(parentCtx); + ClusterListConfig clusterListConfig, + MetricReceiver metricReceiver) { + super(threadpool.executor(), accessLog, metric); MessageBusParams params = new MessageBusParams(new LoadTypeSet(loadTypeConfig)); params.setDocumentmanagerConfig(documentManagerConfig); this.operationHandler = new OperationHandlerImpl(new MessageBusDocumentAccess(params), diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java index 1df0ce3594b..7776c8fa34c 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java @@ -3,12 +3,15 @@ package com.yahoo.vespa.http.server; import com.yahoo.collections.Tuple2; import com.yahoo.container.handler.ThreadpoolConfig; +import com.yahoo.container.handler.threadpool.ContainerThreadPool; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.container.jdisc.messagebus.SessionCache; +import com.yahoo.container.logging.AccessLog; import com.yahoo.document.config.DocumentmanagerConfig; import com.yahoo.documentapi.metrics.DocumentApiMetrics; +import com.yahoo.jdisc.Metric; import com.yahoo.messagebus.ReplyHandler; import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.vespa.http.client.core.Headers; @@ -39,15 +42,17 @@ public class FeedHandler extends LoggingRequestHandler { private final DocumentApiMetrics metricsHelper; @Inject - public FeedHandler(LoggingRequestHandler.Context parentCtx, + public FeedHandler(ContainerThreadPool threadpool, + Metric metric, + AccessLog accessLog, DocumentmanagerConfig documentManagerConfig, SessionCache sessionCache, ThreadpoolConfig threadpoolConfig, MetricReceiver metricReceiver) throws Exception { - super(parentCtx); + super(threadpool.executor(), accessLog, metric); metricsHelper = new DocumentApiMetrics(metricReceiver, "vespa.http.server"); - feedHandlerV3 = new FeedHandlerV3(parentCtx, documentManagerConfig, sessionCache, threadpoolConfig, metricsHelper); - feedReplyHandler = new FeedReplyReader(parentCtx.getMetric(), metricsHelper); + feedHandlerV3 = new FeedHandlerV3(threadpool.executor(), metric, accessLog, documentManagerConfig, sessionCache, threadpoolConfig, metricsHelper); + feedReplyHandler = new FeedReplyReader(metric, metricsHelper); } private Tuple2<HttpResponse, Integer> checkProtocolVersion(HttpRequest request) { diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java index a932ca935e0..9bd48b707f8 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java @@ -7,26 +7,26 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.container.jdisc.messagebus.SessionCache; +import com.yahoo.container.logging.AccessLog; import com.yahoo.document.DocumentTypeManager; import com.yahoo.document.config.DocumentmanagerConfig; import com.yahoo.documentapi.metrics.DocumentApiMetrics; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.ReferencedResource; -import java.util.logging.Level; import com.yahoo.messagebus.ReplyHandler; import com.yahoo.messagebus.SourceSessionParams; import com.yahoo.messagebus.shared.SharedSourceSession; import com.yahoo.vespa.http.client.core.Headers; import com.yahoo.yolean.Exceptions; -import java.time.Duration; -import java.time.Instant; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.Executor; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -48,18 +48,20 @@ public class FeedHandlerV3 extends LoggingRequestHandler { private final AtomicInteger threadsAvailableForFeeding; private static final Logger log = Logger.getLogger(FeedHandlerV3.class.getName()); - public FeedHandlerV3(LoggingRequestHandler.Context parentCtx, + public FeedHandlerV3(Executor executor, + Metric metric, + AccessLog accessLog, DocumentmanagerConfig documentManagerConfig, SessionCache sessionCache, ThreadpoolConfig threadpoolConfig, DocumentApiMetrics metricsHelper) { - super(parentCtx); + super(executor, accessLog, metric); docTypeManager = new DocumentTypeManager(documentManagerConfig); this.sessionCache = sessionCache; - feedReplyHandler = new FeedReplyReader(parentCtx.getMetric(), metricsHelper); + feedReplyHandler = new FeedReplyReader(metric, metricsHelper); cron = new ScheduledThreadPoolExecutor(1, ThreadFactoryFactory.getThreadFactory("feedhandlerv3.cron")); cron.scheduleWithFixedDelay(this::removeOldClients, 16, 11, TimeUnit.MINUTES); - this.metric = parentCtx.getMetric(); + this.metric = metric; // 40% of the threads can be blocking on feeding before we deny requests. if (threadpoolConfig != null) { threadsAvailableForFeeding = new AtomicInteger(Math.max((int) (0.4 * threadpoolConfig.maxthreads()), 1)); diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java index 1efa8129cdb..a6cc52e8aea 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java @@ -120,7 +120,9 @@ public class FeedTesterV3 { Executor threadPool = Executors.newCachedThreadPool(); DocumentmanagerConfig docMan = new DocumentmanagerConfig(new DocumentmanagerConfig.Builder().enablecompression(true)); FeedHandlerV3 feedHandlerV3 = new FeedHandlerV3( - new FeedHandlerV3.Context(threadPool, AccessLog.voidAccessLog(), metric), + threadPool, + metric, + AccessLog.voidAccessLog(), docMan, null /* session cache */, threadPoolConfig /* thread pool config */, |