Skip to content

Commit

Permalink
Record chunked response size before sending last chunk (elastic#102586)
Browse files Browse the repository at this point in the history
We expect that the HTTP response stats should reflect the responses
received by the client, but today we record that a chunked-encoded
response was sent _after_ sending the last chunk, which might mean that
the client can receive the complete response body and then retrieve
stats that do not include that response. With this commit we record the
stats before sending the last chunk.

Closes elastic#102547
  • Loading branch information
DaveCTurner authored and timgrein committed Nov 30, 2023
1 parent ea027aa commit 682fca9
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ public void testNodeHttpStats() throws IOException {
assertHttpStats(new XContentTestUtils.JsonMapView((Map<String, Object>) nodesMap.get(nodeId)));
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/102547")
@SuppressWarnings("unchecked")
public void testClusterInfoHttpStats() throws IOException {
internalCluster().ensureAtLeastNumDataNodes(3);
performHttpRequests();
Expand Down
12 changes: 9 additions & 3 deletions server/src/main/java/org/elasticsearch/rest/RestController.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.common.path.PathTrie;
import org.elasticsearch.common.recycler.Recycler;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.concurrent.RunOnce;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.RestApiVersion;
Expand Down Expand Up @@ -820,12 +821,12 @@ private void close() {
private static class EncodedLengthTrackingChunkedRestResponseBody implements ChunkedRestResponseBody {

private final ChunkedRestResponseBody delegate;
private final MethodHandlers methodHandlers;
private final RunOnce onCompletion;
private long encodedLength = 0;

private EncodedLengthTrackingChunkedRestResponseBody(ChunkedRestResponseBody delegate, MethodHandlers methodHandlers) {
this.delegate = delegate;
this.methodHandlers = methodHandlers;
this.onCompletion = new RunOnce(() -> methodHandlers.addResponseStats(encodedLength));
}

@Override
Expand All @@ -837,6 +838,9 @@ public boolean isDone() {
public ReleasableBytesReference encodeChunk(int sizeHint, Recycler<BytesRef> recycler) throws IOException {
final ReleasableBytesReference bytesReference = delegate.encodeChunk(sizeHint, recycler);
encodedLength += bytesReference.length();
if (isDone()) {
onCompletion.run();
}
return bytesReference;
}

Expand All @@ -848,7 +852,9 @@ public String getResponseContentTypeString() {
@Override
public void close() {
delegate.close();
methodHandlers.addResponseStats(encodedLength);
// the client might close the connection before we send the last chunk, in which case we won't have recorded the response in the
// stats yet, so we do it now:
onCompletion.run();
}
}

Expand Down

0 comments on commit 682fca9

Please sign in to comment.