diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-12-21 16:35:13 +0100 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2021-12-21 16:35:13 +0100 |
commit | e26a12f0419ebf6cfbb1452ec8b590b1254b7095 (patch) | |
tree | 8e3973e6baa84c3c4d07e6257765f4df0d240637 /vespaclient-container-plugin | |
parent | 7a8ed56455a729716d1c334777df0c9856578e93 (diff) |
Add metric for update-doc-not-found, and update httpapi metrics from /doc/v1
Diffstat (limited to 'vespaclient-container-plugin')
5 files changed, 70 insertions, 22 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index a102e8fffd4..2a7e69a0bf4 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -34,6 +34,7 @@ import com.yahoo.documentapi.DocumentAccess; import com.yahoo.documentapi.DocumentOperationParameters; import com.yahoo.documentapi.DocumentResponse; import com.yahoo.documentapi.ProgressToken; +import com.yahoo.documentapi.Response.Outcome; import com.yahoo.documentapi.Result; import com.yahoo.documentapi.VisitorControlHandler; import com.yahoo.documentapi.VisitorDataHandler; @@ -65,6 +66,7 @@ import com.yahoo.restapi.Path; import com.yahoo.search.query.ParameterParser; import com.yahoo.text.Text; import com.yahoo.vespa.config.content.AllClustersBucketSpacesConfig; +import com.yahoo.vespa.http.server.MetricNames; import com.yahoo.yolean.Exceptions; import com.yahoo.yolean.Exceptions.RunnableThrowingIOException; @@ -93,8 +95,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.Phaser; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.Lock; @@ -450,7 +450,8 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE) .withResponseHandler(response -> { outstanding.decrementAndGet(); - handle(path, handler, response); + updatePutMetrics(response.outcome()); + handleFeedOperation(path, handler, response); }); return () -> dispatchOperation(() -> asyncSession.put(put, parameters)); }); @@ -467,7 +468,8 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE) .withResponseHandler(response -> { outstanding.decrementAndGet(); - handle(path, handler, response); + updateUpdateMetrics(response.outcome()); + handleFeedOperation(path, handler, response); }); return () -> dispatchOperation(() -> asyncSession.update(update, parameters)); }); @@ -482,7 +484,8 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { DocumentOperationParameters parameters = parametersFromRequest(request, ROUTE) .withResponseHandler(response -> { outstanding.decrementAndGet(); - handle(path, handler, response); + updateRemoveMetrics(response.outcome()); + handleFeedOperation(path, handler, response); }); return () -> dispatchOperation(() -> asyncSession.remove(remove, parameters)); }); @@ -1032,10 +1035,35 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { } } - private static void handle(DocumentPath path, ResponseHandler handler, com.yahoo.documentapi.Response response) { + private static void handleFeedOperation(DocumentPath path, ResponseHandler handler, com.yahoo.documentapi.Response response) { handle(path, handler, response, (document, jsonResponse) -> jsonResponse.commit(Response.Status.OK)); } + private void updatePutMetrics(Outcome outcome) { + switch (outcome) { + case SUCCESS: metric.add(MetricNames.SUCCEEDED, 1, null); break; + case CONDITION_FAILED: metric.add(MetricNames.CONDITION_NOT_MET, 1, null); break; + default: metric.add(MetricNames.FAILED, 1, null); break; + } + } + + private void updateUpdateMetrics(Outcome outcome) { + switch (outcome) { + case SUCCESS: metric.add(MetricNames.SUCCEEDED, 1, null); break; + case NOT_FOUND: metric.add(MetricNames.NOT_FOUND, 1, null); break; + case CONDITION_FAILED: metric.add(MetricNames.CONDITION_NOT_MET, 1, null); break; + default: metric.add(MetricNames.FAILED, 1, null); break; + } + } + + private void updateRemoveMetrics(Outcome outcome) { + switch (outcome) { + case SUCCESS: metric.add(MetricNames.SUCCEEDED, 1, null); break; + case CONDITION_FAILED: metric.add(MetricNames.CONDITION_NOT_MET, 1, null); break; + default: metric.add(MetricNames.FAILED, 1, null); break; + } + } + // ------------------------------------------------- Visits ------------------------------------------------ private VisitorParameters parseGetParameters(HttpRequest request, DocumentPath path, boolean streamed) { diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java index 522652212a9..97e5e9e63d8 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/documentapi/metrics/DocumentOperationStatus.java @@ -19,15 +19,6 @@ public enum DocumentOperationStatus { OK, REQUEST_ERROR, SERVER_ERROR; - public static DocumentOperationStatus fromHttpStatusCode(int httpStatus) { - switch (httpStatus / 100) { - case 2: return OK; - case 4: return REQUEST_ERROR; - case 5: return SERVER_ERROR; - default: return null; - } - } - public static DocumentOperationStatus fromMessageBusErrorCodes(Set<Integer> errorCodes) { if (errorCodes.size() == 1 && errorCodes.contains(DocumentProtocol.ERROR_NO_SPACE)) return SERVER_ERROR; diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java index 50f79c0a828..b71b5d79520 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedReplyReader.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.http.server; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; +import com.yahoo.documentapi.messagebus.protocol.UpdateDocumentReply; import com.yahoo.documentapi.metrics.DocumentApiMetrics; import com.yahoo.documentapi.metrics.DocumentOperationStatus; import com.yahoo.documentapi.metrics.DocumentOperationType; @@ -54,14 +55,20 @@ public class FeedReplyReader implements ReplyHandler { } else { metricsHelper.reportSuccessful(type, latencyInSeconds); metric.add(MetricNames.SUCCEEDED, 1, null); - if (!conditionMet) + if ( ! conditionMet) metric.add(MetricNames.CONDITION_NOT_MET, 1, testAndSetMetricCtx); + if ( ! updateNotFound(reply)) + metric.add(MetricNames.NOT_FOUND, 1, null); enqueue(context, "Document processed.", ErrorCode.OK, !conditionMet, reply.getTrace()); } } private static boolean conditionMet(Reply reply) { - return !reply.hasErrors() || reply.getError(0).getCode() != DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED; + return ! reply.hasErrors() || reply.getError(0).getCode() != DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED; + } + + private static boolean updateNotFound(Reply reply) { + return reply instanceof UpdateDocumentReply && ! ((UpdateDocumentReply) reply).wasFound(); } private void enqueue(ReplyContext context, String message, ErrorCode status, boolean isConditionNotMet, Trace trace) { diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java index 6ded410ff68..a5987f2398e 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/MetricNames.java @@ -19,6 +19,7 @@ public final class MetricNames { public static final String LATENCY = PREFIX + "latency"; public static final String FAILED = PREFIX + "failed"; public static final String CONDITION_NOT_MET = PREFIX + "condition_not_met"; + public static final String NOT_FOUND = PREFIX + "not_found"; public static final String PARSE_ERROR = PREFIX + "parse_error"; public static final String SUCCEEDED = PREFIX + "succeeded"; public static final String PENDING = PREFIX + "pending"; diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java index de907f70c19..232d49129d7 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java @@ -3,7 +3,6 @@ package com.yahoo.document.restapi.resource; import com.yahoo.cloud.config.ClusterListConfig; import com.yahoo.container.jdisc.RequestHandlerTestDriver; -import com.yahoo.docproc.jdisc.metric.NullMetric; import com.yahoo.document.BucketId; import com.yahoo.document.Document; import com.yahoo.document.DocumentId; @@ -44,6 +43,7 @@ import com.yahoo.documentapi.VisitorResponse; import com.yahoo.documentapi.VisitorSession; import com.yahoo.documentapi.messagebus.protocol.PutDocumentMessage; import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.test.MockMetric; import com.yahoo.messagebus.StaticThrottlePolicy; import com.yahoo.messagebus.Trace; import com.yahoo.messagebus.TraceNode; @@ -126,7 +126,7 @@ public class DocumentV1ApiTest { Map.of("music", "default"))); ManualClock clock; MockDocumentAccess access; - Metric metric; + MockMetric metric; MetricReceiver metrics; DocumentV1ApiHandler handler; @@ -134,7 +134,7 @@ public class DocumentV1ApiTest { public void setUp() { clock = new ManualClock(); access = new MockDocumentAccess(docConfig); - metric = new NullMetric(); + metric = new MockMetric(); metrics = new MetricReceiver.MockReceiver(); handler = new DocumentV1ApiHandler(clock, Duration.ofMillis(1), metric, metrics, access, docConfig, executorConfig, clusterConfig, bucketConfig); @@ -614,6 +614,23 @@ public class DocumentV1ApiTest { "}", response.readAll()); assertEquals(400, response.getStatus()); + // PUT on document which is not found is a 200 + access.session.expect((update, parameters) -> { + parameters.responseHandler().get().handleResponse(new UpdateResponse(0, false)); + return new Result(Result.ResultType.SUCCESS, null); + }); + response = driver.sendRequest("http://localhost/document/v1/space/music/docid/sonny", PUT, + "{" + + " \"fields\": {" + + " \"artist\": { \"assign\": \"The Shivers\" }" + + " }" + + "}"); + assertSameJson("{" + + " \"pathId\": \"/document/v1/space/music/docid/sonny\"," + + " \"id\": \"id:space:music::sonny\"" + + "}", response.readAll()); + assertEquals(200, response.getStatus()); + // DELETE with full document ID is a document remove operation. access.session.expect((remove, parameters) -> { DocumentRemove expectedRemove = new DocumentRemove(doc2.getId()); @@ -667,7 +684,7 @@ public class DocumentV1ApiTest { parameters.responseHandler().get().handleResponse(new Response(0, "disk full", Response.Outcome.INSUFFICIENT_STORAGE)); return new Result(Result.ResultType.SUCCESS, null); }); - response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two"); + response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", DELETE); assertSameJson("{" + " \"pathId\": \"/document/v1/space/music/number/1/two\"," + " \"id\": \"id:space:music:n=1:two\"," + @@ -680,7 +697,7 @@ public class DocumentV1ApiTest { parameters.responseHandler().get().handleResponse(new Response(0, "no dice", Response.Outcome.CONDITION_FAILED)); return new Result(Result.ResultType.SUCCESS, null); }); - response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two"); + response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", DELETE); assertSameJson("{" + " \"pathId\": \"/document/v1/space/music/number/1/two\"," + " \"id\": \"id:space:music:n=1:two\"," + @@ -746,6 +763,10 @@ public class DocumentV1ApiTest { handler.get().handleResponse(new Response(0)); // response may eventually arrive, but too late. } + assertEquals(3, metric.metrics().get("httpapi_succeeded").get(Map.of()), 0); + assertEquals(1, metric.metrics().get("httpapi_condition_not_met").get(Map.of()), 0); + assertEquals(1, metric.metrics().get("httpapi_not_found").get(Map.of()), 0); + assertEquals(1, metric.metrics().get("httpapi_failed").get(Map.of()), 0); driver.close(); } |