Move TimeBudget to vespajlib and use Clock

8 files changed, 48 insertions, 36 deletions

diff --git a/orchestrator/pom.xml b/orchestrator/pom.xml
index 3559e4282c3..ae05a1908c9 100644
--- a/orchestrator/pom.xml
+++ b/orchestrator/pom.xml
@@ -42,6 +42,12 @@
     </dependency>
     <dependency>
       <groupId>com.yahoo.vespa</groupId>
+      <artifactId>vespajlib</artifactId>
+      <version>${project.version}</version>
+      <scope>provided</scope>
+    </dependency>
+    <dependency>
+      <groupId>com.yahoo.vespa</groupId>
       <artifactId>jaxrs_client_utils</artifactId>
       <version>${project.version}</version>
       <scope>compile</scope>
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
index 9e605561e7b..695fd51e8e4 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
@@ -1,9 +1,9 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator;
 
-import com.yahoo.jdisc.TimeBudget;
-import com.yahoo.jdisc.Timer;
+import com.yahoo.time.TimeBudget;
 
+import java.time.Clock;
 import java.time.Duration;
 import java.util.Optional;
 
@@ -14,12 +14,11 @@ import java.util.Optional;
  */
 public class OrchestratorContext {
     private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(10);
-    private static final Duration POST_OPERATION_HEADROOM = Duration.ofMillis(100);
 
     private TimeBudget timeBudget;
 
-    public OrchestratorContext(Timer timer) {
-        this.timeBudget = TimeBudget.fromNow(timer, DEFAULT_TIMEOUT);
+    public OrchestratorContext(Clock clock) {
+        this.timeBudget = TimeBudget.fromNow(clock, DEFAULT_TIMEOUT);
     }
 
     /** Get the original timeout in seconds. */
@@ -28,17 +27,10 @@ public class OrchestratorContext {
     }
 
     /**
-     * Get number of seconds until the deadline, or empty if there's no deadline.
-     *
-     * <p>The returned timeout is slightly shorter than the actual timeout to ensure there's
-     * enough time to wrap up and return from the Orchestrator between when the operation
-     * times out and the actual timeout.
+     * Get number of seconds until the deadline, or empty if there's no deadline, or throw
+     * an UncheckedTimeoutException if timed out.
      */
     public Optional<Float> getSuboperationTimeoutInSeconds() {
-        return getSuboperationTimeoutInSeconds(POST_OPERATION_HEADROOM);
-    }
-
-    private Optional<Float> getSuboperationTimeoutInSeconds(Duration headroom) {
-        return Optional.of((float) (timeBudget.timeBeforeDeadline(headroom).toMillis() / 1000.0));
+        return Optional.of((float) (timeBudget.timeLeftOrThrow().toMillis() / 1000.0));
     }
 }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
index 580d34eccf8..ad8a35312e4 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -1,9 +1,9 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator;
 
+import com.google.common.util.concurrent.UncheckedTimeoutException;
 import com.google.inject.Inject;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.jdisc.Timer;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
@@ -31,6 +31,7 @@ import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry;
 import com.yahoo.vespa.orchestrator.status.StatusService;
 
 import java.io.IOException;
+import java.time.Clock;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -51,21 +52,20 @@ public class OrchestratorImpl implements Orchestrator {
     private final InstanceLookupService instanceLookupService;
     private final int serviceMonitorConvergenceLatencySeconds;
     private final ClusterControllerClientFactory clusterControllerClientFactory;
-    private final Timer timer;
+    private final Clock clock;
 
     @Inject
     public OrchestratorImpl(ClusterControllerClientFactory clusterControllerClientFactory,
                             StatusService statusService,
                             OrchestratorConfig orchestratorConfig,
-                            InstanceLookupService instanceLookupService,
-                            Timer timer)
+                            InstanceLookupService instanceLookupService)
     {
         this(new HostedVespaPolicy(new HostedVespaClusterPolicy(), clusterControllerClientFactory),
                 clusterControllerClientFactory,
                 statusService,
                 instanceLookupService,
                 orchestratorConfig.serviceMonitorConvergenceLatencySeconds(),
-                timer);
+                Clock.systemUTC());
     }
 
     public OrchestratorImpl(Policy policy,
@@ -73,14 +73,14 @@ public class OrchestratorImpl implements Orchestrator {
                             StatusService statusService,
                             InstanceLookupService instanceLookupService,
                             int serviceMonitorConvergenceLatencySeconds,
-                            Timer timer)
+                            Clock clock)
     {
         this.policy = policy;
         this.clusterControllerClientFactory = clusterControllerClientFactory;
         this.statusService = statusService;
         this.serviceMonitorConvergenceLatencySeconds = serviceMonitorConvergenceLatencySeconds;
         this.instanceLookupService = instanceLookupService;
-        this.timer = timer;
+        this.clock = clock;
     }
 
     @Override
