AbstractClient: New retry logic (#192)

* AbstractClient: Extract method for timeout integrity checks

* AbstractClient: New retry logic

* IndexTest: Test host status retry logic

* IndexTest: Fix obsolete or incomplete comments

* IndexTest.hostStatus: Use randomUUID instead of String from random bytes

* IndexTest.hostStatus: explicit method name

* IndexTest.hostStatus: remove useless statuses

* HostStatus: Simplify class

* IndexTest: Update usage of searchAsync

Author: PLNech

algoliasearch/src/main/java/com/algolia/search/saas/AbstractClient.java

@@ -45,6 +45,7 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -87,6 +88,16 @@ public int hashCode() {
         }
     }
 
+    private static class HostStatus {
+        boolean isUp = true;
+        long lastTryTimestamp;
+
+        HostStatus(boolean isUp) {
+            this.isUp = isUp;
+            lastTryTimestamp = new Date().getTime();
+        }
+    }
+
     // ----------------------------------------------------------------------
     // Constants
     // ----------------------------------------------------------------------
@@ -116,10 +127,14 @@ public int hashCode() {
     /** Read timeout for search requests (ms). */
     private int searchTimeout = 5000;
 
+    /** Delay to wait when a host is down before retrying it (ms). */
+    private int hostDownDelay = 5000;
+
     private final String applicationID;
     private final String apiKey;
     private List<String> readHosts;
     private List<String> writeHosts;
+    private HashMap<String, HostStatus> hostStatuses = new HashMap<>();
 
     /**
      * HTTP headers that will be sent with every request.
@@ -235,8 +250,7 @@ public int getConnectTimeout() {
      * @param connectTimeout The new connection timeout (ms).
      */
     public void setConnectTimeout(int connectTimeout) {
-        if (connectTimeout <= 0)
-            throw new IllegalArgumentException();
+        checkTimeout(connectTimeout);
         this.connectTimeout = connectTimeout;
     }
 
@@ -255,8 +269,7 @@ public int getReadTimeout() {
      * @param readTimeout The default read timeout (ms).
      */
     public void setReadTimeout(int readTimeout) {
-        if (readTimeout <= 0)
-            throw new IllegalArgumentException();
+        checkTimeout(readTimeout);
         this.readTimeout = readTimeout;
     }
 
@@ -275,11 +288,29 @@ public int getSearchTimeout() {
      * @param searchTimeout The read timeout for search requests (ms).
      */
     public void setSearchTimeout(int searchTimeout) {
-        if (searchTimeout <= 0)
-            throw  new IllegalArgumentException();
+        checkTimeout(searchTimeout);
         this.searchTimeout = searchTimeout;
     }
 
+    /**
+     * Get the timeout for retrying connection to a down host.
+     *
+     * @return The delay before connecting again to a down host (ms).
+     */
+    public int getHostDownDelay() {
+        return hostDownDelay;
+    }
+
+    /**
+     * Set the timeout for retrying connection to a down host.
+     *
+     * @param hostDownDelay The delay before connecting again to a down host (ms).
+     */
+    public void setHostDownDelay(int hostDownDelay) {
+        checkTimeout(hostDownDelay);
+        this.hostDownDelay = hostDownDelay;
+    }
+
     /**
      * Add a software library to the list of user agents.
      *
@@ -333,6 +364,14 @@ private void updateUserAgents() {
         userAgentRaw = s.toString();
     }
 
+    private List<String> getReadHostsThatAreUp() {
+        return hostsThatAreUp(readHosts);
+    }
+
+    private List<String> getWriteHostsThatAreUp() {
+        return hostsThatAreUp(writeHosts);
+    }
+
     // ----------------------------------------------------------------------
     // Utilities
     // ----------------------------------------------------------------------
@@ -345,27 +384,27 @@ private enum Method {
     }
 
     protected byte[] getRequestRaw(String url, boolean search) throws AlgoliaException {
-        return _requestRaw(Method.GET, url, null, readHosts, connectTimeout, search ? searchTimeout : readTimeout);
+        return _requestRaw(Method.GET, url, null, getReadHostsThatAreUp(), connectTimeout, search ? searchTimeout : readTimeout);
     }
 
     protected JSONObject getRequest(String url, boolean search) throws AlgoliaException {
-        return _request(Method.GET, url, null, readHosts, connectTimeout, search ? searchTimeout : readTimeout);
+        return _request(Method.GET, url, null, getReadHostsThatAreUp(), connectTimeout, search ? searchTimeout : readTimeout);
     }
 
     protected JSONObject deleteRequest(String url) throws AlgoliaException {
-        return _request(Method.DELETE, url, null, writeHosts, connectTimeout, readTimeout);
+        return _request(Method.DELETE, url, null, getWriteHostsThatAreUp(), connectTimeout, readTimeout);
     }
 
     protected JSONObject postRequest(String url, String obj, boolean readOperation) throws AlgoliaException {
-        return _request(Method.POST, url, obj, (readOperation ? readHosts : writeHosts), connectTimeout, (readOperation ? searchTimeout : readTimeout));
+        return _request(Method.POST, url, obj, (readOperation ? getReadHostsThatAreUp() : getWriteHostsThatAreUp()), connectTimeout, (readOperation ? searchTimeout : readTimeout));
     }
 
     protected byte[] postRequestRaw(String url, String obj, boolean readOperation) throws AlgoliaException {
-        return _requestRaw(Method.POST, url, obj, (readOperation ? readHosts : writeHosts), connectTimeout, (readOperation ? searchTimeout : readTimeout));
+        return _requestRaw(Method.POST, url, obj, (readOperation ? getReadHostsThatAreUp() : getWriteHostsThatAreUp()), connectTimeout, (readOperation ? searchTimeout : readTimeout));
     }
 
     protected JSONObject putRequest(String url, String obj) throws AlgoliaException {
-        return _request(Method.PUT, url, obj, writeHosts, connectTimeout, readTimeout);
+        return _request(Method.PUT, url, obj, getWriteHostsThatAreUp(), connectTimeout, readTimeout);
     }
 
     /**
@@ -526,6 +565,7 @@ private byte[] _requestRaw(Method m, String url, String json, List<String> hosts
                 if (stream == null) {
                     throw new IOException(String.format("Null stream when reading connection (status %d)", code));
                 }
+                hostStatuses.put(host, new HostStatus(true));
 
                 final byte[] rawResponse;
                 String encoding = hostConnection.getContentEncoding();
@@ -556,8 +596,8 @@ private byte[] _requestRaw(Method m, String url, String json, List<String> hosts
             catch (UnsupportedEncodingException e) { // fatal
                 consumeQuietly(hostConnection);
                 throw new AlgoliaException("Invalid encoding returned by server", e);
-            }
-            catch (IOException e) { // host error, continue on the next host
+            } catch (IOException e) { // host error, continue on the next host
+                hostStatuses.put(host, new HostStatus(false));
                 consumeQuietly(hostConnection);
                 errors.add(e);
             } finally {
@@ -599,6 +639,32 @@ private static void consumeQuietly(final HttpURLConnection connection) {
         }
     }
 
+    private void checkTimeout(int connectTimeout) {
+        if (connectTimeout <= 0) {
+            throw new IllegalArgumentException();
+        }
+    }
+
+    /**
+     * Get the hosts that are not considered down in a given list.
+     * @param hosts a list of hosts whose {@link HostStatus} will be checked.
+     * @return the hosts considered up, or all hosts if none is known to be reachable.
+     */
+    private List<String> hostsThatAreUp(List<String> hosts) {
+        List<String> upHosts = new ArrayList<>();
+        for (String host : hosts) {
+            if (isUpOrCouldBeRetried(host)) {
+                upHosts.add(host);
+            }
+        }
+        return upHosts.isEmpty() ? hosts : upHosts;
+    }
+
+    boolean isUpOrCouldBeRetried(String host) {
+        HostStatus status = hostStatuses.get(host);
+        return status == null || status.isUp || new Date().getTime() - status.lastTryTimestamp >= hostDownDelay;
+    }
+
     // ----------------------------------------------------------------------
     // Utils
     // ----------------------------------------------------------------------

algoliasearch/src/test/java/com/algolia/search/saas/IndexTest.java

@@ -43,6 +43,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -61,6 +62,7 @@
  * <a href="http://d.android.com/tools/testing/testing_android.html">Testing Fundamentals</a>
  */
 
+@SuppressWarnings("unchecked") //Whitebox requires casts from Object
 @SuppressLint("DefaultLocale") //We use format for logging errors, locale issues are irrelevant
 public class IndexTest extends RobolectricTestCase {
     Client client;
@@ -1114,4 +1116,47 @@ public void doRequestCompleted(JSONObject content, AlgoliaException error) {
             }
         });
     }
+
+    @Test
+    public void retryUsingHostStatus() throws Exception {
+        List<String> hostsArray = (List<String>) Whitebox.getInternalState(client, "readHosts");
+        String randomHostName = getRandomString() + "-dsn.algolia.biz";
+        final String nextHostName = hostsArray.get(1);
+
+        // Given a first host that timeouts, randomized to ensure no system caching
+        hostsArray.set(0, randomHostName);
+        Whitebox.setInternalState(client, "readHosts", hostsArray);
+
+
+        // Expect reachable hosts before any connection
+        assertTrue("Hosts should be considered up before first connection.", client.isUpOrCouldBeRetried(randomHostName));
+        assertTrue("Hosts should be considered up before first connection.", client.isUpOrCouldBeRetried(nextHostName));
+
+        // Expect success after a failing host
+        searchAsync();
+
+        // Expect down host after failed connection, up host after successful connection
+        assertFalse("A host that has failed recently should be considered down.", client.isUpOrCouldBeRetried(randomHostName));
+        assertTrue("A host that has succeeded recently should be considered up.", client.isUpOrCouldBeRetried(nextHostName));
+
+        hostsArray = (List<String>) Whitebox.getInternalState(client, "readHosts");
+        randomHostName = getRandomString() + "-dsn.algolia.biz";
+
+        // Given a short host delay and a first host that timeouts
+        final int delay = 100;
+        client.setHostDownDelay(delay);
+        hostsArray.set(0, randomHostName);
+        Whitebox.setInternalState(client, "readHosts", hostsArray);
+
+        // Expect success after a failing host
+        searchAsync();
+
+        // Expect host to be up again after the delay has passed
+        Thread.sleep(delay);
+        assertTrue("A host that has failed should be considered up once the delay is over.", client.isUpOrCouldBeRetried(randomHostName));
+    }
+
+    private String getRandomString() {
+        return UUID.randomUUID().toString();
+    }
 }