From c22fe4a898f144d85c667bcf20877d2905f88e12 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 25 Sep 2023 17:20:22 +1000 Subject: [PATCH 01/17] WIP --- .../smoke_test_multinode/60_http_stats.yml | 22 +++++ .../org/elasticsearch/TransportVersions.java | 1 + .../elasticsearch/common/path/PathTrie.java | 15 ++++ .../http/AbstractHttpServerTransport.java | 7 +- .../elasticsearch/http/HttpRouteStats.java | 90 +++++++++++++++++++ .../http/HttpRouteStatsTracker.java | 89 ++++++++++++++++++ .../http/HttpServerTransport.java | 5 ++ .../org/elasticsearch/http/HttpStats.java | 45 ++++++++-- .../rest/ChunkedRestResponseBody.java | 35 +++++++- .../elasticsearch/rest/MethodHandlers.java | 16 ++++ .../elasticsearch/rest/RestController.java | 40 +++++++-- .../cluster/node/stats/NodeStatsTests.java | 2 +- .../elasticsearch/http/HttpStatsTests.java | 37 +++++++- .../info/RestClusterInfoActionTests.java | 3 +- 14 files changed, 389 insertions(+), 18 deletions(-) create mode 100644 qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml create mode 100644 server/src/main/java/org/elasticsearch/http/HttpRouteStats.java create mode 100644 server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java diff --git a/qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml b/qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml new file mode 100644 index 0000000000000..a72e9f0316b10 --- /dev/null +++ b/qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml @@ -0,0 +1,22 @@ +# This test needs multiple nodes, because a single-node cluster does not send any transport actions so these stats are empty +--- +"http stats": + - skip: + features: [arbitrary_key] + + - do: + search: + index: "*" + body: + query: + match_all: {} + + - do: + nodes.stats: + metric: [ http ] + human: true + + - set: + nodes._arbitrary_key_: node_id + + - is_true: "nodes.$node_id.http.routes./_cat/nodes" diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 926cc4801dddc..14b81cd7d1b79 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -145,6 +145,7 @@ static TransportVersion def(int id) { public static final TransportVersion WAIT_FOR_CLUSTER_STATE_IN_RECOVERY_ADDED = def(8_502_00_0); public static final TransportVersion RECOVERY_COMMIT_TOO_NEW_EXCEPTION_ADDED = def(8_503_00_0); public static final TransportVersion NODE_INFO_COMPONENT_VERSIONS_ADDED = def(8_504_00_0); + public static final TransportVersion NODE_STATS_HTTP_ROUTE_STATS_ADDED = def(8_505_00_0); /* * STOP! READ THIS FIRST! No, really, * ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _ diff --git a/server/src/main/java/org/elasticsearch/common/path/PathTrie.java b/server/src/main/java/org/elasticsearch/common/path/PathTrie.java index 2c84e6c6817ea..306304ab016a4 100644 --- a/server/src/main/java/org/elasticsearch/common/path/PathTrie.java +++ b/server/src/main/java/org/elasticsearch/common/path/PathTrie.java @@ -8,6 +8,8 @@ package org.elasticsearch.common.path; +import org.elasticsearch.common.collect.Iterators; + import java.util.EnumSet; import java.util.HashMap; import java.util.Iterator; @@ -267,6 +269,15 @@ private void put(Map params, TrieNode node, String value) { } } + Iterator allNodeValues() { + final Iterator childrenIterator = Iterators.flatMap(children.values().iterator(), TrieNode::allNodeValues); + if (value == null) { + return childrenIterator; + } else { + return Iterators.concat(Iterators.single(value), childrenIterator); + } + } + @Override public String toString() { return key; @@ -366,4 +377,8 @@ public T next() { } }; } + + public Iterator allNodeValues() { + return Iterators.concat(Iterators.single(rootValue), root.allNodeValues()); + } } diff --git a/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java b/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java index 767b7bdfb643f..a6f6eb8750cac 100644 --- a/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java +++ b/server/src/main/java/org/elasticsearch/http/AbstractHttpServerTransport.java @@ -168,7 +168,12 @@ public HttpInfo info() { @Override public HttpStats stats() { - return new HttpStats(httpChannels.size(), totalChannelsAccepted.get(), httpClientStatsTracker.getClientStats()); + return new HttpStats( + httpChannels.size(), + totalChannelsAccepted.get(), + httpClientStatsTracker.getClientStats(), + dispatcher.getStats() + ); } protected void bindServer() { diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java new file mode 100644 index 0000000000000..60736be15ed3a --- /dev/null +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.http; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.xcontent.ToXContentObject; +import org.elasticsearch.xcontent.XContentBuilder; + +import java.io.IOException; + +public record HttpRouteStats( + long requestCount, + long totalRequestSize, + long[] requestSizeHistogram, + long responseCount, + long totalResponseSize, + long[] responseSizeHistogram +) implements Writeable, ToXContentObject { + + public HttpRouteStats(StreamInput in) throws IOException { + this(in.readVLong(), in.readVLong(), in.readVLongArray(), in.readVLong(), in.readVLong(), in.readVLongArray()); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + + builder.startObject("requests"); + builder.field("count", requestCount); + builder.humanReadableField("total_size_in_bytes", "total_size", ByteSizeValue.ofBytes(totalRequestSize)); + histogramToXContent(builder, requestSizeHistogram); + builder.endObject(); + + builder.startObject("responses"); + builder.field("count", responseCount); + builder.humanReadableField("total_size_in_bytes", "total_size", ByteSizeValue.ofBytes(totalResponseSize)); + histogramToXContent(builder, responseSizeHistogram); + builder.endObject(); + + return builder.endObject(); + } + + static void histogramToXContent(XContentBuilder builder, long[] sizeHistogram) throws IOException { + final int[] bucketBounds = HttpRouteStatsTracker.getBucketUpperBounds(); + assert sizeHistogram.length == bucketBounds.length + 1; + builder.startArray("histogram"); + + int firstBucket = 0; + long remainingCount = 0L; + for (int i = 0; i < sizeHistogram.length; i++) { + if (remainingCount == 0) { + firstBucket = i; + } + remainingCount += sizeHistogram[i]; + } + + for (int i = firstBucket; i < sizeHistogram.length && 0 < remainingCount; i++) { + builder.startObject(); + if (i > 0) { + builder.humanReadableField("ge_bytes", "ge", ByteSizeValue.ofBytes(bucketBounds[i - 1])); + } + if (i < bucketBounds.length) { + builder.humanReadableField("lt_bytes", "lt", ByteSizeValue.ofBytes(bucketBounds[i])); + } + builder.field("count", sizeHistogram[i]); + builder.endObject(); + remainingCount -= sizeHistogram[i]; + } + builder.endArray(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVLong(requestCount); + out.writeVLong(totalRequestSize); + out.writeVLongArray(requestSizeHistogram); + out.writeVLong(responseCount); + out.writeVLong(totalResponseSize); + out.writeVLongArray(responseSizeHistogram); + } +} diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java new file mode 100644 index 0000000000000..ceb7ba215ac24 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java @@ -0,0 +1,89 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.http; + +import java.util.concurrent.atomic.AtomicLongArray; +import java.util.concurrent.atomic.LongAdder; + +public class HttpRouteStatsTracker { + + /* + * default http.max_content_length is 100 MB so that the last histogram bucket is > 64MB (2^26) + */ + + public static int[] getBucketUpperBounds() { + var bounds = new int[27]; + for (int i = 0; i < bounds.length; i++) { + bounds[i] = 1 << i; + } + return bounds; + } + + private static final int BUCKET_COUNT = getBucketUpperBounds().length + 1; + + private static final long LAST_BUCKET_LOWER_BOUND = getBucketUpperBounds()[BUCKET_COUNT - 2]; + + private record StatsTracker(LongAdder count, LongAdder totalSize, AtomicLongArray histogram) { + StatsTracker { + assert count.longValue() == 0L; + assert totalSize.longValue() == 0L; + assert histogram.length() == BUCKET_COUNT; + } + + StatsTracker() { + this(new LongAdder(), new LongAdder(), new AtomicLongArray(BUCKET_COUNT)); + } + + void addStats(int contentLength) { + count().increment(); + totalSize().add(contentLength); + histogram().incrementAndGet(bucket(contentLength)); + } + + long[] getHistogram() { + long[] histogramCopy = new long[BUCKET_COUNT]; + for (int i = 0; i < BUCKET_COUNT; i++) { + histogramCopy[i] = histogram().get(i); + } + return histogramCopy; + } + } + + private static int bucket(int contentLength) { + if (contentLength <= 0) { + return 0; + } else if (LAST_BUCKET_LOWER_BOUND <= contentLength) { + return BUCKET_COUNT - 1; + } else { + return Integer.SIZE - Integer.numberOfLeadingZeros(contentLength); + } + } + + private final StatsTracker requestStats = new StatsTracker(); + private final StatsTracker responseStats = new StatsTracker(); + + public void addRequestStats(int contentLength) { + requestStats.addStats(contentLength); + } + + public void addResponseStats(int contentLength) { + responseStats.addStats(contentLength); + } + + public HttpRouteStats getStats() { + return new HttpRouteStats( + requestStats.count().longValue(), + requestStats.totalSize().longValue(), + requestStats.getHistogram(), + responseStats.count().longValue(), + responseStats.totalSize().longValue(), + responseStats.getHistogram() + ); + } +} diff --git a/server/src/main/java/org/elasticsearch/http/HttpServerTransport.java b/server/src/main/java/org/elasticsearch/http/HttpServerTransport.java index b2528cf9f87a0..87e91f7657615 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpServerTransport.java +++ b/server/src/main/java/org/elasticsearch/http/HttpServerTransport.java @@ -15,6 +15,8 @@ import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; +import java.util.Map; + public interface HttpServerTransport extends LifecycleComponent, ReportingService { String HTTP_PROFILE_NAME = ".http"; @@ -52,5 +54,8 @@ interface Dispatcher { */ void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause); + default Map getStats() { + return Map.of(); + } } } diff --git a/server/src/main/java/org/elasticsearch/http/HttpStats.java b/server/src/main/java/org/elasticsearch/http/HttpStats.java index 2a8dcb6f6d8b6..ac3e7d64f9a19 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpStats.java @@ -20,18 +20,29 @@ import java.io.IOException; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.stream.Stream; -public record HttpStats(long serverOpen, long totalOpen, List clientStats) implements Writeable, ChunkedToXContent { +import static org.elasticsearch.TransportVersions.NODE_STATS_HTTP_ROUTE_STATS_ADDED; - public static final HttpStats IDENTITY = new HttpStats(0, 0, List.of()); +public record HttpStats(long serverOpen, long totalOpen, List clientStats, Map httpRouteStats) + implements + Writeable, + ChunkedToXContent { + + public static final HttpStats IDENTITY = new HttpStats(0, 0, List.of(), Map.of()); public HttpStats(long serverOpen, long totalOpened) { - this(serverOpen, totalOpened, List.of()); + this(serverOpen, totalOpened, List.of(), Map.of()); } public HttpStats(StreamInput in) throws IOException { - this(in.readVLong(), in.readVLong(), in.readCollectionAsList(ClientStats::new)); + this( + in.readVLong(), + in.readVLong(), + in.readCollectionAsList(ClientStats::new), + in.getTransportVersion().onOrAfter(NODE_STATS_HTTP_ROUTE_STATS_ADDED) ? in.readMap(HttpRouteStats::new) : Map.of() + ); } @Override @@ -39,6 +50,9 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLong(serverOpen); out.writeVLong(totalOpen); out.writeCollection(clientStats); + if (out.getTransportVersion().onOrAfter(NODE_STATS_HTTP_ROUTE_STATS_ADDED)) { + out.writeMap(httpRouteStats, StreamOutput::writeWriteable); + } } public long getServerOpen() { @@ -57,7 +71,8 @@ public static HttpStats merge(HttpStats first, HttpStats second) { return new HttpStats( first.serverOpen + second.serverOpen, first.totalOpen + second.totalOpen, - Stream.concat(first.clientStats.stream(), second.clientStats.stream()).toList() + Stream.concat(first.clientStats.stream(), second.clientStats.stream()).toList(), + Map.of() // TODO: merge ); } @@ -78,6 +93,7 @@ static final class Fields { static final String CLIENT_REQUEST_SIZE_BYTES = "request_size_bytes"; static final String CLIENT_FORWARDED_FOR = "x_forwarded_for"; static final String CLIENT_OPAQUE_ID = "x_opaque_id"; + static final String ROUTES = "routes"; } @Override @@ -90,7 +106,24 @@ public Iterator toXContentChunked(ToXContent.Params outerP .startArray(Fields.CLIENTS) ), clientStats.iterator(), - Iterators.single((builder, params) -> builder.endArray().endObject()) + Iterators.single((builder, params) -> { + builder.endArray(); + if (httpRouteStats.isEmpty() == false) { + builder.startObject(Fields.ROUTES); + } + return builder; + }), + Iterators.map(httpRouteStats.entrySet().iterator(), entry -> (builder, params) -> { + builder.field(entry.getKey()); + entry.getValue().toXContent(builder, params); + return builder; + }), + Iterators.single((builder, params) -> { + if (httpRouteStats.isEmpty() == false) { + builder.endObject(); + } + return builder.endObject(); + }) ); } diff --git a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java index 78e529eef2d98..bc52d8938d9e6 100644 --- a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java +++ b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java @@ -27,6 +27,7 @@ import java.io.Writer; import java.nio.charset.StandardCharsets; import java.util.Iterator; +import java.util.function.Consumer; /** * The body of a rest response that uses chunked HTTP encoding. Implementations are used to avoid materializing full responses on heap and @@ -55,6 +56,10 @@ public interface ChunkedRestResponseBody { */ String getResponseContentTypeString(); + default void setChunkedSizeListener(Consumer sizeConsumer) { + throw new UnsupportedOperationException("not supported"); + } + /** * Create a chunked response body to be written to a specific {@link RestChannel} from a {@link ChunkedToXContent}. * @@ -68,6 +73,9 @@ static ChunkedRestResponseBody fromXContent(ChunkedToXContent chunkedToXContent, return new ChunkedRestResponseBody() { + private int size = 0; + private Consumer sizeConsumer = null; + private final OutputStream out = new OutputStream() { @Override public void write(int b) throws IOException { @@ -95,7 +103,12 @@ public void write(byte[] b, int off, int len) throws IOException { @Override public boolean isDone() { - return serialization.hasNext() == false; + var result = serialization.hasNext() == false; + if (result && sizeConsumer != null) { + sizeConsumer.accept(size); + sizeConsumer = null; + } + return result; } @Override @@ -118,6 +131,7 @@ public ReleasableBytesReference encodeChunk(int sizeHint, Recycler rec () -> Releasables.closeExpectNoException(chunkStream) ); target = null; + size += result.length(); return result; } finally { if (target != null) { @@ -132,6 +146,11 @@ public ReleasableBytesReference encodeChunk(int sizeHint, Recycler rec public String getResponseContentTypeString() { return builder.getResponseContentTypeString(); } + + @Override + public void setChunkedSizeListener(Consumer sizeConsumer) { + this.sizeConsumer = sizeConsumer; + } }; } @@ -141,6 +160,8 @@ public String getResponseContentTypeString() { */ static ChunkedRestResponseBody fromTextChunks(String contentType, Iterator> chunkIterator) { return new ChunkedRestResponseBody() { + private int size = 0; + private Consumer sizeConsumer = null; private RecyclerBytesStreamOutput currentOutput; private final Writer writer = new OutputStreamWriter(new OutputStream() { @Override @@ -170,7 +191,12 @@ public void close() { @Override public boolean isDone() { - return chunkIterator.hasNext() == false; + var result = chunkIterator.hasNext() == false; + if (result && sizeConsumer != null) { + sizeConsumer.accept(size); + sizeConsumer = null; + } + return result; } @Override @@ -209,6 +235,11 @@ public ReleasableBytesReference encodeChunk(int sizeHint, Recycler rec public String getResponseContentTypeString() { return contentType; } + + @Override + public void setChunkedSizeListener(Consumer sizeConsumer) { + this.sizeConsumer = sizeConsumer; + } }; } } diff --git a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java index 1070b61108e48..5d7b128ee223b 100644 --- a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java +++ b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java @@ -9,6 +9,8 @@ package org.elasticsearch.rest; import org.elasticsearch.core.RestApiVersion; +import org.elasticsearch.http.HttpRouteStats; +import org.elasticsearch.http.HttpRouteStatsTracker; import java.util.HashMap; import java.util.Map; @@ -22,6 +24,8 @@ final class MethodHandlers { private final String path; private final Map> methodHandlers; + private final HttpRouteStatsTracker statsTracker = new HttpRouteStatsTracker(); + MethodHandlers(String path) { this.path = path; @@ -75,4 +79,16 @@ RestHandler getHandler(RestRequest.Method method, RestApiVersion version) { Set getValidMethods() { return methodHandlers.keySet(); } + + public void addRequestStats(int contentLength) { + statsTracker.addRequestStats(contentLength); + } + + public void addResponseStats(int contentLength) { + statsTracker.addResponseStats(contentLength); + } + + public HttpRouteStats getStats() { + return statsTracker.getStats(); + } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index c5dd4acd33aa0..666b768d13a6b 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -26,6 +26,7 @@ import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Streams; import org.elasticsearch.http.HttpHeadersValidationException; +import org.elasticsearch.http.HttpRouteStats; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.rest.RestHandler.Route; @@ -46,9 +47,12 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import java.util.function.UnaryOperator; +import java.util.stream.StreamSupport; import static org.elasticsearch.indices.SystemIndices.EXTERNAL_SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY; import static org.elasticsearch.indices.SystemIndices.SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY; @@ -352,8 +356,20 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th } } - private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler, ThreadContext threadContext) - throws Exception { + @Override + public Map getStats() { + return StreamSupport.stream(Spliterators.spliteratorUnknownSize(handlers.allNodeValues(), Spliterator.ORDERED), false) + .filter(mh -> mh.getStats().requestCount() > 0 || mh.getStats().responseCount() > 0) + .collect(Maps.toUnmodifiableSortedMap(MethodHandlers::getPath, MethodHandlers::getStats)); + } + + private void dispatchRequest( + RestRequest request, + RestChannel channel, + RestHandler handler, + MethodHandlers methodHandlers, + ThreadContext threadContext + ) throws Exception { final int contentLength = request.contentLength(); if (contentLength > 0) { if (isContentTypeDisallowed(request) || handler.mediaTypesValid(request) == false) { @@ -390,7 +406,7 @@ private void dispatchRequest(RestRequest request, RestChannel channel, RestHandl inFlightRequestsBreaker(circuitBreakerService).addWithoutBreaking(contentLength); } // iff we could reserve bytes for the request we need to send the response also over this channel - responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength); + responseChannel = new ResourceHandlingHttpChannel(channel, circuitBreakerService, contentLength, methodHandlers); // TODO: Count requests double in the circuit breaker if they need copying? if (handler.allowsUnsafeBuffers() == false) { request.ensureSafeBuffers(); @@ -410,7 +426,6 @@ private void dispatchRequest(RestRequest request, RestChannel channel, RestHandl } else { threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, Boolean.TRUE.toString()); } - handler.handleRequest(request, responseChannel, client); } catch (Exception e) { responseChannel.sendResponse(new RestResponse(responseChannel, e)); @@ -540,7 +555,7 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel } } else { startTrace(threadContext, channel, handlers.getPath()); - dispatchRequest(request, channel, handler, threadContext); + dispatchRequest(request, channel, handler, handlers, threadContext); return; } } @@ -686,12 +701,20 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { private final RestChannel delegate; private final CircuitBreakerService circuitBreakerService; private final int contentLength; + private final MethodHandlers methodHandlers; private final AtomicBoolean closed = new AtomicBoolean(); - ResourceHandlingHttpChannel(RestChannel delegate, CircuitBreakerService circuitBreakerService, int contentLength) { + ResourceHandlingHttpChannel( + RestChannel delegate, + CircuitBreakerService circuitBreakerService, + int contentLength, + MethodHandlers methodHandlers + ) { this.delegate = delegate; this.circuitBreakerService = circuitBreakerService; this.contentLength = contentLength; + this.methodHandlers = methodHandlers; + this.methodHandlers.addRequestStats(contentLength); } @Override @@ -750,6 +773,11 @@ public void sendResponse(RestResponse response) { boolean success = false; try { close(); + if (response.isChunked() == false) { + methodHandlers.addResponseStats(response.content().length()); + } else { + response.chunkedContent().setChunkedSizeListener(methodHandlers::addResponseStats); + } delegate.sendResponse(response); success = true; } finally { diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index 9de3726ba935d..06a48ff3ed3d9 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -859,7 +859,7 @@ public static NodeStats createNodeStats() { ); clientStats.add(cs); } - httpStats = new HttpStats(randomNonNegativeLong(), randomNonNegativeLong(), clientStats); + httpStats = new HttpStats(randomNonNegativeLong(), randomNonNegativeLong(), clientStats, Map.of()); } AllCircuitBreakerStats allCircuitBreakerStats = null; if (frequently()) { diff --git a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java index f63bcbac01950..8e0704ba3e996 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java @@ -8,11 +8,16 @@ package org.elasticsearch.http; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.test.ESTestCase; +import java.util.List; +import java.util.Map; import java.util.stream.IntStream; import java.util.stream.Stream; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; public class HttpStatsTests extends ESTestCase { @@ -29,11 +34,41 @@ public void testMerge() { assertEquals(merged.getClientStats(), Stream.concat(first.getClientStats().stream(), second.getClientStats().stream()).toList()); } + public void testToXContent() { + final var requestSizeHistogram = new long[28]; + requestSizeHistogram[2] = 9; + requestSizeHistogram[4] = 10; + + final var responseSizeHistogram = new long[28]; + responseSizeHistogram[3] = 13; + responseSizeHistogram[5] = 14; + final HttpRouteStats httpRouteStats = new HttpRouteStats( + 1, + ByteSizeUnit.MB.toBytes(2), + requestSizeHistogram, + 3, + ByteSizeUnit.MB.toBytes(4), + responseSizeHistogram + ); + + assertThat( + Strings.toString(new HttpStats(1, 2, List.of(), Map.of("http/path", httpRouteStats)), false, true), + equalTo( + Strings.format( + """ + {"http":{"current_open":1,"total_opened":2,"clients":[],"routes":{"http/path":%s}}}""", + Strings.toString(httpRouteStats, false, true) + ) + ) + ); + } + public static HttpStats randomHttpStats() { return new HttpStats( randomLongBetween(0, Long.MAX_VALUE), randomLongBetween(0, Long.MAX_VALUE), - IntStream.range(1, randomIntBetween(2, 10)).mapToObj(HttpStatsTests::randomClients).toList() + IntStream.range(1, randomIntBetween(2, 10)).mapToObj(HttpStatsTests::randomClients).toList(), + Map.of() ); } diff --git a/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java index 942f5b78b58c1..49b22094fb542 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java @@ -89,7 +89,8 @@ public void testHttpResponseMapper() { .map(HttpStats::clientStats) .map(Collection::stream) .reduce(Stream.of(), Stream::concat) - .toList() + .toList(), + Map.of() ) ); } From 56d857a050b79e66ddb52dcf07b198b3ad981357 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Mon, 25 Sep 2023 17:24:37 +1000 Subject: [PATCH 02/17] Update docs/changelog/99852.yaml --- docs/changelog/99852.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/99852.yaml diff --git a/docs/changelog/99852.yaml b/docs/changelog/99852.yaml new file mode 100644 index 0000000000000..3a26f17737ae8 --- /dev/null +++ b/docs/changelog/99852.yaml @@ -0,0 +1,5 @@ +pr: 99852 +summary: Record more detailed HTTP stats +area: Network +type: enhancement +issues: [] From 32a9e3aaf11b09d159444425b7af13c7143bb0f2 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 26 Sep 2023 11:16:25 +1000 Subject: [PATCH 03/17] fix test --- .../action/admin/cluster/node/stats/NodeStatsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index 06a48ff3ed3d9..94fb39ac9be34 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -541,7 +541,7 @@ private static int expectedChunks(@Nullable IngestStats ingestStats) { } private static int expectedChunks(@Nullable HttpStats httpStats) { - return httpStats == null ? 0 : 2 + httpStats.getClientStats().size(); + return httpStats == null ? 0 : 3 + httpStats.getClientStats().size() + httpStats.httpRouteStats().size(); } private static int expectedChunks(@Nullable TransportStats transportStats) { From e669041d9ac93c29009e5a8786b923a909a5c131 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 26 Sep 2023 12:57:32 +1000 Subject: [PATCH 04/17] add response time --- .../http/DanglingIndicesRestIT.java | 5 -- .../elasticsearch/http/HttpSmokeTestCase.java | 8 ++ .../org/elasticsearch/http/HttpStatsIT.java | 86 +++++++++++++++++++ .../smoke_test_multinode/60_http_stats.yml | 22 ----- .../elasticsearch/http/HttpRouteStats.java | 40 +++++---- .../http/HttpRouteStatsTracker.java | 10 ++- .../rest/ChunkedRestResponseBody.java | 1 + .../elasticsearch/rest/MethodHandlers.java | 4 + .../elasticsearch/rest/RestController.java | 5 +- .../elasticsearch/http/HttpStatsTests.java | 8 +- 10 files changed, 144 insertions(+), 45 deletions(-) create mode 100644 qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java delete mode 100644 qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/DanglingIndicesRestIT.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/DanglingIndicesRestIT.java index 6880c558cc4d6..70df4aaeaf5de 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/DanglingIndicesRestIT.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/DanglingIndicesRestIT.java @@ -30,7 +30,6 @@ import static org.elasticsearch.cluster.metadata.IndexGraveyard.SETTING_MAX_TOMBSTONES; import static org.elasticsearch.indices.IndicesService.WRITE_DANGLING_INDICES_INFO_SETTING; import static org.elasticsearch.rest.RestStatus.ACCEPTED; -import static org.elasticsearch.rest.RestStatus.OK; import static org.elasticsearch.test.XContentTestUtils.createJsonMapView; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; @@ -184,10 +183,6 @@ private List listDanglingIndexIds() throws IOException { return danglingIndexIds; } - private void assertOK(Response response) { - assertThat(response.getStatusLine().getStatusCode(), equalTo(OK.getStatus())); - } - /** * Given a node name, finds the corresponding node ID. */ diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java index d62a490d092df..804aa5a2bda3a 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.http; +import org.elasticsearch.client.Response; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; @@ -17,6 +18,9 @@ import java.util.Collection; import java.util.List; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.equalTo; + public abstract class HttpSmokeTestCase extends ESIntegTestCase { @Override @@ -42,4 +46,8 @@ protected Collection> nodePlugins() { protected boolean ignoreExternalCluster() { return true; } + + public static void assertOK(Response response) { + assertThat(response.getStatusLine().getStatusCode(), anyOf(equalTo(200), equalTo(201))); + } } diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java new file mode 100644 index 0000000000000..d082706858258 --- /dev/null +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java @@ -0,0 +1,86 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.http; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.XContentTestUtils; +import org.elasticsearch.xcontent.json.JsonXContent; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.notNullValue; + +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 1, numClientNodes = 0) +public class HttpStatsIT extends HttpSmokeTestCase { + + @SuppressWarnings("unchecked") + public void testHttpStats() throws IOException { + // basic request + final RestClient restClient = getRestClient(); + assertOK(restClient.performRequest(new Request("GET", "/"))); + // request with body and URL placeholder + final Request searchRequest = new Request("GET", "*/_search"); + searchRequest.setJsonEntity(""" + {"query":{"match_all":{}}}"""); + assertOK(restClient.performRequest(searchRequest)); + // chunked response + assertOK(restClient.performRequest(new Request("GET", "/_cluster/state"))); + // chunked text response + assertOK(restClient.performRequest(new Request("GET", "/_cat/nodes"))); + + final Response response = restClient.performRequest(new Request("GET", "/_nodes/stats/http")); + assertOK(response); + + final Map responseMap = XContentHelper.convertToMap( + JsonXContent.jsonXContent, + response.getEntity().getContent(), + false + ); + final Map nodesMap = (Map) responseMap.get("nodes"); + + assertThat(nodesMap, aMapWithSize(1)); + final String nodeId = nodesMap.keySet().iterator().next(); + final XContentTestUtils.JsonMapView nodeView = new XContentTestUtils.JsonMapView((Map) nodesMap.get(nodeId)); + + final List routes = List.of("/", "/_cat/nodes", "/{index}/_search", "/_cluster/state"); + + for (var route : routes) { + assertThat(nodeView.get("http.routes." + route), notNullValue()); + assertThat(nodeView.get("http.routes." + route + ".requests.count"), equalTo(1)); + assertThat(nodeView.get("http.routes." + route + ".requests.total_size_in_bytes"), greaterThanOrEqualTo(0)); + assertThat(nodeView.get("http.routes." + route + ".responses.count"), equalTo(1)); + assertThat(nodeView.get("http.routes." + route + ".responses.total_size_in_bytes"), greaterThan(1)); + assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram"), hasSize(1)); + assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram.0.count"), equalTo(1)); + assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram.0.lt_bytes"), notNullValue()); + if (route.equals("/{index}/_search")) { + assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram.0.ge_bytes"), notNullValue()); + } + assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram"), hasSize(1)); + assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram.0.count"), equalTo(1)); + assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram.0.lt_bytes"), notNullValue()); + assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram.0.ge_bytes"), notNullValue()); + assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram"), hasSize(1)); + assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram.0.count"), equalTo(1)); + assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram.0.lt_millis"), notNullValue()); + assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram.0.ge_millis"), notNullValue()); + } + } +} diff --git a/qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml b/qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml deleted file mode 100644 index a72e9f0316b10..0000000000000 --- a/qa/smoke-test-multinode/src/yamlRestTest/resources/rest-api-spec/test/smoke_test_multinode/60_http_stats.yml +++ /dev/null @@ -1,22 +0,0 @@ -# This test needs multiple nodes, because a single-node cluster does not send any transport actions so these stats are empty ---- -"http stats": - - skip: - features: [arbitrary_key] - - - do: - search: - index: "*" - body: - query: - match_all: {} - - - do: - nodes.stats: - metric: [ http ] - human: true - - - set: - nodes._arbitrary_key_: node_id - - - is_true: "nodes.$node_id.http.routes./_cat/nodes" diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java index 60736be15ed3a..45e1964abe3cc 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.network.HandlingTimeTracker; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; @@ -23,11 +24,12 @@ public record HttpRouteStats( long[] requestSizeHistogram, long responseCount, long totalResponseSize, - long[] responseSizeHistogram + long[] responseSizeHistogram, + long[] responseTimeHistogram ) implements Writeable, ToXContentObject { public HttpRouteStats(StreamInput in) throws IOException { - this(in.readVLong(), in.readVLong(), in.readVLongArray(), in.readVLong(), in.readVLong(), in.readVLongArray()); + this(in.readVLong(), in.readVLong(), in.readVLongArray(), in.readVLong(), in.readVLong(), in.readVLongArray(), in.readVLongArray()); } @Override @@ -37,43 +39,50 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject("requests"); builder.field("count", requestCount); builder.humanReadableField("total_size_in_bytes", "total_size", ByteSizeValue.ofBytes(totalRequestSize)); - histogramToXContent(builder, requestSizeHistogram); + histogramToXContent(builder, "size_histogram", "bytes", requestSizeHistogram, HttpRouteStatsTracker.getBucketUpperBounds()); builder.endObject(); builder.startObject("responses"); builder.field("count", responseCount); builder.humanReadableField("total_size_in_bytes", "total_size", ByteSizeValue.ofBytes(totalResponseSize)); - histogramToXContent(builder, responseSizeHistogram); + histogramToXContent(builder, "size_histogram", "bytes", responseSizeHistogram, HttpRouteStatsTracker.getBucketUpperBounds()); + histogramToXContent( + builder, + "handling_time_histogram", + "millis", + responseTimeHistogram, + HandlingTimeTracker.getBucketUpperBounds() + ); builder.endObject(); return builder.endObject(); } - static void histogramToXContent(XContentBuilder builder, long[] sizeHistogram) throws IOException { - final int[] bucketBounds = HttpRouteStatsTracker.getBucketUpperBounds(); - assert sizeHistogram.length == bucketBounds.length + 1; - builder.startArray("histogram"); + static void histogramToXContent(XContentBuilder builder, String fieldName, String unitName, long[] histogram, int[] bucketBounds) + throws IOException { + assert histogram.length == bucketBounds.length + 1; + builder.startArray(fieldName); int firstBucket = 0; long remainingCount = 0L; - for (int i = 0; i < sizeHistogram.length; i++) { + for (int i = 0; i < histogram.length; i++) { if (remainingCount == 0) { firstBucket = i; } - remainingCount += sizeHistogram[i]; + remainingCount += histogram[i]; } - for (int i = firstBucket; i < sizeHistogram.length && 0 < remainingCount; i++) { + for (int i = firstBucket; i < histogram.length && 0 < remainingCount; i++) { builder.startObject(); if (i > 0) { - builder.humanReadableField("ge_bytes", "ge", ByteSizeValue.ofBytes(bucketBounds[i - 1])); + builder.humanReadableField("ge_" + unitName, "ge", ByteSizeValue.ofBytes(bucketBounds[i - 1])); } if (i < bucketBounds.length) { - builder.humanReadableField("lt_bytes", "lt", ByteSizeValue.ofBytes(bucketBounds[i])); + builder.humanReadableField("lt_" + unitName, "lt", ByteSizeValue.ofBytes(bucketBounds[i])); } - builder.field("count", sizeHistogram[i]); + builder.field("count", histogram[i]); builder.endObject(); - remainingCount -= sizeHistogram[i]; + remainingCount -= histogram[i]; } builder.endArray(); } @@ -86,5 +95,6 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLong(responseCount); out.writeVLong(totalResponseSize); out.writeVLongArray(responseSizeHistogram); + out.writeVLongArray(responseTimeHistogram); } } diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java index ceb7ba215ac24..b6cda33bfc1f6 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java @@ -8,6 +8,8 @@ package org.elasticsearch.http; +import org.elasticsearch.common.network.HandlingTimeTracker; + import java.util.concurrent.atomic.AtomicLongArray; import java.util.concurrent.atomic.LongAdder; @@ -67,6 +69,7 @@ private static int bucket(int contentLength) { private final StatsTracker requestStats = new StatsTracker(); private final StatsTracker responseStats = new StatsTracker(); + private final HandlingTimeTracker responseTimeTracker = new HandlingTimeTracker(); public void addRequestStats(int contentLength) { requestStats.addStats(contentLength); @@ -76,6 +79,10 @@ public void addResponseStats(int contentLength) { responseStats.addStats(contentLength); } + public void addResponseTime(long timeMillis) { + responseTimeTracker.addHandlingTime(timeMillis); + } + public HttpRouteStats getStats() { return new HttpRouteStats( requestStats.count().longValue(), @@ -83,7 +90,8 @@ public HttpRouteStats getStats() { requestStats.getHistogram(), responseStats.count().longValue(), responseStats.totalSize().longValue(), - responseStats.getHistogram() + responseStats.getHistogram(), + responseTimeTracker.getHistogram() ); } } diff --git a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java index 9fa9e5db80562..45b05757c7bcd 100644 --- a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java +++ b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java @@ -236,6 +236,7 @@ public ReleasableBytesReference encodeChunk(int sizeHint, Recycler rec () -> Releasables.closeExpectNoException(chunkOutput) ); currentOutput = null; + size += result.length(); return result; } finally { if (currentOutput != null) { diff --git a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java index 5d7b128ee223b..6c62c4cdcd1dc 100644 --- a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java +++ b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java @@ -88,6 +88,10 @@ public void addResponseStats(int contentLength) { statsTracker.addResponseStats(contentLength); } + public void addResponseTime(long timeMillis) { + statsTracker.addResponseTime(timeMillis); + } + public HttpRouteStats getStats() { return statsTracker.getStats(); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 666b768d13a6b..338c6e346169f 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -702,6 +702,7 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { private final CircuitBreakerService circuitBreakerService; private final int contentLength; private final MethodHandlers methodHandlers; + private final long startTime; private final AtomicBoolean closed = new AtomicBoolean(); ResourceHandlingHttpChannel( @@ -714,7 +715,7 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { this.circuitBreakerService = circuitBreakerService; this.contentLength = contentLength; this.methodHandlers = methodHandlers; - this.methodHandlers.addRequestStats(contentLength); + this.startTime = System.currentTimeMillis(); } @Override @@ -773,6 +774,7 @@ public void sendResponse(RestResponse response) { boolean success = false; try { close(); + methodHandlers.addRequestStats(contentLength); if (response.isChunked() == false) { methodHandlers.addResponseStats(response.content().length()); } else { @@ -781,6 +783,7 @@ public void sendResponse(RestResponse response) { delegate.sendResponse(response); success = true; } finally { + methodHandlers.addResponseTime(System.currentTimeMillis() - startTime); if (success == false) { releaseOutputBuffer(); } diff --git a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java index 8e0704ba3e996..1b49dd084f3f6 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java @@ -42,13 +42,19 @@ public void testToXContent() { final var responseSizeHistogram = new long[28]; responseSizeHistogram[3] = 13; responseSizeHistogram[5] = 14; + + final var responseTimeHistogram = new long[18]; + responseTimeHistogram[4] = 17; + responseTimeHistogram[6] = 18; + final HttpRouteStats httpRouteStats = new HttpRouteStats( 1, ByteSizeUnit.MB.toBytes(2), requestSizeHistogram, 3, ByteSizeUnit.MB.toBytes(4), - responseSizeHistogram + responseSizeHistogram, + responseTimeHistogram ); assertThat( From 2873701711f42b7daaeea9df3cf409ca9c2e784f Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 26 Sep 2023 14:41:36 +1000 Subject: [PATCH 05/17] add merge and info stats support --- .../org/elasticsearch/http/HttpStatsIT.java | 90 ++++++++++++------- .../elasticsearch/http/HttpRouteStats.java | 23 +++++ .../org/elasticsearch/http/HttpStats.java | 4 +- 3 files changed, 83 insertions(+), 34 deletions(-) diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java index d082706858258..5be12958bd17e 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java @@ -27,25 +27,15 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; -@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 1, numClientNodes = 0) +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 0, numClientNodes = 0) public class HttpStatsIT extends HttpSmokeTestCase { @SuppressWarnings("unchecked") - public void testHttpStats() throws IOException { - // basic request - final RestClient restClient = getRestClient(); - assertOK(restClient.performRequest(new Request("GET", "/"))); - // request with body and URL placeholder - final Request searchRequest = new Request("GET", "*/_search"); - searchRequest.setJsonEntity(""" - {"query":{"match_all":{}}}"""); - assertOK(restClient.performRequest(searchRequest)); - // chunked response - assertOK(restClient.performRequest(new Request("GET", "/_cluster/state"))); - // chunked text response - assertOK(restClient.performRequest(new Request("GET", "/_cat/nodes"))); + public void testNodeHttpStats() throws IOException { + internalCluster().startNode(); + performHttpRequests(); - final Response response = restClient.performRequest(new Request("GET", "/_nodes/stats/http")); + final Response response = getRestClient().performRequest(new Request("GET", "/_nodes/stats/http")); assertOK(response); final Map responseMap = XContentHelper.convertToMap( @@ -57,30 +47,64 @@ public void testHttpStats() throws IOException { assertThat(nodesMap, aMapWithSize(1)); final String nodeId = nodesMap.keySet().iterator().next(); - final XContentTestUtils.JsonMapView nodeView = new XContentTestUtils.JsonMapView((Map) nodesMap.get(nodeId)); + assertHttpStats(new XContentTestUtils.JsonMapView((Map) nodesMap.get(nodeId))); + } + + @SuppressWarnings("unchecked") + public void testClusterInfoHttpStats() throws IOException { + internalCluster().ensureAtLeastNumDataNodes(3); + performHttpRequests(); + + final Response response = getRestClient().performRequest(new Request("GET", "/_info/http")); + assertOK(response); + + final Map responseMap = XContentHelper.convertToMap( + JsonXContent.jsonXContent, + response.getEntity().getContent(), + false + ); + assertHttpStats(new XContentTestUtils.JsonMapView(responseMap)); + } + + private void performHttpRequests() throws IOException { + // basic request + final RestClient restClient = getRestClient(); + assertOK(restClient.performRequest(new Request("GET", "/"))); + // request with body and URL placeholder + final Request searchRequest = new Request("GET", "*/_search"); + searchRequest.setJsonEntity(""" + {"query":{"match_all":{}}}"""); + assertOK(restClient.performRequest(searchRequest)); + // chunked response + assertOK(restClient.performRequest(new Request("GET", "/_cluster/state"))); + // chunked text response + assertOK(restClient.performRequest(new Request("GET", "/_cat/nodes"))); + } + + private void assertHttpStats(XContentTestUtils.JsonMapView jsonMapView) { final List routes = List.of("/", "/_cat/nodes", "/{index}/_search", "/_cluster/state"); for (var route : routes) { - assertThat(nodeView.get("http.routes." + route), notNullValue()); - assertThat(nodeView.get("http.routes." + route + ".requests.count"), equalTo(1)); - assertThat(nodeView.get("http.routes." + route + ".requests.total_size_in_bytes"), greaterThanOrEqualTo(0)); - assertThat(nodeView.get("http.routes." + route + ".responses.count"), equalTo(1)); - assertThat(nodeView.get("http.routes." + route + ".responses.total_size_in_bytes"), greaterThan(1)); - assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram"), hasSize(1)); - assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram.0.count"), equalTo(1)); - assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram.0.lt_bytes"), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route + ".requests.count"), equalTo(1)); + assertThat(jsonMapView.get("http.routes." + route + ".requests.total_size_in_bytes"), greaterThanOrEqualTo(0)); + assertThat(jsonMapView.get("http.routes." + route + ".responses.count"), equalTo(1)); + assertThat(jsonMapView.get("http.routes." + route + ".responses.total_size_in_bytes"), greaterThan(1)); + assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram"), hasSize(1)); + assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.count"), equalTo(1)); + assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.lt_bytes"), notNullValue()); if (route.equals("/{index}/_search")) { - assertThat(nodeView.get("http.routes." + route + ".requests.size_histogram.0.ge_bytes"), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.ge_bytes"), notNullValue()); } - assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram"), hasSize(1)); - assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram.0.count"), equalTo(1)); - assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram.0.lt_bytes"), notNullValue()); - assertThat(nodeView.get("http.routes." + route + ".responses.size_histogram.0.ge_bytes"), notNullValue()); - assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram"), hasSize(1)); - assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram.0.count"), equalTo(1)); - assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram.0.lt_millis"), notNullValue()); - assertThat(nodeView.get("http.routes." + route + ".responses.handling_time_histogram.0.ge_millis"), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram"), hasSize(1)); + assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.count"), equalTo(1)); + assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.lt_bytes"), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.ge_bytes"), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram"), hasSize(1)); + assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.count"), equalTo(1)); + assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.lt_millis"), notNullValue()); + assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.ge_millis"), notNullValue()); } } } diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java index 45e1964abe3cc..1a69414bc6745 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java @@ -17,6 +17,7 @@ import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.util.stream.IntStream; public record HttpRouteStats( long requestCount, @@ -87,6 +88,28 @@ static void histogramToXContent(XContentBuilder builder, String fieldName, Strin builder.endArray(); } + public static HttpRouteStats merge(HttpRouteStats first, HttpRouteStats second) { + assert first.requestSizeHistogram.length == second.requestSizeHistogram.length + && first.responseSizeHistogram.length == second.responseSizeHistogram.length + && first.responseTimeHistogram.length == second.responseTimeHistogram.length; + + return new HttpRouteStats( + first.requestCount + second.requestCount, + first.totalRequestSize + second.totalRequestSize, + IntStream.range(0, first.requestSizeHistogram.length) + .mapToLong(i -> first.requestSizeHistogram[i] + second.requestSizeHistogram[i]) + .toArray(), + first.responseCount + second.responseCount, + first.totalResponseSize + second.totalResponseSize, + IntStream.range(0, first.responseSizeHistogram.length) + .mapToLong(i -> first.responseSizeHistogram[i] + second.responseSizeHistogram[i]) + .toArray(), + IntStream.range(0, first.responseTimeHistogram.length) + .mapToLong(i -> first.responseTimeHistogram[i] + second.responseTimeHistogram[i]) + .toArray() + ); + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeVLong(requestCount); diff --git a/server/src/main/java/org/elasticsearch/http/HttpStats.java b/server/src/main/java/org/elasticsearch/http/HttpStats.java index ac3e7d64f9a19..45ab382208df4 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpStats.java @@ -21,6 +21,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import java.util.stream.Stream; import static org.elasticsearch.TransportVersions.NODE_STATS_HTTP_ROUTE_STATS_ADDED; @@ -72,7 +73,8 @@ public static HttpStats merge(HttpStats first, HttpStats second) { first.serverOpen + second.serverOpen, first.totalOpen + second.totalOpen, Stream.concat(first.clientStats.stream(), second.clientStats.stream()).toList(), - Map.of() // TODO: merge + Stream.concat(first.httpRouteStats.entrySet().stream(), second.httpRouteStats.entrySet().stream()) + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue, HttpRouteStats::merge)) ); } From 4504e52574d0c97325aa0c0518fae58d358b1963 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 26 Sep 2023 16:58:29 +1000 Subject: [PATCH 06/17] tests tweak --- .../elasticsearch/http/HttpRouteStats.java | 25 +++++++++++++++++++ .../elasticsearch/rest/RestController.java | 2 +- .../cluster/node/stats/NodeStatsTests.java | 8 +++++- .../elasticsearch/http/HttpStatsTests.java | 19 +++++++++++++- .../info/RestClusterInfoActionTests.java | 8 +++++- 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java index 1a69414bc6745..f91547c55816b 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java @@ -17,6 +17,8 @@ import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Arrays; +import java.util.Objects; import java.util.stream.IntStream; public record HttpRouteStats( @@ -120,4 +122,27 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLongArray(responseSizeHistogram); out.writeVLongArray(responseTimeHistogram); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + HttpRouteStats that = (HttpRouteStats) o; + return requestCount == that.requestCount + && totalRequestSize == that.totalRequestSize + && responseCount == that.responseCount + && totalResponseSize == that.totalResponseSize + && Arrays.equals(requestSizeHistogram, that.requestSizeHistogram) + && Arrays.equals(responseSizeHistogram, that.responseSizeHistogram) + && Arrays.equals(responseTimeHistogram, that.responseTimeHistogram); + } + + @Override + public int hashCode() { + int result = Objects.hash(requestCount, totalRequestSize, responseCount, totalResponseSize); + result = 31 * result + Arrays.hashCode(requestSizeHistogram); + result = 31 * result + Arrays.hashCode(responseSizeHistogram); + result = 31 * result + Arrays.hashCode(responseTimeHistogram); + return result; + } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 338c6e346169f..e44cd3ea116ab 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -775,6 +775,7 @@ public void sendResponse(RestResponse response) { try { close(); methodHandlers.addRequestStats(contentLength); + methodHandlers.addResponseTime(System.currentTimeMillis() - startTime); if (response.isChunked() == false) { methodHandlers.addResponseStats(response.content().length()); } else { @@ -783,7 +784,6 @@ public void sendResponse(RestResponse response) { delegate.sendResponse(response); success = true; } finally { - methodHandlers.addResponseTime(System.currentTimeMillis() - startTime); if (success == false) { releaseOutputBuffer(); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index 94fb39ac9be34..71b1b83238d47 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.core.Tuple; import org.elasticsearch.discovery.DiscoveryStats; import org.elasticsearch.http.HttpStats; +import org.elasticsearch.http.HttpStatsTests; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.bulk.stats.BulkStats; @@ -859,7 +860,12 @@ public static NodeStats createNodeStats() { ); clientStats.add(cs); } - httpStats = new HttpStats(randomNonNegativeLong(), randomNonNegativeLong(), clientStats, Map.of()); + httpStats = new HttpStats( + randomNonNegativeLong(), + randomNonNegativeLong(), + clientStats, + randomMap(1, 3, () -> new Tuple<>(randomAlphaOfLength(10), HttpStatsTests.randomHttpRouteStats())) + ); } AllCircuitBreakerStats allCircuitBreakerStats = null; if (frequently()) { diff --git a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java index 1b49dd084f3f6..139017bcb9b99 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java @@ -10,8 +10,10 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.unit.ByteSizeUnit; +import org.elasticsearch.core.Tuple; import org.elasticsearch.test.ESTestCase; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.IntStream; @@ -32,6 +34,9 @@ public void testMerge() { assertEquals(merged.getTotalOpen(), first.getTotalOpen() + second.getTotalOpen()); assertThat(merged.getClientStats(), hasSize(first.getClientStats().size() + second.getClientStats().size())); assertEquals(merged.getClientStats(), Stream.concat(first.getClientStats().stream(), second.getClientStats().stream()).toList()); + final Map m = new HashMap<>(first.httpRouteStats()); + second.httpRouteStats().forEach((k, v) -> m.merge(k, v, HttpRouteStats::merge)); + assertEquals(merged.httpRouteStats(), m); } public void testToXContent() { @@ -74,7 +79,7 @@ public static HttpStats randomHttpStats() { randomLongBetween(0, Long.MAX_VALUE), randomLongBetween(0, Long.MAX_VALUE), IntStream.range(1, randomIntBetween(2, 10)).mapToObj(HttpStatsTests::randomClients).toList(), - Map.of() + randomMap(1, 3, () -> new Tuple<>(randomAlphaOfLength(10), randomHttpRouteStats())) ); } @@ -94,4 +99,16 @@ public static HttpStats.ClientStats randomClients(int i) { randomLong() ); } + + public static HttpRouteStats randomHttpRouteStats() { + return new HttpRouteStats( + randomLongBetween(0, 99), + randomLongBetween(0, 9999), + IntStream.range(0, 28).mapToLong(i -> randomLongBetween(0, 42)).toArray(), + randomLongBetween(0, 99), + randomLongBetween(0, 9999), + IntStream.range(0, 28).mapToLong(i -> randomLongBetween(0, 42)).toArray(), + IntStream.range(0, 18).mapToLong(i -> randomLongBetween(0, 42)).toArray() + ); + } } diff --git a/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java index 49b22094fb542..ccc7f1c031e0e 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.http.HttpRouteStats; import org.elasticsearch.http.HttpStats; import org.elasticsearch.http.HttpStatsTests; import org.elasticsearch.test.ESTestCase; @@ -21,6 +22,7 @@ import java.io.IOException; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.IntStream; @@ -90,7 +92,11 @@ public void testHttpResponseMapper() { .map(Collection::stream) .reduce(Stream.of(), Stream::concat) .toList(), - Map.of() + nodeStats.stream().map(NodeStats::getHttp).map(HttpStats::httpRouteStats).reduce(Map.of(), (l, r) -> { + final var m = new HashMap<>(l); + r.forEach((k, v) -> m.merge(k, v, HttpRouteStats::merge)); + return Map.copyOf(m); + }) ) ); } From 5c1fdb781c4513377b779c7c3f6ef0d7586e7b42 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 27 Sep 2023 12:50:38 +1000 Subject: [PATCH 07/17] wrap chunked body and report size on close --- .../org/elasticsearch/http/HttpStatsIT.java | 40 ++++++++++------- .../elasticsearch/http/HttpRouteStats.java | 15 +++++++ .../rest/ChunkedRestResponseBody.java | 34 +------------- .../elasticsearch/rest/RestController.java | 44 ++++++++++++++++++- 4 files changed, 83 insertions(+), 50 deletions(-) diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java index 5be12958bd17e..c582191c085f4 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpStatsIT.java @@ -26,6 +26,7 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 0, numClientNodes = 0) public class HttpStatsIT extends HttpSmokeTestCase { @@ -86,25 +87,30 @@ private void assertHttpStats(XContentTestUtils.JsonMapView jsonMapView) { final List routes = List.of("/", "/_cat/nodes", "/{index}/_search", "/_cluster/state"); for (var route : routes) { - assertThat(jsonMapView.get("http.routes." + route), notNullValue()); - assertThat(jsonMapView.get("http.routes." + route + ".requests.count"), equalTo(1)); - assertThat(jsonMapView.get("http.routes." + route + ".requests.total_size_in_bytes"), greaterThanOrEqualTo(0)); - assertThat(jsonMapView.get("http.routes." + route + ".responses.count"), equalTo(1)); - assertThat(jsonMapView.get("http.routes." + route + ".responses.total_size_in_bytes"), greaterThan(1)); - assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram"), hasSize(1)); - assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.count"), equalTo(1)); - assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.lt_bytes"), notNullValue()); + assertThat(route, jsonMapView.get("http.routes." + route), notNullValue()); + assertThat(route, jsonMapView.get("http.routes." + route + ".requests.count"), equalTo(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".requests.total_size_in_bytes"), greaterThanOrEqualTo(0)); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.count"), equalTo(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.total_size_in_bytes"), greaterThan(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".requests.size_histogram"), hasSize(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.count"), equalTo(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.lt_bytes"), notNullValue()); if (route.equals("/{index}/_search")) { - assertThat(jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.ge_bytes"), notNullValue()); + assertThat(route, jsonMapView.get("http.routes." + route + ".requests.size_histogram.0.ge_bytes"), notNullValue()); } - assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram"), hasSize(1)); - assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.count"), equalTo(1)); - assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.lt_bytes"), notNullValue()); - assertThat(jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.ge_bytes"), notNullValue()); - assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram"), hasSize(1)); - assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.count"), equalTo(1)); - assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.lt_millis"), notNullValue()); - assertThat(jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.ge_millis"), notNullValue()); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.size_histogram"), hasSize(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.count"), equalTo(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.lt_bytes"), notNullValue()); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.size_histogram.0.ge_bytes"), notNullValue()); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram"), hasSize(1)); + assertThat(route, jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.count"), equalTo(1)); + final int ltMillis = jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.lt_millis"); + assertThat(route, ltMillis, notNullValue()); + assertThat( + route, + jsonMapView.get("http.routes." + route + ".responses.handling_time_histogram.0.ge_millis"), + ltMillis > 1 ? notNullValue() : nullValue() + ); } } } diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java index f91547c55816b..41a134923e136 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java @@ -21,6 +21,21 @@ import java.util.Objects; import java.util.stream.IntStream; +/** + * This class encapsulates the stats for a single HTTP route {@link org.elasticsearch.rest.MethodHandlers} + * + * @param requestCount the number of request handled by the HTTP route + * @param totalRequestSize the total body size (bytes) of requests handled by the HTTP route + * @param requestSizeHistogram an array of frequencies of request size (bytes) in buckets with upper bounds + * as returned by {@link HttpRouteStatsTracker#getBucketUpperBounds()}, plus + * an extra bucket for handling size larger than the largest upper bound (currently 64MB). + * @param responseCount the number of responses produced by the HTTP route + * @param totalResponseSize the total body size (bytes) of responses produced by the HTTP route + * @param responseSizeHistogram similar to {@code requestSizeHistogram} but for response size + * @param responseTimeHistogram an array of frequencies of response time (millis) in buckets with upper bounds + * as returned by {@link HandlingTimeTracker#getBucketUpperBounds()}, plus + * an extra bucket for handling response time larger than the longest upper bound (currently 65536ms). + */ public record HttpRouteStats( long requestCount, long totalRequestSize, diff --git a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java index 45b05757c7bcd..9cfe7b84577db 100644 --- a/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java +++ b/server/src/main/java/org/elasticsearch/rest/ChunkedRestResponseBody.java @@ -29,7 +29,6 @@ import java.io.Writer; import java.nio.charset.StandardCharsets; import java.util.Iterator; -import java.util.function.Consumer; /** * The body of a rest response that uses chunked HTTP encoding. Implementations are used to avoid materializing full responses on heap and @@ -58,10 +57,6 @@ public interface ChunkedRestResponseBody extends Releasable { */ String getResponseContentTypeString(); - default void setChunkedSizeListener(Consumer sizeConsumer) { - throw new UnsupportedOperationException("not supported"); - } - /** * Create a chunked response body to be written to a specific {@link RestChannel} from a {@link ChunkedToXContent}. * @@ -80,9 +75,6 @@ static ChunkedRestResponseBody fromXContent( return new ChunkedRestResponseBody() { - private int size = 0; - private Consumer sizeConsumer = null; - private final OutputStream out = new OutputStream() { @Override public void write(int b) throws IOException { @@ -110,12 +102,7 @@ public void write(byte[] b, int off, int len) throws IOException { @Override public boolean isDone() { - var result = serialization.hasNext() == false; - if (result && sizeConsumer != null) { - sizeConsumer.accept(size); - sizeConsumer = null; - } - return result; + return serialization.hasNext() == false; } @Override @@ -138,7 +125,6 @@ public ReleasableBytesReference encodeChunk(int sizeHint, Recycler rec () -> Releasables.closeExpectNoException(chunkStream) ); target = null; - size += result.length(); return result; } finally { if (target != null) { @@ -155,10 +141,6 @@ public String getResponseContentTypeString() { } @Override - public void setChunkedSizeListener(Consumer sizeConsumer) { - this.sizeConsumer = sizeConsumer; - } - public void close() { Releasables.closeExpectNoException(releasable); } @@ -175,8 +157,6 @@ static ChunkedRestResponseBody fromTextChunks( @Nullable Releasable releasable ) { return new ChunkedRestResponseBody() { - private int size = 0; - private Consumer sizeConsumer = null; private RecyclerBytesStreamOutput currentOutput; private final Writer writer = new OutputStreamWriter(new OutputStream() { @Override @@ -206,12 +186,7 @@ public void close() { @Override public boolean isDone() { - var result = chunkIterator.hasNext() == false; - if (result && sizeConsumer != null) { - sizeConsumer.accept(size); - sizeConsumer = null; - } - return result; + return chunkIterator.hasNext() == false; } @Override @@ -236,7 +211,6 @@ public ReleasableBytesReference encodeChunk(int sizeHint, Recycler rec () -> Releasables.closeExpectNoException(chunkOutput) ); currentOutput = null; - size += result.length(); return result; } finally { if (currentOutput != null) { @@ -253,10 +227,6 @@ public String getResponseContentTypeString() { } @Override - public void setChunkedSizeListener(Consumer sizeConsumer) { - this.sizeConsumer = sizeConsumer; - } - public void close() { Releasables.closeExpectNoException(releasable); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index e44cd3ea116ab..a376b15f2f243 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -11,15 +11,18 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.bytes.ReleasableBytesReference; import org.elasticsearch.common.io.stream.BytesStream; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; +import org.elasticsearch.common.recycler.Recycler; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Nullable; @@ -50,6 +53,7 @@ import java.util.Spliterator; import java.util.Spliterators; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.function.UnaryOperator; import java.util.stream.StreamSupport; @@ -779,7 +783,10 @@ public void sendResponse(RestResponse response) { if (response.isChunked() == false) { methodHandlers.addResponseStats(response.content().length()); } else { - response.chunkedContent().setChunkedSizeListener(methodHandlers::addResponseStats); + response = RestResponse.chunked( + response.status(), + new EncodeLengthTrackingChunkedRestResponseBody(response.chunkedContent(), methodHandlers::addResponseStats) + ); } delegate.sendResponse(response); success = true; @@ -799,6 +806,41 @@ private void close() { } } + private static class EncodeLengthTrackingChunkedRestResponseBody implements ChunkedRestResponseBody { + + private final ChunkedRestResponseBody delegate; + private final Consumer encodedLengthConsumer; + private int encodedLength = 0; + + private EncodeLengthTrackingChunkedRestResponseBody(ChunkedRestResponseBody delegate, Consumer encodedLengthConsumer) { + this.delegate = delegate; + this.encodedLengthConsumer = encodedLengthConsumer; + } + + @Override + public boolean isDone() { + return delegate.isDone(); + } + + @Override + public ReleasableBytesReference encodeChunk(int sizeHint, Recycler recycler) throws IOException { + final ReleasableBytesReference bytesReference = delegate.encodeChunk(sizeHint, recycler); + encodedLength += bytesReference.length(); + return bytesReference; + } + + @Override + public String getResponseContentTypeString() { + return delegate.getResponseContentTypeString(); + } + + @Override + public void close() { + delegate.close(); + encodedLengthConsumer.accept(encodedLength); + } + } + private static CircuitBreaker inFlightRequestsBreaker(CircuitBreakerService circuitBreakerService) { // We always obtain a fresh breaker to reflect changes to the breaker configuration. return circuitBreakerService.getBreaker(CircuitBreaker.IN_FLIGHT_REQUESTS); From 4a4cd4a87df3ce8f865abfa4e7fb3aca315b199a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 27 Sep 2023 13:00:38 +1000 Subject: [PATCH 08/17] use nanoTime --- .../main/java/org/elasticsearch/rest/RestController.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index a376b15f2f243..0efb03c3db1a3 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -28,6 +28,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Streams; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.http.HttpHeadersValidationException; import org.elasticsearch.http.HttpRouteStats; import org.elasticsearch.http.HttpServerTransport; @@ -719,7 +720,7 @@ private static final class ResourceHandlingHttpChannel implements RestChannel { this.circuitBreakerService = circuitBreakerService; this.contentLength = contentLength; this.methodHandlers = methodHandlers; - this.startTime = System.currentTimeMillis(); + this.startTime = rawRelativeTimeInMillis(); } @Override @@ -779,7 +780,7 @@ public void sendResponse(RestResponse response) { try { close(); methodHandlers.addRequestStats(contentLength); - methodHandlers.addResponseTime(System.currentTimeMillis() - startTime); + methodHandlers.addResponseTime(rawRelativeTimeInMillis() - startTime); if (response.isChunked() == false) { methodHandlers.addResponseStats(response.content().length()); } else { @@ -797,6 +798,10 @@ public void sendResponse(RestResponse response) { } } + private static long rawRelativeTimeInMillis() { + return TimeValue.nsecToMSec(System.nanoTime()); + } + private void close() { // attempt to close once atomically if (closed.compareAndSet(false, true) == false) { From 0d207b0af96942dc8aa50c66e9dd062e7eb16426 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 27 Sep 2023 13:55:34 +1000 Subject: [PATCH 09/17] add tests for httpRouteStats --- .../elasticsearch/http/HttpRouteStats.java | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java index 41a134923e136..5f93ab1ee2813 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStats.java @@ -13,12 +13,14 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.network.HandlingTimeTracker; import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; import java.util.Arrays; import java.util.Objects; +import java.util.function.Function; import java.util.stream.IntStream; /** @@ -57,17 +59,32 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject("requests"); builder.field("count", requestCount); builder.humanReadableField("total_size_in_bytes", "total_size", ByteSizeValue.ofBytes(totalRequestSize)); - histogramToXContent(builder, "size_histogram", "bytes", requestSizeHistogram, HttpRouteStatsTracker.getBucketUpperBounds()); + histogramToXContent( + builder, + "size_histogram", + "bytes", + ByteSizeValue::ofBytes, + requestSizeHistogram, + HttpRouteStatsTracker.getBucketUpperBounds() + ); builder.endObject(); builder.startObject("responses"); builder.field("count", responseCount); builder.humanReadableField("total_size_in_bytes", "total_size", ByteSizeValue.ofBytes(totalResponseSize)); - histogramToXContent(builder, "size_histogram", "bytes", responseSizeHistogram, HttpRouteStatsTracker.getBucketUpperBounds()); + histogramToXContent( + builder, + "size_histogram", + "bytes", + ByteSizeValue::ofBytes, + responseSizeHistogram, + HttpRouteStatsTracker.getBucketUpperBounds() + ); histogramToXContent( builder, "handling_time_histogram", "millis", + TimeValue::timeValueMillis, responseTimeHistogram, HandlingTimeTracker.getBucketUpperBounds() ); @@ -76,8 +93,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder.endObject(); } - static void histogramToXContent(XContentBuilder builder, String fieldName, String unitName, long[] histogram, int[] bucketBounds) - throws IOException { + static void histogramToXContent( + XContentBuilder builder, + String fieldName, + String unitName, + Function humanReadableValueFunc, + long[] histogram, + int[] bucketBounds + ) throws IOException { assert histogram.length == bucketBounds.length + 1; builder.startArray(fieldName); @@ -93,10 +116,10 @@ static void histogramToXContent(XContentBuilder builder, String fieldName, Strin for (int i = firstBucket; i < histogram.length && 0 < remainingCount; i++) { builder.startObject(); if (i > 0) { - builder.humanReadableField("ge_" + unitName, "ge", ByteSizeValue.ofBytes(bucketBounds[i - 1])); + builder.humanReadableField("ge_" + unitName, "ge", humanReadableValueFunc.apply(bucketBounds[i - 1])); } if (i < bucketBounds.length) { - builder.humanReadableField("lt_" + unitName, "lt", ByteSizeValue.ofBytes(bucketBounds[i])); + builder.humanReadableField("lt_" + unitName, "lt", humanReadableValueFunc.apply(bucketBounds[i])); } builder.field("count", histogram[i]); builder.endObject(); From 860341ec95ca7a4b4410e5ea1ed73e76b0b16627 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 27 Sep 2023 13:56:14 +1000 Subject: [PATCH 10/17] add tests for httpRouteStats --- .../http/HttpRouteStatsTests.java | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java diff --git a/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java new file mode 100644 index 0000000000000..da9c1a7892054 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java @@ -0,0 +1,240 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.http; + +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.network.HandlingTimeTracker; +import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.ToXContentFragment; + +import java.util.Arrays; + +public class HttpRouteStatsTests extends ESTestCase { + + public void testToXContent() { + final var requestSizeHistogram = new long[28]; + requestSizeHistogram[2] = 9; + requestSizeHistogram[4] = 10; + + final var responseSizeHistogram = new long[28]; + responseSizeHistogram[3] = 13; + responseSizeHistogram[5] = 14; + + final var responseTimeHistogram = new long[18]; + responseTimeHistogram[4] = 17; + responseTimeHistogram[6] = 18; + + assertEquals( + """ + {"requests":{"count":7,"total_size":"8b","total_size_in_bytes":8,"size_histogram":[\ + {"ge":"2b","ge_bytes":2,"lt":"4b","lt_bytes":4,"count":9},\ + {"ge":"4b","ge_bytes":4,"lt":"8b","lt_bytes":8,"count":0},\ + {"ge":"8b","ge_bytes":8,"lt":"16b","lt_bytes":16,"count":10}\ + ]},"responses":{"count":11,"total_size":"12b","total_size_in_bytes":12,"size_histogram":[\ + {"ge":"4b","ge_bytes":4,"lt":"8b","lt_bytes":8,"count":13},\ + {"ge":"8b","ge_bytes":8,"lt":"16b","lt_bytes":16,"count":0},\ + {"ge":"16b","ge_bytes":16,"lt":"32b","lt_bytes":32,"count":14}\ + ],"handling_time_histogram":[\ + {"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":17},\ + {"ge":"16ms","ge_millis":16,"lt":"32ms","lt_millis":32,"count":0},\ + {"ge":"32ms","ge_millis":32,"lt":"64ms","lt_millis":64,"count":18}\ + ]}}""", + Strings.toString( + new HttpRouteStats(7, 8, requestSizeHistogram, 11, 12, responseSizeHistogram, responseTimeHistogram), + false, + true + ) + ); + } + + public void testSizeHistogram() { + final var histogram = new long[28]; + + asserSizeHistogram(histogram, """ + {"size_histogram":[]}"""); + + histogram[0] = 10; + asserSizeHistogram(histogram, """ + {"size_histogram":[{"lt":"1b","lt_bytes":1,"count":10}]}"""); + + histogram[0] = 0; + histogram[4] = 10; + asserSizeHistogram(histogram, """ + {"size_histogram":[{"ge":"8b","ge_bytes":8,"lt":"16b","lt_bytes":16,"count":10}]}"""); + + histogram[6] = 20; + asserSizeHistogram(histogram, """ + {"size_histogram":[\ + {"ge":"8b","ge_bytes":8,"lt":"16b","lt_bytes":16,"count":10},\ + {"ge":"16b","ge_bytes":16,"lt":"32b","lt_bytes":32,"count":0},\ + {"ge":"32b","ge_bytes":32,"lt":"64b","lt_bytes":64,"count":20}\ + ]}"""); + + histogram[0] = 30; + asserSizeHistogram(histogram, """ + {"size_histogram":[\ + {"lt":"1b","lt_bytes":1,"count":30},\ + {"ge":"1b","ge_bytes":1,"lt":"2b","lt_bytes":2,"count":0},\ + {"ge":"2b","ge_bytes":2,"lt":"4b","lt_bytes":4,"count":0},\ + {"ge":"4b","ge_bytes":4,"lt":"8b","lt_bytes":8,"count":0},\ + {"ge":"8b","ge_bytes":8,"lt":"16b","lt_bytes":16,"count":10},\ + {"ge":"16b","ge_bytes":16,"lt":"32b","lt_bytes":32,"count":0},\ + {"ge":"32b","ge_bytes":32,"lt":"64b","lt_bytes":64,"count":20}\ + ]}"""); + + Arrays.fill(histogram, 0L); + histogram[histogram.length - 1] = 5; + asserSizeHistogram(histogram, """ + {"size_histogram":[{"ge":"64mb","ge_bytes":67108864,"count":5}]}"""); + + histogram[histogram.length - 3] = 6; + asserSizeHistogram(histogram, """ + {"size_histogram":[\ + {"ge":"16mb","ge_bytes":16777216,"lt":"32mb","lt_bytes":33554432,"count":6},\ + {"ge":"32mb","ge_bytes":33554432,"lt":"64mb","lt_bytes":67108864,"count":0},\ + {"ge":"64mb","ge_bytes":67108864,"count":5}\ + ]}"""); + + Arrays.fill(histogram, 1L); + asserSizeHistogram(histogram, """ + {"size_histogram":[\ + {"lt":"1b","lt_bytes":1,"count":1},\ + {"ge":"1b","ge_bytes":1,"lt":"2b","lt_bytes":2,"count":1},\ + {"ge":"2b","ge_bytes":2,"lt":"4b","lt_bytes":4,"count":1},\ + {"ge":"4b","ge_bytes":4,"lt":"8b","lt_bytes":8,"count":1},\ + {"ge":"8b","ge_bytes":8,"lt":"16b","lt_bytes":16,"count":1},\ + {"ge":"16b","ge_bytes":16,"lt":"32b","lt_bytes":32,"count":1},\ + {"ge":"32b","ge_bytes":32,"lt":"64b","lt_bytes":64,"count":1},\ + {"ge":"64b","ge_bytes":64,"lt":"128b","lt_bytes":128,"count":1},\ + {"ge":"128b","ge_bytes":128,"lt":"256b","lt_bytes":256,"count":1},\ + {"ge":"256b","ge_bytes":256,"lt":"512b","lt_bytes":512,"count":1},\ + {"ge":"512b","ge_bytes":512,"lt":"1kb","lt_bytes":1024,"count":1},\ + {"ge":"1kb","ge_bytes":1024,"lt":"2kb","lt_bytes":2048,"count":1},\ + {"ge":"2kb","ge_bytes":2048,"lt":"4kb","lt_bytes":4096,"count":1},\ + {"ge":"4kb","ge_bytes":4096,"lt":"8kb","lt_bytes":8192,"count":1},\ + {"ge":"8kb","ge_bytes":8192,"lt":"16kb","lt_bytes":16384,"count":1},\ + {"ge":"16kb","ge_bytes":16384,"lt":"32kb","lt_bytes":32768,"count":1},\ + {"ge":"32kb","ge_bytes":32768,"lt":"64kb","lt_bytes":65536,"count":1},\ + {"ge":"64kb","ge_bytes":65536,"lt":"128kb","lt_bytes":131072,"count":1},\ + {"ge":"128kb","ge_bytes":131072,"lt":"256kb","lt_bytes":262144,"count":1},\ + {"ge":"256kb","ge_bytes":262144,"lt":"512kb","lt_bytes":524288,"count":1},\ + {"ge":"512kb","ge_bytes":524288,"lt":"1mb","lt_bytes":1048576,"count":1},\ + {"ge":"1mb","ge_bytes":1048576,"lt":"2mb","lt_bytes":2097152,"count":1},\ + {"ge":"2mb","ge_bytes":2097152,"lt":"4mb","lt_bytes":4194304,"count":1},\ + {"ge":"4mb","ge_bytes":4194304,"lt":"8mb","lt_bytes":8388608,"count":1},\ + {"ge":"8mb","ge_bytes":8388608,"lt":"16mb","lt_bytes":16777216,"count":1},\ + {"ge":"16mb","ge_bytes":16777216,"lt":"32mb","lt_bytes":33554432,"count":1},\ + {"ge":"32mb","ge_bytes":33554432,"lt":"64mb","lt_bytes":67108864,"count":1},\ + {"ge":"64mb","ge_bytes":67108864,"count":1}\ + ]}"""); + } + + private static void asserSizeHistogram(long[] histogram, String expectedJson) { + assertEquals(expectedJson, Strings.toString((ToXContentFragment) (builder, params) -> { + HttpRouteStats.histogramToXContent( + builder, + "size_histogram", + "bytes", + ByteSizeValue::ofBytes, + histogram, + HttpRouteStatsTracker.getBucketUpperBounds() + ); + return builder; + }, false, true)); + } + + public void testHandlingTimeHistogram() { + final var histogram = new long[18]; + + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[]}"""); + + histogram[0] = 10; + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[{"lt":"1ms","lt_millis":1,"count":10}]}"""); + + histogram[0] = 0; + histogram[4] = 10; + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[{"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10}]}"""); + + histogram[6] = 20; + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[\ + {"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10},\ + {"ge":"16ms","ge_millis":16,"lt":"32ms","lt_millis":32,"count":0},\ + {"ge":"32ms","ge_millis":32,"lt":"64ms","lt_millis":64,"count":20}\ + ]}"""); + + histogram[0] = 30; + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[\ + {"lt":"1ms","lt_millis":1,"count":30},\ + {"ge":"1ms","ge_millis":1,"lt":"2ms","lt_millis":2,"count":0},\ + {"ge":"2ms","ge_millis":2,"lt":"4ms","lt_millis":4,"count":0},\ + {"ge":"4ms","ge_millis":4,"lt":"8ms","lt_millis":8,"count":0},\ + {"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":10},\ + {"ge":"16ms","ge_millis":16,"lt":"32ms","lt_millis":32,"count":0},\ + {"ge":"32ms","ge_millis":32,"lt":"64ms","lt_millis":64,"count":20}\ + ]}"""); + + Arrays.fill(histogram, 0L); + histogram[histogram.length - 1] = 5; + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[{"ge":"1m","ge_millis":65536,"count":5}]}"""); + + histogram[histogram.length - 3] = 6; + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[\ + {"ge":"16.3s","ge_millis":16384,"lt":"32.7s","lt_millis":32768,"count":6},\ + {"ge":"32.7s","ge_millis":32768,"lt":"1m","lt_millis":65536,"count":0},\ + {"ge":"1m","ge_millis":65536,"count":5}\ + ]}"""); + + Arrays.fill(histogram, 1L); + assertHandlingTimeHistogram(histogram, """ + {"handling_time_histogram":[\ + {"lt":"1ms","lt_millis":1,"count":1},\ + {"ge":"1ms","ge_millis":1,"lt":"2ms","lt_millis":2,"count":1},\ + {"ge":"2ms","ge_millis":2,"lt":"4ms","lt_millis":4,"count":1},\ + {"ge":"4ms","ge_millis":4,"lt":"8ms","lt_millis":8,"count":1},\ + {"ge":"8ms","ge_millis":8,"lt":"16ms","lt_millis":16,"count":1},\ + {"ge":"16ms","ge_millis":16,"lt":"32ms","lt_millis":32,"count":1},\ + {"ge":"32ms","ge_millis":32,"lt":"64ms","lt_millis":64,"count":1},\ + {"ge":"64ms","ge_millis":64,"lt":"128ms","lt_millis":128,"count":1},\ + {"ge":"128ms","ge_millis":128,"lt":"256ms","lt_millis":256,"count":1},\ + {"ge":"256ms","ge_millis":256,"lt":"512ms","lt_millis":512,"count":1},\ + {"ge":"512ms","ge_millis":512,"lt":"1s","lt_millis":1024,"count":1},\ + {"ge":"1s","ge_millis":1024,"lt":"2s","lt_millis":2048,"count":1},\ + {"ge":"2s","ge_millis":2048,"lt":"4s","lt_millis":4096,"count":1},\ + {"ge":"4s","ge_millis":4096,"lt":"8.1s","lt_millis":8192,"count":1},\ + {"ge":"8.1s","ge_millis":8192,"lt":"16.3s","lt_millis":16384,"count":1},\ + {"ge":"16.3s","ge_millis":16384,"lt":"32.7s","lt_millis":32768,"count":1},\ + {"ge":"32.7s","ge_millis":32768,"lt":"1m","lt_millis":65536,"count":1},\ + {"ge":"1m","ge_millis":65536,"count":1}\ + ]}"""); + } + + private static void assertHandlingTimeHistogram(long[] histogram, String expectedJson) { + assertEquals(expectedJson, Strings.toString((ToXContentFragment) (builder, params) -> { + HttpRouteStats.histogramToXContent( + builder, + "handling_time_histogram", + "millis", + TimeValue::timeValueMillis, + histogram, + HandlingTimeTracker.getBucketUpperBounds() + ); + return builder; + }, false, true)); + } + +} From 5849e3d1e3752df71fe6e89fab0584b4f3acbcf2 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 27 Sep 2023 22:49:41 +1000 Subject: [PATCH 11/17] address feedback --- .../http/HttpRouteStatsTracker.java | 5 ++- .../org/elasticsearch/http/HttpStats.java | 11 +---- .../elasticsearch/rest/RestController.java | 44 +++++++++++++------ .../http/HttpRouteStatsTests.java | 20 +++++---- .../elasticsearch/http/HttpStatsTests.java | 4 +- .../info/RestClusterInfoActionTests.java | 11 ++--- 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java index b6cda33bfc1f6..85140b072dff8 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java @@ -16,11 +16,12 @@ public class HttpRouteStatsTracker { /* - * default http.max_content_length is 100 MB so that the last histogram bucket is > 64MB (2^26) + * Default http.max_content_length is 100 MB. But response size can be much larger. + * So we choose the last histogram bucket to be > 1GB (2^30) */ public static int[] getBucketUpperBounds() { - var bounds = new int[27]; + var bounds = new int[31]; for (int i = 0; i < bounds.length; i++) { bounds[i] = 1 << i; } diff --git a/server/src/main/java/org/elasticsearch/http/HttpStats.java b/server/src/main/java/org/elasticsearch/http/HttpStats.java index 45ab382208df4..3383ab6446bf3 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpStats.java +++ b/server/src/main/java/org/elasticsearch/http/HttpStats.java @@ -110,9 +110,7 @@ public Iterator toXContentChunked(ToXContent.Params outerP clientStats.iterator(), Iterators.single((builder, params) -> { builder.endArray(); - if (httpRouteStats.isEmpty() == false) { - builder.startObject(Fields.ROUTES); - } + builder.startObject(Fields.ROUTES); return builder; }), Iterators.map(httpRouteStats.entrySet().iterator(), entry -> (builder, params) -> { @@ -120,12 +118,7 @@ public Iterator toXContentChunked(ToXContent.Params outerP entry.getValue().toXContent(builder, params); return builder; }), - Iterators.single((builder, params) -> { - if (httpRouteStats.isEmpty() == false) { - builder.endObject(); - } - return builder.endObject(); - }) + Iterators.single((builder, params) -> builder.endObject().endObject()) ); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 0efb03c3db1a3..15890e19e6b9f 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; import org.elasticsearch.common.recycler.Recycler; +import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Nullable; @@ -45,19 +46,18 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.Spliterator; -import java.util.Spliterators; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Consumer; import java.util.function.Supplier; import java.util.function.UnaryOperator; -import java.util.stream.StreamSupport; import static org.elasticsearch.indices.SystemIndices.EXTERNAL_SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY; import static org.elasticsearch.indices.SystemIndices.SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY; @@ -363,9 +363,16 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th @Override public Map getStats() { - return StreamSupport.stream(Spliterators.spliteratorUnknownSize(handlers.allNodeValues(), Spliterator.ORDERED), false) - .filter(mh -> mh.getStats().requestCount() > 0 || mh.getStats().responseCount() > 0) - .collect(Maps.toUnmodifiableSortedMap(MethodHandlers::getPath, MethodHandlers::getStats)); + final Iterator methodHandlersIterator = handlers.allNodeValues(); + final SortedMap allStats = new TreeMap<>(); + while (methodHandlersIterator.hasNext()) { + final MethodHandlers mh = methodHandlersIterator.next(); + final HttpRouteStats stats = mh.getStats(); + if (stats.requestCount() > 0 || stats.responseCount() > 0) { + allStats.put(mh.getPath(), stats); + } + } + return Collections.unmodifiableSortedMap(allStats); } private void dispatchRequest( @@ -786,7 +793,7 @@ public void sendResponse(RestResponse response) { } else { response = RestResponse.chunked( response.status(), - new EncodeLengthTrackingChunkedRestResponseBody(response.chunkedContent(), methodHandlers::addResponseStats) + new EncodedLengthTrackingChunkedRestResponseBody(response.chunkedContent(), methodHandlers) ); } delegate.sendResponse(response); @@ -811,15 +818,15 @@ private void close() { } } - private static class EncodeLengthTrackingChunkedRestResponseBody implements ChunkedRestResponseBody { + private static class EncodedLengthTrackingChunkedRestResponseBody implements ChunkedRestResponseBody { private final ChunkedRestResponseBody delegate; - private final Consumer encodedLengthConsumer; + private final MethodHandlers methodHandlers; private int encodedLength = 0; - private EncodeLengthTrackingChunkedRestResponseBody(ChunkedRestResponseBody delegate, Consumer encodedLengthConsumer) { + private EncodedLengthTrackingChunkedRestResponseBody(ChunkedRestResponseBody delegate, MethodHandlers methodHandlers) { this.delegate = delegate; - this.encodedLengthConsumer = encodedLengthConsumer; + this.methodHandlers = methodHandlers; } @Override @@ -830,7 +837,16 @@ public boolean isDone() { @Override public ReleasableBytesReference encodeChunk(int sizeHint, Recycler recycler) throws IOException { final ReleasableBytesReference bytesReference = delegate.encodeChunk(sizeHint, recycler); - encodedLength += bytesReference.length(); + try { + encodedLength = Math.addExact(encodedLength, bytesReference.length()); + } catch (ArithmeticException e) { + logger.debug( + "response size for [{}] is greater than [{}]", + methodHandlers.getPath(), + ByteSizeValue.ofBytes(Integer.MAX_VALUE) + ); + encodedLength = Integer.MAX_VALUE; + } return bytesReference; } @@ -842,7 +858,7 @@ public String getResponseContentTypeString() { @Override public void close() { delegate.close(); - encodedLengthConsumer.accept(encodedLength); + methodHandlers.addResponseStats(encodedLength); } } diff --git a/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java index da9c1a7892054..27cb629c03441 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpRouteStatsTests.java @@ -20,11 +20,11 @@ public class HttpRouteStatsTests extends ESTestCase { public void testToXContent() { - final var requestSizeHistogram = new long[28]; + final var requestSizeHistogram = new long[32]; requestSizeHistogram[2] = 9; requestSizeHistogram[4] = 10; - final var responseSizeHistogram = new long[28]; + final var responseSizeHistogram = new long[32]; responseSizeHistogram[3] = 13; responseSizeHistogram[5] = 14; @@ -56,7 +56,7 @@ public void testToXContent() { } public void testSizeHistogram() { - final var histogram = new long[28]; + final var histogram = new long[32]; asserSizeHistogram(histogram, """ {"size_histogram":[]}"""); @@ -93,14 +93,14 @@ public void testSizeHistogram() { Arrays.fill(histogram, 0L); histogram[histogram.length - 1] = 5; asserSizeHistogram(histogram, """ - {"size_histogram":[{"ge":"64mb","ge_bytes":67108864,"count":5}]}"""); + {"size_histogram":[{"ge":"1gb","ge_bytes":1073741824,"count":5}]}"""); histogram[histogram.length - 3] = 6; asserSizeHistogram(histogram, """ {"size_histogram":[\ - {"ge":"16mb","ge_bytes":16777216,"lt":"32mb","lt_bytes":33554432,"count":6},\ - {"ge":"32mb","ge_bytes":33554432,"lt":"64mb","lt_bytes":67108864,"count":0},\ - {"ge":"64mb","ge_bytes":67108864,"count":5}\ + {"ge":"256mb","ge_bytes":268435456,"lt":"512mb","lt_bytes":536870912,"count":6},\ + {"ge":"512mb","ge_bytes":536870912,"lt":"1gb","lt_bytes":1073741824,"count":0},\ + {"ge":"1gb","ge_bytes":1073741824,"count":5}\ ]}"""); Arrays.fill(histogram, 1L); @@ -133,7 +133,11 @@ public void testSizeHistogram() { {"ge":"8mb","ge_bytes":8388608,"lt":"16mb","lt_bytes":16777216,"count":1},\ {"ge":"16mb","ge_bytes":16777216,"lt":"32mb","lt_bytes":33554432,"count":1},\ {"ge":"32mb","ge_bytes":33554432,"lt":"64mb","lt_bytes":67108864,"count":1},\ - {"ge":"64mb","ge_bytes":67108864,"count":1}\ + {"ge":"64mb","ge_bytes":67108864,"lt":"128mb","lt_bytes":134217728,"count":1},\ + {"ge":"128mb","ge_bytes":134217728,"lt":"256mb","lt_bytes":268435456,"count":1},\ + {"ge":"256mb","ge_bytes":268435456,"lt":"512mb","lt_bytes":536870912,"count":1},\ + {"ge":"512mb","ge_bytes":536870912,"lt":"1gb","lt_bytes":1073741824,"count":1},\ + {"ge":"1gb","ge_bytes":1073741824,"count":1}\ ]}"""); } diff --git a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java index 139017bcb9b99..a86f497ad45aa 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java @@ -40,11 +40,11 @@ public void testMerge() { } public void testToXContent() { - final var requestSizeHistogram = new long[28]; + final var requestSizeHistogram = new long[32]; requestSizeHistogram[2] = 9; requestSizeHistogram[4] = 10; - final var responseSizeHistogram = new long[28]; + final var responseSizeHistogram = new long[32]; responseSizeHistogram[3] = 13; responseSizeHistogram[5] = 14; diff --git a/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java index ccc7f1c031e0e..f3ef110ad4ce8 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/info/RestClusterInfoActionTests.java @@ -81,6 +81,11 @@ public void testHttpResponseMapper() { var httpStats = (HttpStats) RestClusterInfoAction.RESPONSE_MAPPER.get("http").apply(response); + final Map httpRouteStatsMap = new HashMap<>(); + for (var ns : nodeStats) { + ns.getHttp().httpRouteStats().forEach((k, v) -> httpRouteStatsMap.merge(k, v, HttpRouteStats::merge)); + } + assertEquals( httpStats, new HttpStats( @@ -92,11 +97,7 @@ public void testHttpResponseMapper() { .map(Collection::stream) .reduce(Stream.of(), Stream::concat) .toList(), - nodeStats.stream().map(NodeStats::getHttp).map(HttpStats::httpRouteStats).reduce(Map.of(), (l, r) -> { - final var m = new HashMap<>(l); - r.forEach((k, v) -> m.merge(k, v, HttpRouteStats::merge)); - return Map.copyOf(m); - }) + httpRouteStatsMap ) ); } From 49b95f8765af2801adb76fee643fb104baa8a5da Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 27 Sep 2023 23:58:37 +1000 Subject: [PATCH 12/17] upgrade encoded response length to long --- .../elasticsearch/http/HttpRouteStatsTracker.java | 14 +++++++++++--- .../org/elasticsearch/rest/MethodHandlers.java | 2 +- .../org/elasticsearch/rest/RestController.java | 14 ++------------ 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java index 85140b072dff8..44db13c0ada91 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java @@ -30,7 +30,7 @@ public static int[] getBucketUpperBounds() { private static final int BUCKET_COUNT = getBucketUpperBounds().length + 1; - private static final long LAST_BUCKET_LOWER_BOUND = getBucketUpperBounds()[BUCKET_COUNT - 2]; + private static final int LAST_BUCKET_LOWER_BOUND = getBucketUpperBounds()[BUCKET_COUNT - 2]; private record StatsTracker(LongAdder count, LongAdder totalSize, AtomicLongArray histogram) { StatsTracker { @@ -43,7 +43,7 @@ private record StatsTracker(LongAdder count, LongAdder totalSize, AtomicLongArra this(new LongAdder(), new LongAdder(), new AtomicLongArray(BUCKET_COUNT)); } - void addStats(int contentLength) { + void addStats(long contentLength) { count().increment(); totalSize().add(contentLength); histogram().incrementAndGet(bucket(contentLength)); @@ -58,6 +58,14 @@ long[] getHistogram() { } } + private static int bucket(long contentLength) { + if (contentLength > Integer.MAX_VALUE) { + return bucket(Integer.MAX_VALUE); + } else { + return bucket((int) contentLength); + } + } + private static int bucket(int contentLength) { if (contentLength <= 0) { return 0; @@ -76,7 +84,7 @@ public void addRequestStats(int contentLength) { requestStats.addStats(contentLength); } - public void addResponseStats(int contentLength) { + public void addResponseStats(long contentLength) { responseStats.addStats(contentLength); } diff --git a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java index 76c53ac26b20b..ef706ea03cdfe 100644 --- a/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java +++ b/server/src/main/java/org/elasticsearch/rest/MethodHandlers.java @@ -84,7 +84,7 @@ public void addRequestStats(int contentLength) { statsTracker.addRequestStats(contentLength); } - public void addResponseStats(int contentLength) { + public void addResponseStats(long contentLength) { statsTracker.addResponseStats(contentLength); } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 15890e19e6b9f..b51468edff63b 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -23,7 +23,6 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; import org.elasticsearch.common.recycler.Recycler; -import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Nullable; @@ -822,7 +821,7 @@ private static class EncodedLengthTrackingChunkedRestResponseBody implements Chu private final ChunkedRestResponseBody delegate; private final MethodHandlers methodHandlers; - private int encodedLength = 0; + private long encodedLength = 0; private EncodedLengthTrackingChunkedRestResponseBody(ChunkedRestResponseBody delegate, MethodHandlers methodHandlers) { this.delegate = delegate; @@ -837,16 +836,7 @@ public boolean isDone() { @Override public ReleasableBytesReference encodeChunk(int sizeHint, Recycler recycler) throws IOException { final ReleasableBytesReference bytesReference = delegate.encodeChunk(sizeHint, recycler); - try { - encodedLength = Math.addExact(encodedLength, bytesReference.length()); - } catch (ArithmeticException e) { - logger.debug( - "response size for [{}] is greater than [{}]", - methodHandlers.getPath(), - ByteSizeValue.ofBytes(Integer.MAX_VALUE) - ); - encodedLength = Integer.MAX_VALUE; - } + encodedLength += bytesReference.length(); return bytesReference; } From 9f84029f69bb8546b113de44390956524472ee32 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 28 Sep 2023 00:00:40 +1000 Subject: [PATCH 13/17] fix test --- .../src/test/java/org/elasticsearch/http/HttpStatsTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java index a86f497ad45aa..8d6b0c7ef8861 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpStatsTests.java @@ -104,10 +104,10 @@ public static HttpRouteStats randomHttpRouteStats() { return new HttpRouteStats( randomLongBetween(0, 99), randomLongBetween(0, 9999), - IntStream.range(0, 28).mapToLong(i -> randomLongBetween(0, 42)).toArray(), + IntStream.range(0, 32).mapToLong(i -> randomLongBetween(0, 42)).toArray(), randomLongBetween(0, 99), randomLongBetween(0, 9999), - IntStream.range(0, 28).mapToLong(i -> randomLongBetween(0, 42)).toArray(), + IntStream.range(0, 32).mapToLong(i -> randomLongBetween(0, 42)).toArray(), IntStream.range(0, 18).mapToLong(i -> randomLongBetween(0, 42)).toArray() ); } From 589b35d2dfb5a8a5037151f12646021f48043c37 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 28 Sep 2023 00:03:13 +1000 Subject: [PATCH 14/17] assertion --- .../main/java/org/elasticsearch/http/HttpRouteStatsTracker.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java index 44db13c0ada91..5695cfa3d3566 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java +++ b/server/src/main/java/org/elasticsearch/http/HttpRouteStatsTracker.java @@ -62,6 +62,7 @@ private static int bucket(long contentLength) { if (contentLength > Integer.MAX_VALUE) { return bucket(Integer.MAX_VALUE); } else { + assert contentLength >= 0; return bucket((int) contentLength); } } From 345623a68ce0651a2cd3deeeaa4c390e34fee85e Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 12 Oct 2023 21:18:16 +1100 Subject: [PATCH 15/17] Update qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java Co-authored-by: David Turner --- .../java/org/elasticsearch/http/HttpSmokeTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java index 804aa5a2bda3a..21896cdd3bc89 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java @@ -48,6 +48,6 @@ protected boolean ignoreExternalCluster() { } public static void assertOK(Response response) { - assertThat(response.getStatusLine().getStatusCode(), anyOf(equalTo(200), equalTo(201))); + assertThat(response.getStatusLine().getStatusCode(), oneOf(200, 201); } } From a4355c4a430f967fd0b3dfc7c0f7608885f1e273 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 12 Oct 2023 21:19:21 +1100 Subject: [PATCH 16/17] fix import --- .../java/org/elasticsearch/http/HttpSmokeTestCase.java | 5 ++--- .../java/org/elasticsearch/gateway/GatewayServiceTests.java | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java index 21896cdd3bc89..2533b213d469c 100644 --- a/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java +++ b/qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/HttpSmokeTestCase.java @@ -18,8 +18,7 @@ import java.util.Collection; import java.util.List; -import static org.hamcrest.Matchers.anyOf; -import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.oneOf; public abstract class HttpSmokeTestCase extends ESIntegTestCase { @@ -48,6 +47,6 @@ protected boolean ignoreExternalCluster() { } public static void assertOK(Response response) { - assertThat(response.getStatusLine().getStatusCode(), oneOf(200, 201); + assertThat(response.getStatusLine().getStatusCode(), oneOf(200, 201)); } } diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java index b4296ae684840..b0847c862b8a7 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java @@ -36,6 +36,7 @@ import org.junit.Before; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Stream; import static org.elasticsearch.common.settings.ClusterSettings.createBuiltInClusterSettings; import static org.elasticsearch.gateway.GatewayService.EXPECTED_DATA_NODES_SETTING; @@ -69,6 +70,10 @@ public void setUp() throws Exception { dataNodeIdPrefix = randomAlphaOfLength(10) + "-"; } + public void test() { + System.out.println(Stream.of(1, 2, null).mapToInt(i -> i).max().orElse(0)); + } + private GatewayService createGatewayService(final Settings.Builder settingsBuilder, final ClusterState initialState) { return createGatewayService(createClusterService(settingsBuilder, initialState)); } From bd4ef2cea9db6b40f8802ab657cc2b883bd6397b Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 12 Oct 2023 21:26:35 +1100 Subject: [PATCH 17/17] tweak --- .../java/org/elasticsearch/gateway/GatewayServiceTests.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java b/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java index b0847c862b8a7..b4296ae684840 100644 --- a/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java +++ b/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java @@ -36,7 +36,6 @@ import org.junit.Before; import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Stream; import static org.elasticsearch.common.settings.ClusterSettings.createBuiltInClusterSettings; import static org.elasticsearch.gateway.GatewayService.EXPECTED_DATA_NODES_SETTING; @@ -70,10 +69,6 @@ public void setUp() throws Exception { dataNodeIdPrefix = randomAlphaOfLength(10) + "-"; } - public void test() { - System.out.println(Stream.of(1, 2, null).mapToInt(i -> i).max().orElse(0)); - } - private GatewayService createGatewayService(final Settings.Builder settingsBuilder, final ClusterState initialState) { return createGatewayService(createClusterService(settingsBuilder, initialState)); }