@@ -127,7 +127,7 @@ public class OrchestratorImpl implements Orchestrator {
 
         ApplicationInstance appInstance = getApplicationInstance(hostName);
 
-        OrchestratorContext context = new OrchestratorContext(timer);
+        OrchestratorContext context = new OrchestratorContext(clock);
         try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(
                 appInstance.reference(),
                 context.getOriginalTimeoutInSeconds())) {
@@ -156,7 +156,7 @@ public class OrchestratorImpl implements Orchestrator {
         ApplicationInstance appInstance = getApplicationInstance(hostName);
         NodeGroup nodeGroup = new NodeGroup(appInstance, hostName);
 
-        OrchestratorContext context = new OrchestratorContext(timer);
+        OrchestratorContext context = new OrchestratorContext(clock);
         try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(
                 appInstance.reference(),
                 context.getOriginalTimeoutInSeconds())) {
@@ -174,7 +174,7 @@ public class OrchestratorImpl implements Orchestrator {
     public void suspendGroup(NodeGroup nodeGroup) throws HostStateChangeDeniedException, HostNameNotFoundException {
         ApplicationInstanceReference applicationReference = nodeGroup.getApplicationReference();
 
-        OrchestratorContext context = new OrchestratorContext(timer);
+        OrchestratorContext context = new OrchestratorContext(clock);
         try (MutableStatusRegistry hostStatusRegistry =
                      statusService.lockApplicationInstance_forCurrentThreadOnly(
                              applicationReference,
@@ -301,7 +301,7 @@ public class OrchestratorImpl implements Orchestrator {
 
     private void setApplicationStatus(ApplicationId appId, ApplicationInstanceStatus status) 
             throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException{
-        OrchestratorContext context = new OrchestratorContext(timer);
+        OrchestratorContext context = new OrchestratorContext(clock);
         ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
         try (MutableStatusRegistry statusRegistry =
                      statusService.lockApplicationInstance_forCurrentThreadOnly(
@@ -355,6 +355,10 @@ public class OrchestratorImpl implements Orchestrator {
                 }
             } catch (IOException e) {
                 throw new ApplicationStateChangeDeniedException(e.getMessage());
+            } catch (UncheckedTimeoutException e) {
+                throw new ApplicationStateChangeDeniedException(
+                        "Timed out while waiting for cluster controllers " + clusterControllers +
+                                " with cluster ID " + clusterId.s() + ": " + e.getMessage());
             }
         }
     }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java
index c2559bdd0da..87907df4db2 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.controller;
 
+import com.yahoo.time.UncheckedTimeoutException;
 import com.yahoo.vespa.orchestrator.OrchestratorContext;
 
 import java.io.IOException;
@@ -14,6 +15,7 @@ public interface ClusterControllerClient {
      * Requests that a cluster controller sets the requested node to the requested state.
      *
      * @throws IOException if there was a problem communicating with the cluster controller
+     * @throws UncheckedTimeoutException if operation times out
      */
     ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException;
 
@@ -21,6 +23,7 @@ public interface ClusterControllerClient {
      * Requests that a cluster controller sets all nodes in the cluster to the requested state.
      *
      * @throws IOException if there was a problem communicating with the cluster controller
+     * @throws UncheckedTimeoutException if operation times out
      */
     ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException;
 
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java
index a2732bca88a..3e387012d2c 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.model;
 
+import com.google.common.util.concurrent.UncheckedTimeoutException;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.ClusterId;
@@ -74,6 +75,13 @@ public class StorageNodeImpl implements StorageNode {
                     HostedVespaPolicy.CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT,
                     "Failed to communicate with cluster controllers " + clusterControllers + ": " + e,
                     e);
+        } catch (UncheckedTimeoutException e) {
+            throw new HostStateChangeDeniedException(
+                    hostName(),
+                    HostedVespaPolicy.DEADLINE_CONSTRAINT,
+                    "Timeout while waiting for setNodeState(" + nodeIndex + ", " + wantedNodeState +
+                            ") against " + clusterControllers + ": " + e.getMessage(),
+                    e);
         }
 
         if ( ! response.wasModified) {
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
index e1664466283..a781fd2358a 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
@@ -28,6 +28,7 @@ public class HostedVespaPolicy implements Policy {
     public static final String ENOUGH_SERVICES_UP_CONSTRAINT = "enough-services-up";
     public static final String SET_NODE_STATE_CONSTRAINT = "controller-set-node-state";
     public static final String CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT = "controller-available";
+    public static final String DEADLINE_CONSTRAINT = "deadline";
 
     private static final Logger log = Logger.getLogger(HostedVespaPolicy.class.getName());
 
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
index c3b1ceb66ec..76d9398c44e 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.orchestrator;
 
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.jdisc.Timer;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceId;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
@@ -78,8 +77,7 @@ public class OrchestratorImplTest {
                 clustercontroller,
                 new InMemoryStatusService(),
                 new OrchestratorConfig(new OrchestratorConfig.Builder()),
-                new DummyInstanceLookupService(),
-                mock(Timer.class));
+                new DummyInstanceLookupService());
 
         clustercontroller.setAllDummyNodesAsUp();
     }
@@ -309,8 +307,7 @@ public class OrchestratorImplTest {
                 clusterControllerClientFactory,
                 statusService,
                 new OrchestratorConfig(new OrchestratorConfig.Builder()),
-                lookupService,
-                mock(Timer.class));
+                lookupService);
 
         HostName hostName = new HostName("host.yahoo.com");
         TenantId tenantId = new TenantId("tenant");
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
index 49f1a33febb..45ba862c8f1 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
@@ -40,6 +40,7 @@ import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 import java.net.URI;
+import java.time.Clock;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
@@ -61,7 +62,7 @@ import static org.mockito.Mockito.when;
  * @author hakonhall
  */
 public class HostResourceTest {
-    private static final Timer timer = mock(Timer.class);
+    private static final Clock clock = mock(Clock.class);
     private static final int SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS = 0;
     private static final TenantId TENANT_ID = new TenantId("tenantId");
     private static final ApplicationInstanceId APPLICATION_INSTANCE_ID = new ApplicationInstanceId("applicationId");
@@ -136,7 +137,7 @@ public class HostResourceTest {
             new ClusterControllerClientFactoryMock(),
             EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, mockInstanceLookupService,
             SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-            timer
+            clock
     );
 
     private static final OrchestratorImpl hostNotFoundOrchestrator = new OrchestratorImpl(
@@ -144,7 +145,7 @@ public class HostResourceTest {
             new ClusterControllerClientFactoryMock(),
             EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, alwaysEmptyInstanceLookUpService,
             SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-            timer
+            clock
     );
 
     private final UriInfo uriInfo = mock(UriInfo.class);
@@ -242,7 +243,7 @@ public class HostResourceTest {
                 new ClusterControllerClientFactoryMock(),
                 EVERY_HOST_IS_UP_HOST_STATUS_SERVICE,mockInstanceLookupService,
                 SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-                timer);
+                clock);
 
         try {
             HostResource hostResource = new HostResource(alwaysRejectResolver, uriInfo);
@@ -261,7 +262,7 @@ public class HostResourceTest {
                 EVERY_HOST_IS_UP_HOST_STATUS_SERVICE,
                 mockInstanceLookupService,
                 SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-                timer);
+                clock);
 
         try {
             HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysRejectResolver);