Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/123272.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 123272
summary: Set Connect Timeout to 5s
area: Machine Learning
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.inference.external.http;

import org.apache.http.HttpResponse;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.concurrent.FutureCallback;
import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
Expand Down Expand Up @@ -54,16 +55,21 @@ public static HttpClient create(
HttpSettings settings,
ThreadPool threadPool,
PoolingNHttpClientConnectionManager connectionManager,
ThrottlerManager throttlerManager
ThrottlerManager throttlerManager,
RequestConfig requestConfig
) {
CloseableHttpAsyncClient client = createAsyncClient(Objects.requireNonNull(connectionManager));
var client = createAsyncClient(Objects.requireNonNull(connectionManager), Objects.requireNonNull(requestConfig));

return new HttpClient(settings, client, threadPool, throttlerManager);
}

private static CloseableHttpAsyncClient createAsyncClient(PoolingNHttpClientConnectionManager connectionManager) {
HttpAsyncClientBuilder clientBuilder = HttpAsyncClientBuilder.create();
clientBuilder.setConnectionManager(connectionManager);
private static CloseableHttpAsyncClient createAsyncClient(
PoolingNHttpClientConnectionManager connectionManager,
RequestConfig requestConfig
) {
HttpAsyncClientBuilder clientBuilder = HttpAsyncClientBuilder.create()
.setConnectionManager(connectionManager)
.setDefaultRequestConfig(requestConfig);
// The apache client will be shared across all connections because it can be expensive to create it
// so we don't want to support cookies to avoid accidental authentication for unauthorized users
clientBuilder.disableCookieManagement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.inference.external.http;

import org.apache.http.client.config.RequestConfig;
import org.apache.http.config.Registry;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.impl.nio.conn.PoolingNHttpClientConnectionManager;
Expand Down Expand Up @@ -74,6 +75,7 @@ public class HttpClientManager implements Closeable {
);

private static final TimeValue DEFAULT_CONNECTION_MAX_IDLE_TIME_SETTING = DEFAULT_CONNECTION_EVICTION_THREAD_INTERVAL_TIME;
private static final TimeValue DEFAULT_CONNECT_TIMEOUT = TimeValue.timeValueSeconds(5);
/**
* The max duration of time for a connection to be marked as idle and ready to be closed. This defines the amount of time
* a connection can be unused in the connection pool before being closed the next time the eviction thread runs.
Expand All @@ -89,6 +91,12 @@ public class HttpClientManager implements Closeable {
Setting.Property.Dynamic
);

public static final Setting<TimeValue> CONNECTION_TIMEOUT = Setting.timeSetting(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be added to getSettingsDefinitions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we move this into HttpSettings. I don't think we need to add a addSettingsUpdateConsumer because we'd have to close the client and recreate it but I think that class was for the settings for the client.

"xpack.inference.http.connect_timeout",
DEFAULT_CONNECT_TIMEOUT,
Setting.Property.NodeScope
);

private final ThreadPool threadPool;
private final PoolingNHttpClientConnectionManager connectionManager;
private IdleConnectionEvictor connectionEvictor;
Expand Down Expand Up @@ -137,7 +145,15 @@ public static HttpClientManager create(
setMaxConnections(MAX_TOTAL_CONNECTIONS.get(settings));
setMaxRouteConnections(MAX_ROUTE_CONNECTIONS.get(settings));

this.httpClient = HttpClient.create(new HttpSettings(settings, clusterService), threadPool, connectionManager, throttlerManager);
var requestConfig = RequestConfig.custom().setConnectTimeout(Math.toIntExact(CONNECTION_TIMEOUT.get(settings).getMillis())).build();

this.httpClient = HttpClient.create(
new HttpSettings(settings, clusterService),
threadPool,
connectionManager,
throttlerManager,
requestConfig
);

this.evictionInterval = CONNECTION_EVICTION_THREAD_INTERVAL_SETTING.get(settings);
this.connectionMaxIdle = CONNECTION_MAX_IDLE_TIME_SETTING.get(settings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import static org.elasticsearch.xpack.core.inference.action.InferenceAction.Request.DEFAULT_TIMEOUT;
import static org.elasticsearch.xpack.inference.services.elastic.ElasticInferenceService.ELASTIC_INFERENCE_SERVICE_IDENTIFIER;

/**
Expand All @@ -39,6 +38,7 @@ public class ElasticInferenceServiceAuthorizationHandler {

private static final String FAILED_TO_RETRIEVE_MESSAGE =
"Failed to retrieve the authorization information from the Elastic Inference Service.";
private static final TimeValue DEFAULT_AUTH_TIMEOUT = TimeValue.timeValueMinutes(1);
private static final ResponseHandler AUTH_RESPONSE_HANDLER = createAuthResponseHandler();

private static ResponseHandler createAuthResponseHandler() {
Expand Down Expand Up @@ -110,7 +110,7 @@ public void getAuthorization(ActionListener<ElasticInferenceServiceAuthorization

var request = new ElasticInferenceServiceAuthorizationRequest(baseUrl, getCurrentTraceInfo());

sender.sendWithoutQueuing(logger, request, AUTH_RESPONSE_HANDLER, DEFAULT_TIMEOUT, newListener);
sender.sendWithoutQueuing(logger, request, AUTH_RESPONSE_HANDLER, DEFAULT_AUTH_TIMEOUT, newListener);
} catch (Exception e) {
logger.warn(Strings.format("Retrieving the authorization information encountered an exception: %s", e));
requestCompleteLatch.countDown();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.inference.external.http;

import org.apache.http.HttpHeaders;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.protocol.HttpClientContext;
Expand Down Expand Up @@ -84,7 +85,15 @@ public void testSend_MockServerReceivesRequest() throws Exception {
String paramValue = randomAlphaOfLength(3);
var httpPost = createHttpPost(webServer.getPort(), paramKey, paramValue);

try (var httpClient = HttpClient.create(emptyHttpSettings(), threadPool, createConnectionManager(), mockThrottlerManager())) {
try (
var httpClient = HttpClient.create(
emptyHttpSettings(),
threadPool,
createConnectionManager(),
mockThrottlerManager(),
RequestConfig.DEFAULT
)
) {
httpClient.start();

PlainActionFuture<HttpResult> listener = new PlainActionFuture<>();
Expand All @@ -102,7 +111,15 @@ public void testSend_MockServerReceivesRequest() throws Exception {
}

public void testSend_ThrowsErrorIfCalledBeforeStart() throws Exception {
try (var httpClient = HttpClient.create(emptyHttpSettings(), threadPool, createConnectionManager(), mockThrottlerManager())) {
try (
var httpClient = HttpClient.create(
emptyHttpSettings(),
threadPool,
createConnectionManager(),
mockThrottlerManager(),
RequestConfig.DEFAULT
)
) {
PlainActionFuture<HttpResult> listener = new PlainActionFuture<>();
var thrownException = expectThrows(
AssertionError.class,
Expand Down Expand Up @@ -237,7 +254,15 @@ public void testSend_FailsWhenMaxBytesReadIsExceeded() throws Exception {
Settings settings = Settings.builder().put(HttpSettings.MAX_HTTP_RESPONSE_SIZE.getKey(), ByteSizeValue.ONE).build();
var httpSettings = createHttpSettings(settings);

try (var httpClient = HttpClient.create(httpSettings, threadPool, createConnectionManager(), mockThrottlerManager())) {
try (
var httpClient = HttpClient.create(
httpSettings,
threadPool,
createConnectionManager(),
mockThrottlerManager(),
RequestConfig.DEFAULT
)
) {
httpClient.start();

PlainActionFuture<HttpResult> listener = new PlainActionFuture<>();
Expand Down