Skip to content

Commit ad67f1b

Browse files
Improve Stability of Mock APIs (elastic#49518)
This commit ensures that even for requests that are known to be empty body we at least attempt to read one bytes from the request body input stream. This is done to work around the behavior in `sun.net.httpserver.ServerImpl.Dispatcher#handleEvent` that will close a TCP/HTTP connection that does not have the `eof` flag (see `sun.net.httpserver.LeftOverInputStream#isEOF`) set on its input stream. As far as I can tell the only way to set this flag is to do a read when there's no more bytes buffered. This fixes the numerous connection closing issues because the `ServerImpl` stops closing connections that it thinks weren't fully drained. Also, I removed a now redundant drain loop in the Azure handler as well as removed the connection closing in the error handler's drain action (this shouldn't have an effect but makes things more predictable/easier to reason about IMO). I would suggest merging this and closing related issue after verifying that this fixes things on CI. The way to locally reproduce the issues we're seeing in tests is to make the retry timings more aggressive in e.g. the azure tests and move them to single digit values. This makes the retries happen quickly enough that they run into the async connecting closing of allegedly non-eof connections by `ServerImpl` and produces the exact kinds of failures we're seeing currently. Relates elastic#49401, elastic#49429
1 parent 3a96f4e commit ad67f1b

File tree

4 files changed

+13
-7
lines changed

4 files changed

+13
-7
lines changed

test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131

3232
import java.io.ByteArrayOutputStream;
3333
import java.io.IOException;
34-
import java.io.InputStream;
3534
import java.io.InputStreamReader;
3635
import java.nio.charset.StandardCharsets;
3736
import java.util.Arrays;
@@ -63,6 +62,10 @@ public AzureHttpHandler(final String container) {
6362
@Override
6463
public void handle(final HttpExchange exchange) throws IOException {
6564
final String request = exchange.getRequestMethod() + " " + exchange.getRequestURI().toString();
65+
if (request.startsWith("GET") || request.startsWith("HEAD") || request.startsWith("DELETE")) {
66+
int read = exchange.getRequestBody().read();
67+
assert read == -1 : "Request body should have been empty but saw [" + read + "]";
68+
}
6669
try {
6770
if (Regex.simpleMatch("PUT /" + container + "/*blockid=*", request)) {
6871
// Put Block (https://docs.microsoft.com/en-us/rest/api/storageservices/put-block)
@@ -140,9 +143,6 @@ public void handle(final HttpExchange exchange) throws IOException {
140143

141144
} else if (Regex.simpleMatch("DELETE /" + container + "/*", request)) {
142145
// Delete Blob (https://docs.microsoft.com/en-us/rest/api/storageservices/delete-blob)
143-
try (InputStream is = exchange.getRequestBody()) {
144-
while (is.read() >= 0);
145-
}
146146
blobs.entrySet().removeIf(blob -> blob.getKey().startsWith(exchange.getRequestURI().getPath()));
147147
exchange.sendResponseHeaders(RestStatus.ACCEPTED.getStatus(), -1);
148148

test/fixtures/gcs-fixture/src/main/java/fixture/gcs/GoogleCloudStorageHttpHandler.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ public GoogleCloudStorageHttpHandler(final String bucket) {
7676
@Override
7777
public void handle(final HttpExchange exchange) throws IOException {
7878
final String request = exchange.getRequestMethod() + " " + exchange.getRequestURI().toString();
79+
if (request.startsWith("GET") || request.startsWith("HEAD") || request.startsWith("DELETE")) {
80+
int read = exchange.getRequestBody().read();
81+
assert read == -1 : "Request body should have been empty but saw [" + read + "]";
82+
}
7983
try {
8084
if (Regex.simpleMatch("GET /storage/v1/b/" + bucket + "/o*", request)) {
8185
// List Objects https://cloud.google.com/storage/docs/json_api/v1/objects/list

test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ public S3HttpHandler(final String bucket, @Nullable final String basePath) {
7979
@Override
8080
public void handle(final HttpExchange exchange) throws IOException {
8181
final String request = exchange.getRequestMethod() + " " + exchange.getRequestURI().toString();
82+
if (request.startsWith("GET") || request.startsWith("HEAD") || request.startsWith("DELETE")) {
83+
int read = exchange.getRequestBody().read();
84+
assert read == -1 : "Request body should have been empty but saw [" + read + "]";
85+
}
8286
try {
8387
if (Regex.simpleMatch("POST /" + path + "/*?uploads", request)) {
8488
final String uploadId = UUIDs.randomBase64UUID();

test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,7 @@ protected static String httpServerUrl() {
162162
* Consumes and closes the given {@link InputStream}
163163
*/
164164
protected static void drainInputStream(final InputStream inputStream) throws IOException {
165-
try (InputStream is = inputStream) {
166-
while (is.read(BUFFER) >= 0);
167-
}
165+
while (inputStream.read(BUFFER) >= 0) ;
168166
}
169167

170168
/**

0 commit comments

Comments
 (0)