From 565a5e245657ddb078a17ebdeddd62864149cae6 Mon Sep 17 00:00:00 2001 From: Max Hniebergall Date: Thu, 12 Dec 2024 15:44:51 -0500 Subject: [PATCH 1/3] Replace ElasticsearchTimeoutException with ElasticsearchTimeoutException with 408 status to avoid cuasing 503s --- .../xpack/inference/external/http/sender/RequestTask.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java index 9ccb93a0858ae..4d91415d69e4c 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.inference.external.http.sender; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ListenerTimeouts; @@ -14,6 +15,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; import org.elasticsearch.inference.InferenceServiceResults; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.threadpool.ThreadPool; import java.util.Objects; @@ -64,7 +66,10 @@ private ActionListener getListener( threadPool.executor(UTILITY_THREAD_POOL_NAME), notificationListener, (ignored) -> notificationListener.onFailure( - new ElasticsearchTimeoutException(Strings.format("Request timed out waiting to be sent after [%s]", timeout)) + new ElasticsearchStatusException( + Strings.format("Request timed out waiting to be sent after [%s]", timeout), + RestStatus.REQUEST_TIMEOUT + ) ) ); } From 2a86978d3d56a5ab2d4a8dfad092569adb6754b5 Mon Sep 17 00:00:00 2001 From: Max Hniebergall Date: Thu, 12 Dec 2024 16:13:21 -0500 Subject: [PATCH 2/3] Fix tests --- .../external/http/sender/HttpRequestSenderTests.java | 7 +++++-- .../external/http/sender/RequestExecutorServiceTests.java | 4 +++- .../inference/external/http/sender/RequestTaskTests.java | 5 +++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java index 79f6aa8164b75..1ff38c38eeb27 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.inference.external.http.sender; import org.apache.http.HttpHeaders; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.common.settings.Settings; @@ -162,12 +163,13 @@ public void testHttpRequestSender_Throws_WhenATimeoutOccurs() throws Exception { PlainActionFuture listener = new PlainActionFuture<>(); sender.send(RequestManagerTests.createMock(), new DocumentsOnlyInput(List.of()), TimeValue.timeValueNanos(1), listener); - var thrownException = expectThrows(ElasticsearchTimeoutException.class, () -> listener.actionGet(TIMEOUT)); + var thrownException = expectThrows(ElasticsearchStatusException.class, () -> listener.actionGet(TIMEOUT)); assertThat( thrownException.getMessage(), is(format("Request timed out waiting to be sent after [%s]", TimeValue.timeValueNanos(1))) ); + assertThat(thrownException.status().getStatus(), is(408)); } } @@ -187,12 +189,13 @@ public void testHttpRequestSenderWithTimeout_Throws_WhenATimeoutOccurs() throws PlainActionFuture listener = new PlainActionFuture<>(); sender.send(RequestManagerTests.createMock(), new DocumentsOnlyInput(List.of()), TimeValue.timeValueNanos(1), listener); - var thrownException = expectThrows(ElasticsearchTimeoutException.class, () -> listener.actionGet(TIMEOUT)); + var thrownException = expectThrows(ElasticsearchStatusException.class, () -> listener.actionGet(TIMEOUT)); assertThat( thrownException.getMessage(), is(format("Request timed out waiting to be sent after [%s]", TimeValue.timeValueNanos(1))) ); + assertThat(thrownException.status().getStatus(), is(408)); } } diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java index e09e4968571e5..6d67cf23dc70f 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.inference.external.http.sender; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.PlainActionFuture; @@ -238,12 +239,13 @@ public void testExecute_CallsOnFailure_WhenRequestTimesOut() { var listener = new PlainActionFuture(); service.execute(RequestManagerTests.createMock(), new DocumentsOnlyInput(List.of()), TimeValue.timeValueNanos(1), listener); - var thrownException = expectThrows(ElasticsearchTimeoutException.class, () -> listener.actionGet(TIMEOUT)); + var thrownException = expectThrows(ElasticsearchStatusException.class, () -> listener.actionGet(TIMEOUT)); assertThat( thrownException.getMessage(), is(format("Request timed out waiting to be sent after [%s]", TimeValue.timeValueNanos(1))) ); + assertThat(thrownException.status().getStatus(), is(408)); } public void testExecute_PreservesThreadContext() throws InterruptedException, ExecutionException, TimeoutException { diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTaskTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTaskTests.java index c839c266e9320..e37a1a213569e 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTaskTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTaskTests.java @@ -7,7 +7,7 @@ package org.elasticsearch.xpack.inference.external.http.sender; -import org.elasticsearch.ElasticsearchTimeoutException; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.core.TimeValue; @@ -86,13 +86,14 @@ public void testRequest_ReturnsTimeoutException() { listener ); - var thrownException = expectThrows(ElasticsearchTimeoutException.class, () -> listener.actionGet(TIMEOUT)); + var thrownException = expectThrows(ElasticsearchStatusException.class, () -> listener.actionGet(TIMEOUT)); assertThat( thrownException.getMessage(), is(format("Request timed out waiting to be sent after [%s]", TimeValue.timeValueMillis(1))) ); assertTrue(requestTask.hasCompleted()); assertTrue(requestTask.getRequestCompletedFunction().get()); + assertThat(thrownException.status().getStatus(), is(408)); } public void testRequest_DoesNotCallOnFailureTwiceWhenTimingOut() throws Exception { From e2a301730961c0e94b37e4b057771b9988696c4e Mon Sep 17 00:00:00 2001 From: Max Hniebergall Date: Thu, 12 Dec 2024 16:28:12 -0500 Subject: [PATCH 3/3] precommit --- .../xpack/inference/external/http/sender/RequestTask.java | 1 - .../inference/external/http/sender/HttpRequestSenderTests.java | 1 - .../external/http/sender/RequestExecutorServiceTests.java | 1 - 3 files changed, 3 deletions(-) diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java index 4d91415d69e4c..e5c29adeb9176 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/sender/RequestTask.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.inference.external.http.sender; import org.elasticsearch.ElasticsearchStatusException; -import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ListenerTimeouts; import org.elasticsearch.common.Strings; diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java index 1ff38c38eeb27..b3e7db6009204 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/HttpRequestSenderTests.java @@ -9,7 +9,6 @@ import org.apache.http.HttpHeaders; import org.elasticsearch.ElasticsearchStatusException; -import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; diff --git a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java index 6d67cf23dc70f..7e29fad56812d 100644 --- a/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java +++ b/x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/sender/RequestExecutorServiceTests.java @@ -9,7 +9,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchStatusException; -import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.common.Strings;