Avoid setting connection request timeout (#30384)

We've been setting this value to 500ms in the default low-level REST
client configuration, misunderstanding the effect that it would have.
This proved very problematic, as it ends up causing `TimeoutException`
returned from the leased pool in some cases even for successful requests.

Closes #24069

client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java

@@ -43,7 +43,6 @@ public final class RestClientBuilder {
     public static final int DEFAULT_CONNECT_TIMEOUT_MILLIS = 1000;
     public static final int DEFAULT_SOCKET_TIMEOUT_MILLIS = 30000;
     public static final int DEFAULT_MAX_RETRY_TIMEOUT_MILLIS = DEFAULT_SOCKET_TIMEOUT_MILLIS;
-    public static final int DEFAULT_CONNECTION_REQUEST_TIMEOUT_MILLIS = 500;
     public static final int DEFAULT_MAX_CONN_PER_ROUTE = 10;
     public static final int DEFAULT_MAX_CONN_TOTAL = 30;
 
@@ -196,8 +195,7 @@ private CloseableHttpAsyncClient createHttpClient() {
         //default timeouts are all infinite
         RequestConfig.Builder requestConfigBuilder = RequestConfig.custom()
                 .setConnectTimeout(DEFAULT_CONNECT_TIMEOUT_MILLIS)
-                .setSocketTimeout(DEFAULT_SOCKET_TIMEOUT_MILLIS)
-                .setConnectionRequestTimeout(DEFAULT_CONNECTION_REQUEST_TIMEOUT_MILLIS);
+                .setSocketTimeout(DEFAULT_SOCKET_TIMEOUT_MILLIS);
         if (requestConfigCallback != null) {
             requestConfigBuilder = requestConfigCallback.customizeRequestConfig(requestConfigBuilder);
         }

client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java

@@ -177,4 +177,24 @@ private static void assertSetPathPrefixThrows(final String pathPrefix) {
         }
     }
 
+    /**
+     * This test verifies that we don't change the default value for the connection request timeout as that causes problems.
+     * See https://github.com/elastic/elasticsearch/issues/24069
+     */
+    public void testDefaultConnectionRequestTimeout() throws IOException {
+        RestClientBuilder builder = RestClient.builder(new HttpHost("localhost", 9200));
+        builder.setRequestConfigCallback(new RestClientBuilder.RequestConfigCallback() {
+            @Override
+            public RequestConfig.Builder customizeRequestConfig(RequestConfig.Builder requestConfigBuilder) {
+                RequestConfig requestConfig = requestConfigBuilder.build();
+                assertEquals(RequestConfig.DEFAULT.getConnectionRequestTimeout(), requestConfig.getConnectionRequestTimeout());
+                //this way we get notified if the default ever changes
+                assertEquals(-1, requestConfig.getConnectionRequestTimeout());
+                return requestConfigBuilder;
+            }
+        });
+        try (RestClient restClient = builder.build()) {
+            assertNotNull(restClient);
+        }
+    }
 }

client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java

@@ -25,20 +25,20 @@
 import com.sun.net.httpserver.HttpServer;
 import org.apache.http.Consts;
 import org.apache.http.Header;
+import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
 import org.apache.http.auth.AuthScope;
 import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.BasicCredentialsProvider;
 import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
+import org.apache.http.nio.entity.NStringEntity;
 import org.apache.http.util.EntityUtils;
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
 import org.elasticsearch.mocksocket.MockHttpServer;
 import org.junit.After;
-import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.BeforeClass;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -51,6 +51,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.client.RestClientTestUtil.getAllStatusCodes;
 import static org.elasticsearch.client.RestClientTestUtil.getHttpMethods;
@@ -166,6 +169,41 @@ public void stopHttpServers() throws IOException {
         httpServer = null;
     }
 
+    /**
+     * Tests sending a bunch of async requests works well (e.g. no TimeoutException from the leased pool)
+     * See https://github.com/elastic/elasticsearch/issues/24069
+     */
+    public void testManyAsyncRequests() throws Exception {
+        int iters = randomIntBetween(500, 1000);
+        final CountDownLatch latch = new CountDownLatch(iters);
+        final List<Exception> exceptions = new CopyOnWriteArrayList<>();
+        for (int i = 0; i < iters; i++) {
+            HttpEntity entity = new NStringEntity("{}", ContentType.APPLICATION_JSON);
+            restClient.performRequestAsync("PUT", "/200", Collections.<String, String>emptyMap(), entity, new ResponseListener() {
+                @Override
+                public void onSuccess(Response response) {
+                    latch.countDown();
+                }
+
+                @Override
+                public void onFailure(Exception exception) {
+                    exceptions.add(exception);
+                    latch.countDown();
+                }
+            });
+        }
+
+        assertTrue("timeout waiting for requests to be sent", latch.await(10, TimeUnit.SECONDS));
+        if (exceptions.isEmpty() == false) {
+            AssertionError error = new AssertionError("expected no failures but got some. see suppressed for first 10 of ["
+                                        + exceptions.size() + "] failures");
+            for (Exception exception : exceptions.subList(0, Math.min(10, exceptions.size()))) {
+                error.addSuppressed(exception);
+            }
+            throw error;
+        }
+    }
+
     /**
      * End to end test for headers. We test it explicitly against a real http client as there are different ways
      * to set/add headers to the {@link org.apache.http.client.HttpClient}.