summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJon Bratseth <bratseth@gmail.com>2020-09-02 16:42:10 +0200
committerJon Bratseth <bratseth@gmail.com>2020-09-02 16:42:10 +0200
commit3c9de78274d5f7e5d7c7c19105e4925a91103e9e (patch)
tree1e0027dc08663b890ecdc9b68b6543fadb394691
parent5a10a3cc2bbad52d783e75a92c1527e33e976fe9 (diff)
Allow setting a request type explicitly
This lets handler authors control the requestType explicitly by setting it on the HttpResponse, which is useful to avoid misclassification of POST requests to reading handlers as writes.
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionContentReadResponse.java3
-rw-r--r--container-core/abi-spec.json4
-rw-r--r--container-core/src/main/java/com/yahoo/container/jdisc/HttpResponse.java29
-rw-r--r--container-core/src/main/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandler.java1
-rw-r--r--container-search-and-docproc/src/main/java/com/yahoo/container/handler/observability/ApplicationStatusHandler.java1
-rw-r--r--container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java1
-rw-r--r--container-search/src/main/java/com/yahoo/search/Result.java3
-rw-r--r--container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java2
-rw-r--r--jdisc_core/src/main/java/com/yahoo/jdisc/Request.java11
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java38
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java72
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java36
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java1
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java101
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java7
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java28
-rw-r--r--processing/src/main/java/com/yahoo/processing/Response.java10
17 files changed, 183 insertions, 165 deletions
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-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/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()) {