diff options
4 files changed, 43 insertions, 15 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); } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java index c7aae6ea93d..5571eedeec6 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java @@ -41,7 +41,13 @@ public class RetryingClusterControllerClientFactory implements ClusterController Set<HostName> clusterControllerSet = clusterControllers.stream().collect(Collectors.toSet()); JaxRsStrategy<ClusterControllerJaxRsApi> jaxRsApi = new JaxRsStrategyFactory(clusterControllerSet, HARDCODED_CLUSTERCONTROLLER_PORT, jaxRsClientFactory, CLUSTERCONTROLLER_SCHEME) - .apiWithRetries(ClusterControllerJaxRsApi.class, CLUSTERCONTROLLER_API_PATH); + .apiWithRetries(ClusterControllerJaxRsApi.class, CLUSTERCONTROLLER_API_PATH) + // Use max iteration 1. The JaxRsStrategyFactory will try host 1, 2, then 3: + // - If host 1 responds, it will redirect to master if necessary. Otherwise + // - If host 2 responds, it will redirect to master if necessary. Otherwise + // - If host 3 responds, it may redirect to master if necessary (if they're up + // after all), but more likely there's no quorum and this will fail too. + .setMaxIterations(1); return new ClusterControllerClientImpl(jaxRsApi, clusterName); } |