Remove support for maxRetryTimeout from low-level REST client

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

@@ -39,16 +39,6 @@ public ResponseException(Response response) throws IOException {
         this.response = response;
     }
 
-    /**
-     * Wrap a {@linkplain ResponseException} with another one with the current
-     * stack trace. This is used during synchronous calls so that the caller
-     * ends up in the stack trace of the exception thrown.
-     */
-    ResponseException(ResponseException e) throws IOException {
-        super(e.getMessage(), e);
-        this.response = e.getResponse();
-    }
-
     static String buildMessage(Response response) throws IOException {
         String message = String.format(Locale.ROOT,
             "method [%s], host [%s], URI [%s], status line [%s]",

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

@@ -42,14 +42,12 @@
 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_MAX_CONN_PER_ROUTE = 10;
     public static final int DEFAULT_MAX_CONN_TOTAL = 30;
 
     private static final Header[] EMPTY_HEADERS = new Header[0];
 
     private final List<Node> nodes;
-    private int maxRetryTimeout = DEFAULT_MAX_RETRY_TIMEOUT_MILLIS;
     private Header[] defaultHeaders = EMPTY_HEADERS;
     private RestClient.FailureListener failureListener;
     private HttpClientConfigCallback httpClientConfigCallback;
@@ -102,20 +100,6 @@ public RestClientBuilder setFailureListener(RestClient.FailureListener failureLi
         return this;
     }
 
-    /**
-     * Sets the maximum timeout (in milliseconds) to honour in case of multiple retries of the same request.
-     * {@link #DEFAULT_MAX_RETRY_TIMEOUT_MILLIS} if not specified.
-     *
-     * @throws IllegalArgumentException if {@code maxRetryTimeoutMillis} is not greater than 0
-     */
-    public RestClientBuilder setMaxRetryTimeoutMillis(int maxRetryTimeoutMillis) {
-        if (maxRetryTimeoutMillis <= 0) {
-            throw new IllegalArgumentException("maxRetryTimeoutMillis must be greater than 0");
-        }
-        this.maxRetryTimeout = maxRetryTimeoutMillis;
-        return this;
-    }
-
     /**
      * Sets the {@link HttpClientConfigCallback} to be used to customize http client configuration
      *
@@ -208,7 +192,7 @@ public CloseableHttpAsyncClient run() {
                 return createHttpClient();
             }
         });
-        RestClient restClient = new RestClient(httpClient, maxRetryTimeout, defaultHeaders, nodes,
+        RestClient restClient = new RestClient(httpClient, defaultHeaders, nodes,
                 pathPrefix, failureListener, nodeSelector, strictDeprecationMode);
         httpClient.start();
         return restClient;

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

@@ -82,14 +82,6 @@ public void testBuild() throws IOException {
             assertNotNull(restClient);
         }
 
-        try {
-            RestClient.builder(new HttpHost("localhost", 9200))
-                    .setMaxRetryTimeoutMillis(randomIntBetween(Integer.MIN_VALUE, 0));
-            fail("should have failed");
-        } catch(IllegalArgumentException e) {
-            assertEquals("maxRetryTimeoutMillis must be greater than 0", e.getMessage());
-        }
-
         try {
             RestClient.builder(new HttpHost("localhost", 9200)).setDefaultHeaders(null);
             fail("should have failed");
@@ -156,12 +148,9 @@ public RequestConfig.Builder customizeRequestConfig(RequestConfig.Builder reques
             builder.setDefaultHeaders(headers);
         }
         if (randomBoolean()) {
-            builder.setMaxRetryTimeoutMillis(randomIntBetween(1, Integer.MAX_VALUE));
-        }
-        if (randomBoolean()) {
-            String pathPrefix = (randomBoolean() ? "/" : "") + randomAsciiOfLengthBetween(2, 5);
+            String pathPrefix = (randomBoolean() ? "/" : "") + randomAsciiLettersOfLengthBetween(2, 5);
             while (pathPrefix.length() < 20 && randomBoolean()) {
-                pathPrefix += "/" + randomAsciiOfLengthBetween(3, 6);
+                pathPrefix += "/" + randomAsciiLettersOfLengthBetween(3, 6);
             }
             builder.setPathPrefix(pathPrefix + (randomBoolean() ? "/" : ""));
         }

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

@@ -47,6 +47,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
@@ -87,25 +88,23 @@ public Future<HttpResponse> answer(InvocationOnMock invocationOnMock) throws Thr
                 final HttpHost httpHost = requestProducer.getTarget();
                 HttpClientContext context = (HttpClientContext) invocationOnMock.getArguments()[2];
                 assertThat(context.getAuthCache().get(httpHost), instanceOf(BasicScheme.class));
-                final FutureCallback<HttpResponse> futureCallback = (FutureCallback<HttpResponse>) invocationOnMock.getArguments()[3];
                 //return the desired status code or exception depending on the path
-                exec.execute(new Runnable() {
+                return exec.submit(new Callable<HttpResponse>() {
                     @Override
-                    public void run() {
+                    public HttpResponse call() throws Exception {
                         if (request.getURI().getPath().equals("/soe")) {
-                            futureCallback.failed(new SocketTimeoutException(httpHost.toString()));
+                            throw new SocketTimeoutException(httpHost.toString());
                         } else if (request.getURI().getPath().equals("/coe")) {
-                            futureCallback.failed(new ConnectTimeoutException(httpHost.toString()));
+                            throw new ConnectTimeoutException(httpHost.toString());
                         } else if (request.getURI().getPath().equals("/ioe")) {
-                            futureCallback.failed(new IOException(httpHost.toString()));
+                            throw new IOException(httpHost.toString());
                         } else {
                             int statusCode = Integer.parseInt(request.getURI().getPath().substring(1));
                             StatusLine statusLine = new BasicStatusLine(new ProtocolVersion("http", 1, 1), statusCode, "");
-                            futureCallback.completed(new BasicHttpResponse(statusLine));
+                            return new BasicHttpResponse(statusLine);
                         }
                     }
                 });
-                return null;
             }
         });
         int numNodes = RandomNumbers.randomIntBetween(getRandom(), 2, 5);
@@ -115,7 +114,7 @@ public void run() {
         }
         nodes = Collections.unmodifiableList(nodes);
         failureListener = new HostsTrackingFailureListener();
-        return new RestClient(httpClient, 10000, new Header[0], nodes, null, failureListener, nodeSelector, false);
+        return new RestClient(httpClient, new Header[0], nodes, null, failureListener, nodeSelector, false);
     }
 
     /**
@@ -182,11 +181,6 @@ public void testRoundRobinRetryErrors() throws IOException {
             restClient.performRequest(new Request(randomHttpMethod(getRandom()), retryEndpoint));
             fail("request should have failed");
         } catch (ResponseException e) {
-            /*
-             * Unwrap the top level failure that was added so the stack trace contains
-             * the caller. It wraps the exception that contains the failed hosts.
-             */
-            e = (ResponseException) e.getCause();
             Set<HttpHost> hostsSet = hostsSet();
             //first request causes all the hosts to be blacklisted, the returned exception holds one suppressed exception each
             failureListener.assertCalled(nodes);
@@ -206,11 +200,6 @@ public void testRoundRobinRetryErrors() throws IOException {
             } while(e != null);
             assertEquals("every host should have been used but some weren't: " + hostsSet, 0, hostsSet.size());
         } catch (IOException e) {
-            /*
-             * Unwrap the top level failure that was added so the stack trace contains
-             * the caller. It wraps the exception that contains the failed hosts.
-             */
-            e = (IOException) e.getCause();
             Set<HttpHost> hostsSet = hostsSet();
             //first request causes all the hosts to be blacklisted, the returned exception holds one suppressed exception each
             failureListener.assertCalled(nodes);
@@ -247,11 +236,6 @@ public void testRoundRobinRetryErrors() throws IOException {
                     failureListener.assertCalled(response.getHost());
                     assertEquals(0, e.getSuppressed().length);
                 } catch (IOException e) {
-                    /*
-                     * Unwrap the top level failure that was added so the stack trace contains
-                     * the caller. It wraps the exception that contains the failed hosts.
-                     */
-                    e = (IOException) e.getCause();
                     HttpHost httpHost = HttpHost.create(e.getMessage());
                     assertTrue("host [" + httpHost + "] not found, most likely used multiple times", hostsSet.remove(httpHost));
                     //after the first request, all hosts are blacklisted, a single one gets resurrected each time
@@ -293,11 +277,6 @@ public void testRoundRobinRetryErrors() throws IOException {
                         assertThat(response.getHost(), equalTo(selectedHost));
                         failureListener.assertCalled(selectedHost);
                     } catch(IOException e) {
-                        /*
-                         * Unwrap the top level failure that was added so the stack trace contains
-                         * the caller. It wraps the exception that contains the failed hosts.
-                         */
-                        e = (IOException) e.getCause();
                         HttpHost httpHost = HttpHost.create(e.getMessage());
                         assertThat(httpHost, equalTo(selectedHost));
                         failureListener.assertCalled(selectedHost);