Move to HttpSettings

Author: prwhelan

x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/HttpClient.java

@@ -55,21 +55,20 @@ public static HttpClient create(
         HttpSettings settings,
         ThreadPool threadPool,
         PoolingNHttpClientConnectionManager connectionManager,
-        ThrottlerManager throttlerManager,
-        RequestConfig requestConfig
+        ThrottlerManager throttlerManager
     ) {
-        var client = createAsyncClient(Objects.requireNonNull(connectionManager), Objects.requireNonNull(requestConfig));
+        var client = createAsyncClient(Objects.requireNonNull(connectionManager), Objects.requireNonNull(settings));
 
         return new HttpClient(settings, client, threadPool, throttlerManager);
     }
 
     private static CloseableHttpAsyncClient createAsyncClient(
         PoolingNHttpClientConnectionManager connectionManager,
-        RequestConfig requestConfig
+        HttpSettings settings
     ) {
-        HttpAsyncClientBuilder clientBuilder = HttpAsyncClientBuilder.create()
-            .setConnectionManager(connectionManager)
-            .setDefaultRequestConfig(requestConfig);
+        var requestConfig = RequestConfig.custom().setConnectTimeout(settings.connectionTimeout()).build();
+
+        var 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();

x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/HttpClientManager.java

@@ -7,7 +7,6 @@
 
 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;
@@ -75,7 +74,6 @@ 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.
@@ -91,12 +89,6 @@ public class HttpClientManager implements Closeable {
         Setting.Property.Dynamic
     );
 
-    public static final Setting<TimeValue> CONNECTION_TIMEOUT = Setting.timeSetting(
-        "xpack.inference.http.connect_timeout",
-        DEFAULT_CONNECT_TIMEOUT,
-        Setting.Property.NodeScope
-    );
-
     private final ThreadPool threadPool;
     private final PoolingNHttpClientConnectionManager connectionManager;
     private IdleConnectionEvictor connectionEvictor;
@@ -145,15 +137,7 @@ public static HttpClientManager create(
         setMaxConnections(MAX_TOTAL_CONNECTIONS.get(settings));
         setMaxRouteConnections(MAX_ROUTE_CONNECTIONS.get(settings));
 
-        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.httpClient = HttpClient.create(new HttpSettings(settings, clusterService), threadPool, connectionManager, throttlerManager);
 
         this.evictionInterval = CONNECTION_EVICTION_THREAD_INTERVAL_SETTING.get(settings);
         this.connectionMaxIdle = CONNECTION_MAX_IDLE_TIME_SETTING.get(settings);
@@ -218,8 +202,7 @@ public static List<Setting<?>> getSettingsDefinitions() {
             MAX_TOTAL_CONNECTIONS,
             MAX_ROUTE_CONNECTIONS,
             CONNECTION_EVICTION_THREAD_INTERVAL_SETTING,
-            CONNECTION_MAX_IDLE_TIME_SETTING,
-            CONNECTION_TIMEOUT
+            CONNECTION_MAX_IDLE_TIME_SETTING
         );
     }
 

x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/http/HttpSettings.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.common.unit.ByteSizeValue;
+import org.elasticsearch.core.TimeValue;
 
 import java.util.List;
 import java.util.Objects;
@@ -27,12 +28,21 @@ public class HttpSettings {
         Setting.Property.Dynamic
     );
 
+    // The time we wait for a connection to establish
+    public static final Setting<TimeValue> CONNECTION_TIMEOUT = Setting.timeSetting(
+        "xpack.inference.http.connect_timeout",
+        TimeValue.timeValueSeconds(5),
+        Setting.Property.NodeScope
+    );
+
     private volatile ByteSizeValue maxResponseSize;
+    private final int connectionTimeout;
 
     public HttpSettings(Settings settings, ClusterService clusterService) {
         Objects.requireNonNull(clusterService);
         Objects.requireNonNull(settings);
         maxResponseSize = MAX_HTTP_RESPONSE_SIZE.get(settings);
+        connectionTimeout = Math.toIntExact(CONNECTION_TIMEOUT.get(settings).getMillis());
 
         clusterService.getClusterSettings().addSettingsUpdateConsumer(MAX_HTTP_RESPONSE_SIZE, this::setMaxResponseSize);
     }
@@ -41,11 +51,15 @@ public ByteSizeValue getMaxResponseSize() {
         return maxResponseSize;
     }
 
+    public int connectionTimeout() {
+        return connectionTimeout;
+    }
+
     private void setMaxResponseSize(ByteSizeValue maxResponseSize) {
         this.maxResponseSize = maxResponseSize;
     }
 
     public static List<Setting<?>> getSettingsDefinitions() {
-        return List.of(MAX_HTTP_RESPONSE_SIZE);
+        return List.of(MAX_HTTP_RESPONSE_SIZE, CONNECTION_TIMEOUT);
     }
 }

x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/http/HttpClientTests.java

@@ -8,7 +8,6 @@
 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;
@@ -85,15 +84,7 @@ 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(),
-                RequestConfig.DEFAULT
-            )
-        ) {
+        try (var httpClient = HttpClient.create(emptyHttpSettings(), threadPool, createConnectionManager(), mockThrottlerManager())) {
             httpClient.start();
 
             PlainActionFuture<HttpResult> listener = new PlainActionFuture<>();
@@ -111,15 +102,7 @@ public void testSend_MockServerReceivesRequest() throws Exception {
     }
 
     public void testSend_ThrowsErrorIfCalledBeforeStart() throws Exception {
-        try (
-            var httpClient = HttpClient.create(
-                emptyHttpSettings(),
-                threadPool,
-                createConnectionManager(),
-                mockThrottlerManager(),
-                RequestConfig.DEFAULT
-            )
-        ) {
+        try (var httpClient = HttpClient.create(emptyHttpSettings(), threadPool, createConnectionManager(), mockThrottlerManager())) {
             PlainActionFuture<HttpResult> listener = new PlainActionFuture<>();
             var thrownException = expectThrows(
                 AssertionError.class,
@@ -254,15 +237,7 @@ 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(),
-                RequestConfig.DEFAULT
-            )
-        ) {
+        try (var httpClient = HttpClient.create(httpSettings, threadPool, createConnectionManager(), mockThrottlerManager())) {
             httpClient.start();
 
             PlainActionFuture<HttpResult> listener = new PlainActionFuture<>();