diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-10-26 09:44:35 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-10-26 09:44:35 +0200 |
commit | 67878e49f9442d43d42d35f0ebbb57735ad2edbf (patch) | |
tree | 5becab9bd4740bf63256d57bf9bb6370b6863140 /orchestrator | |
parent | 0ccd403b2eed0476a17548a55107bb036567c3f0 (diff) |
Fixes after review round
Diffstat (limited to 'orchestrator')
5 files changed, 30 insertions, 34 deletions
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 |