diff options
9 files changed, 39 insertions, 43 deletions
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java index b8fe0303cfc..7161fb1be79 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java @@ -56,7 +56,7 @@ public class SetNodeStateRequestTest { } @Test - public void testProbing() throws StateRestApiException { + public void testProbingDoesntChangeState() throws StateRestApiException { probe = true; testSetStateRequest( "maintenance", diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java index e04226c278d..6758d6f94d6 100644 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java +++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java @@ -7,16 +7,16 @@ import java.time.Duration; */ public interface JaxRsTimeouts { /** - * The connect timeout. + * The connect timeout, which must be at least 1ms. * * Throws com.google.common.util.concurrent.UncheckedTimeoutException on timeout. */ - Duration getConnectTimeout(); + Duration getConnectTimeoutOrThrow(); /** - * The read timeout. + * The read timeout, which must be at least 1ms. * * Throws com.google.common.util.concurrent.UncheckedTimeoutException on timeout. */ - Duration getReadTimeout(); + Duration getReadTimeoutOrThrow(); } diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java index 7bc3a450f51..3f2139f6bf0 100644 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java +++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java @@ -15,12 +15,12 @@ public class LegacyJaxRsTimeouts implements JaxRsTimeouts { private static final Duration READ_TIMEOUT = Duration.ofSeconds(30); @Override - public Duration getConnectTimeout() { + public Duration getConnectTimeoutOrThrow() { return CONNECT_TIMEOUT; } @Override - public Duration getReadTimeout() { + public Duration getReadTimeoutOrThrow() { return READ_TIMEOUT; } } 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 b66d9ff6f66..4c97147d61e 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 @@ -79,8 +79,8 @@ public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> { for (final HostName hostName : hostNames) { URI uri = UriBuilder.fromPath(pathPrefix).port(port).scheme(scheme).host(hostName.s()).build(); JaxRsClientFactory.Params<T> params = new JaxRsClientFactory.Params<>(apiClass, uri); - params.setConnectTimeout(timeouts.getConnectTimeout()); - params.setReadTimeout(timeouts.getReadTimeout()); + params.setConnectTimeout(timeouts.getConnectTimeoutOrThrow()); + params.setReadTimeout(timeouts.getReadTimeoutOrThrow()); final T jaxRsClient = jaxRsClientFactory.createClient(params); try { return function.apply(jaxRsClient); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java index 50782ea081f..d7e63ccfc76 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java @@ -42,7 +42,7 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{ return clusterControllerApi.apply(api -> api.setNodeState( clusterName, storageNodeIndex, - timeouts.getServerTimeout().toMillis() / 1000.0f, + timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f, stateRequest), timeouts); } catch (IOException | UncheckedTimeoutException e) { @@ -72,7 +72,7 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{ try { return clusterControllerApi.apply(api -> api.setClusterState( clusterName, - timeouts.getServerTimeout().toMillis() / 1000.0f, + timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f, stateRequest), timeouts); } catch (IOException | UncheckedTimeoutException e) { diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java index 6c08ae53e70..5b0685e88a0 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java @@ -76,12 +76,12 @@ public class ClusterControllerClientTimeouts implements JaxRsTimeouts { } @Override - public Duration getConnectTimeout() { + public Duration getConnectTimeoutOrThrow() { return CONNECT_TIMEOUT; } @Override - public Duration getReadTimeout() { + public Duration getReadTimeoutOrThrow() { Duration timeLeft = timeBudget.timeLeft().get(); if (timeLeft.toMillis() <= 0) { throw new UncheckedTimeoutException("Exceeded the timeout " + timeBudget.originalTimeout().get() + @@ -89,7 +89,7 @@ public class ClusterControllerClientTimeouts implements JaxRsTimeouts { } Duration clientTimeout = min(timeLeft, maxClientTimeout); - verifyPositive(timeLeft, maxClientTimeout); + verifyPositive(timeLeft, clientTimeout); // clientTimeout = overheadPerCall + connectTimeout + readTimeout Duration readTimeout = clientTimeout.minus(IN_PROCESS_OVERHEAD_PER_CALL).minus(CONNECT_TIMEOUT); @@ -98,9 +98,9 @@ public class ClusterControllerClientTimeouts implements JaxRsTimeouts { return readTimeout; } - public Duration getServerTimeout() { + public Duration getServerTimeoutOrThrow() { // readTimeout = networkOverhead + serverTimeout - Duration serverTimeout = getReadTimeout().minus(NETWORK_OVERHEAD_PER_CALL); + Duration serverTimeout = getReadTimeoutOrThrow().minus(NETWORK_OVERHEAD_PER_CALL); if (serverTimeout.toMillis() < MIN_SERVER_TIMEOUT.toMillis()) { throw new UncheckedTimeoutException("Server would be given too little time to complete: " + serverTimeout + ". Original timeout was " + timeBudget.originalTimeout().get()); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java index 768e5290412..76adef72b2b 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java @@ -55,7 +55,7 @@ public interface StatusService { */ MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( ApplicationInstanceReference applicationInstanceReference, - Duration timeoutSeconds); + Duration timeout); /** * Returns all application instances that are allowed to be down. The intention is to use this diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java index 91909391fe7..b12cd5aa7be 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java @@ -32,7 +32,7 @@ public class ClusterControllerClientTest { OrchestratorContext context = mock(OrchestratorContext.class); ClusterControllerClientTimeouts timeouts = mock(ClusterControllerClientTimeouts.class); when(context.getClusterControllerTimeouts(any())).thenReturn(timeouts); - when(timeouts.getServerTimeout()).thenReturn(Duration.ofSeconds(1)); + when(timeouts.getServerTimeoutOrThrow()).thenReturn(Duration.ofSeconds(1)); clusterControllerClient.setNodeState(context, STORAGE_NODE_INDEX, wantedState); final ClusterControllerStateRequest expectedNodeStateRequest = new ClusterControllerStateRequest( diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java index 36dd4c4a83f..9fe65447ac1 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java @@ -55,24 +55,20 @@ public class ClusterControllerClientTimeoutsTest { @Test public void makes2RequestsWithMaxProcessingTime() { - assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeout()); - assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeout()); - assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeout()); + assertStandardTimeouts(); Duration maxProcessingTime = IN_PROCESS_OVERHEAD_PER_CALL .plus(CONNECT_TIMEOUT) - .plus(timeouts.getReadTimeout()); + .plus(timeouts.getReadTimeoutOrThrow()); assertEquals(1450, maxProcessingTime.toMillis()); clock.advance(maxProcessingTime); - assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeout()); - assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeout()); - assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeout()); + assertStandardTimeouts(); clock.advance(maxProcessingTime); try { - timeouts.getServerTimeout(); + timeouts.getServerTimeoutOrThrow(); fail(); } catch (UncheckedTimeoutException e) { assertEquals( @@ -83,22 +79,22 @@ public class ClusterControllerClientTimeoutsTest { @Test public void makesAtLeast3RequestsWithShortProcessingTime() { - assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeout()); - assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeout()); - assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeout()); + assertStandardTimeouts(); - Duration shortPocessingTime = Duration.ofMillis(200); - clock.advance(shortPocessingTime); + Duration shortProcessingTime = Duration.ofMillis(200); + clock.advance(shortProcessingTime); - assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeout()); - assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeout()); - assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeout()); + assertStandardTimeouts(); - clock.advance(shortPocessingTime); + clock.advance(shortProcessingTime); - assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeout()); - assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeout()); - assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeout()); + assertStandardTimeouts(); + } + + private void assertStandardTimeouts() { + assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeoutOrThrow()); + assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeoutOrThrow()); } @Test @@ -106,7 +102,7 @@ public class ClusterControllerClientTimeoutsTest { clock.advance(Duration.ofSeconds(4)); try { - timeouts.getServerTimeout(); + timeouts.getServerTimeoutOrThrow(); fail(); } catch (UncheckedTimeoutException e) { assertEquals( @@ -119,7 +115,7 @@ public class ClusterControllerClientTimeoutsTest { public void justTooLittleTime() { clock.advance(originalTimeout.minus(MINIMUM_TIME_LEFT).plus(Duration.ofMillis(1))); try { - timeouts.getServerTimeout(); + timeouts.getServerTimeoutOrThrow(); fail(); } catch (UncheckedTimeoutException e) { assertEquals( @@ -131,14 +127,14 @@ public class ClusterControllerClientTimeoutsTest { @Test public void justEnoughTime() { clock.advance(originalTimeout.minus(MINIMUM_TIME_LEFT)); - timeouts.getServerTimeout(); + timeouts.getServerTimeoutOrThrow(); } @Test public void justTooLittleInitialTime() { makeTimeouts(MINIMUM_ORIGINAL_TIMEOUT.minus(Duration.ofMillis(1))); try { - timeouts.getServerTimeout(); + timeouts.getServerTimeoutOrThrow(); fail(); } catch (UncheckedTimeoutException e) { assertEquals( @@ -150,6 +146,6 @@ public class ClusterControllerClientTimeoutsTest { @Test public void justEnoughInitialTime() { makeTimeouts(MINIMUM_ORIGINAL_TIMEOUT); - timeouts.getServerTimeout(); + timeouts.getServerTimeoutOrThrow(); } }
\ No newline at end of file |