Avoid set-node-state retry

Today, the Orchestrator will call each cluster controller twice, e.g. indices
1, 2, 0, 1, 2, 0, if each time out.

This is unnecessary. The minimum number of calls is 2:
 - Either the first CC is up and will redirect to master if necessary, or
 - the second is up and will redirect to master if necessary, or
 - the third won't have quorum.

This PR changes the current strategy to call all CCs once, e.g. indices 1, 2,
and 0.

3 files changed, 36 insertions, 14 deletions

diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java
index 6523a0c138f..1459024767d 100644
--- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java
@@ -53,7 +53,7 @@ public class JaxRsStrategyFactory {
         this.scheme = scheme;
     }
 
-    public <T> JaxRsStrategy<T> apiWithRetries(final Class<T> apiClass, final String pathPrefix) {
+    public <T> RetryingJaxRsStrategy<T> apiWithRetries(final Class<T> apiClass, final String pathPrefix) {
         Objects.requireNonNull(apiClass, "apiClass argument may not be null");
         Objects.requireNonNull(pathPrefix, "pathPrefix argument may not be null");
         return new RetryingJaxRsStrategy<T>(hostNames, port, jaxRsClientFactory, apiClass, pathPrefix, scheme);
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java
index 73320a4c72d..65b302ef4ff 100644
--- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java
@@ -21,7 +21,6 @@ import java.util.logging.Logger;
  */
 public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> {
     private static final Logger logger = Logger.getLogger(RetryingJaxRsStrategy.class.getName());
-    private static final int NUM_LOOP_ATTEMPTS = 2;
 
     private final List<HostName> hostNames;
     private final int port;
@@ -30,6 +29,8 @@ public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> {
     private String pathPrefix;
     private final String scheme;
 
+    private int maxIterations = 2;
+
     public RetryingJaxRsStrategy(
             final Set<HostName> hostNames,
             final int port,
@@ -52,11 +53,21 @@ public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> {
         this.scheme = scheme;
     }
 
+    /**
+     * The the max number of times the hostnames should be iterated over, before giving up.
+     *
+     * <p>By default, maxIterations is 2.
+     */
+    public RetryingJaxRsStrategy<T> setMaxIterations(int maxIterations) {
+        this.maxIterations = maxIterations;
+        return this;
+    }
+
     @Override
     public <R> R apply(final Function<T, R> function) throws IOException {
         ProcessingException sampleException = null;
 
-        for (int i = 0; i < NUM_LOOP_ATTEMPTS; ++i) {
+        for (int i = 0; i < maxIterations; ++i) {
             for (final HostName hostName : hostNames) {
                 final T jaxRsClient = jaxRsClientFactory.createClient(apiClass, hostName, port, pathPrefix, scheme);
                 try {
@@ -72,7 +83,7 @@ public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> {
 
         final String message = String.format(
                 "Giving up invoking REST API after %d tries against hosts %s.%s",
-                NUM_LOOP_ATTEMPTS,
+                maxIterations,
                 hostNames,
                 sampleException == null ? "" : ", sample error: " + sampleException.getMessage());
 
diff --git a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java
index 10dde1ff820..e31920febd6 100644
--- a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java
+++ b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java
@@ -6,6 +6,7 @@ import com.yahoo.vespa.defaults.Defaults;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.stubbing.OngoingStubbing;
 
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
@@ -47,7 +48,7 @@ public class RetryingJaxRsStrategyTest {
 
     private final JaxRsClientFactory jaxRsClientFactory = mock(JaxRsClientFactory.class);
     private final TestJaxRsApi mockApi = mock(TestJaxRsApi.class);
-    private final JaxRsStrategy<TestJaxRsApi> jaxRsStrategy = new RetryingJaxRsStrategy<>(
+    private final RetryingJaxRsStrategy<TestJaxRsApi> jaxRsStrategy = new RetryingJaxRsStrategy<>(
             SERVER_HOSTS, REST_PORT, jaxRsClientFactory, TestJaxRsApi.class, API_PATH, "http");
 
     @Before
@@ -111,14 +112,24 @@ public class RetryingJaxRsStrategyTest {
     }
 
     @Test
-    public void testRetryGivesUpAfterTwoLoopsOverAvailableServers() throws Exception {
-        when(mockApi.doSomething())
-                .thenThrow(new ProcessingException("Fake timeout 1 induced by test"))
-                .thenThrow(new ProcessingException("Fake timeout 2 induced by test"))
-                .thenThrow(new ProcessingException("Fake timeout 3 induced by test"))
-                .thenThrow(new ProcessingException("Fake timeout 4 induced by test"))
-                .thenThrow(new ProcessingException("Fake timeout 5 induced by test"))
-                .thenThrow(new ProcessingException("Fake timeout 6 induced by test"));
+    public void testRetryGivesUpAfterOneLoopOverAvailableServers() {
+        jaxRsStrategy.setMaxIterations(1);
+        testRetryGivesUpAfterXIterations(1);
+    }
+
+    @Test
+    public void testRetryGivesUpAfterTwoLoopsOverAvailableServers() {
+        testRetryGivesUpAfterXIterations(2);
+    }
+
+    private void testRetryGivesUpAfterXIterations(int iterations) {
+        OngoingStubbing<String> stub = when(mockApi.doSomething());
+        for (int i = 0; i < iterations; ++i) {
+            stub = stub
+                    .thenThrow(new ProcessingException("Fake timeout 1 iteration " + i))
+                    .thenThrow(new ProcessingException("Fake timeout 2 iteration " + i))
+                    .thenThrow(new ProcessingException("Fake timeout 3 iteration " + i));
+        }
 
         try {
             jaxRsStrategy.apply(TestJaxRsApi::doSomething);
@@ -127,7 +138,7 @@ public class RetryingJaxRsStrategyTest {
             // As expected.
         }
 
-        verify(mockApi, times(6)).doSomething();
+        verify(mockApi, times(iterations * 3)).doSomething();
         verifyAllServersContacted(jaxRsClientFactory);
     }