Fixes after review round

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