Revert "Move TimeBudget to vespajlib and use Clock"

8 files changed, 36 insertions, 49 deletions

diff --git a/orchestrator/pom.xml b/orchestrator/pom.xml
index ae05a1908c9..3559e4282c3 100644
--- a/orchestrator/pom.xml
+++ b/orchestrator/pom.xml
@@ -42,12 +42,6 @@
     </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 880eab0c755..9e605561e7b 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
@@ -1,10 +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.google.common.util.concurrent.UncheckedTimeoutException;
-import com.yahoo.time.TimeBudget;
+import com.yahoo.jdisc.TimeBudget;
+import com.yahoo.jdisc.Timer;
 
-import java.time.Clock;
 import java.time.Duration;
 import java.util.Optional;
 
@@ -15,11 +14,12 @@ 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(Clock clock) {
-        this.timeBudget = TimeBudget.fromNow(clock, DEFAULT_TIMEOUT);
+    public OrchestratorContext(Timer timer) {
+        this.timeBudget = TimeBudget.fromNow(timer, DEFAULT_TIMEOUT);
     }
 
     /** Get the original timeout in seconds. */
@@ -28,10 +28,17 @@ public class OrchestratorContext {
     }
 
     /**
-     * Get number of seconds until the deadline, or empty if there's no deadline, or throw
-     * an {@link UncheckedTimeoutException} if timed out.
+     * 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.
      */
     public Optional<Float> getSuboperationTimeoutInSeconds() {
-        return Optional.of((float) (timeBudget.timeLeftOrThrow().toMillis() / 1000.0));
+        return getSuboperationTimeoutInSeconds(POST_OPERATION_HEADROOM);
+    }
+
+    private Optional<Float> getSuboperationTimeoutInSeconds(Duration headroom) {
+        return Optional.of((float) (timeBudget.timeBeforeDeadline(headroom).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 ad8a35312e4..580d34eccf8 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,7 +31,6 @@ 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;
@@ -52,20 +51,21 @@ public class OrchestratorImpl implements Orchestrator {
     private final InstanceLookupService instanceLookupService;
     private final int serviceMonitorConvergenceLatencySeconds;
     private final ClusterControllerClientFactory clusterControllerClientFactory;
-    private final Clock clock;
+    private final Timer timer;
 
     @Inject
     public OrchestratorImpl(ClusterControllerClientFactory clusterControllerClientFactory,
                             StatusService statusService,
                             OrchestratorConfig orchestratorConfig,
-                            InstanceLookupService instanceLookupService)
+                            InstanceLookupService instanceLookupService,
+                            Timer timer)
     {
         this(new HostedVespaPolicy(new HostedVespaClusterPolicy(), clusterControllerClientFactory),
                 clusterControllerClientFactory,
                 statusService,
                 instanceLookupService,
                 orchestratorConfig.serviceMonitorConvergenceLatencySeconds(),
-                Clock.systemUTC());
+                timer);
     }
 
     public OrchestratorImpl(Policy policy,
@@ -73,14 +73,14 @@ public class OrchestratorImpl implements Orchestrator {
                             StatusService statusService,
                             InstanceLookupService instanceLookupService,
                             int serviceMonitorConvergenceLatencySeconds,
-                            Clock clock)
+                            Timer timer)
     {
         this.policy = policy;
         this.clusterControllerClientFactory = clusterControllerClientFactory;
         this.statusService = statusService;
         this.serviceMonitorConvergenceLatencySeconds = serviceMonitorConvergenceLatencySeconds;
         this.instanceLookupService = instanceLookupService;
-        this.clock = clock;
+        this.timer = timer;
     }
 
     @Override
@@ -127,7 +127,7 @@ public class OrchestratorImpl implements Orchestrator {
 
         ApplicationInstance appInstance = getApplicationInstance(hostName);
 
-        OrchestratorContext context = new OrchestratorContext(clock);
+        OrchestratorContext context = new OrchestratorContext(timer);
         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(clock);
+        OrchestratorContext context = new OrchestratorContext(timer);
         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(clock);
+        OrchestratorContext context = new OrchestratorContext(timer);
         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(clock);
+        OrchestratorContext context = new OrchestratorContext(timer);
         ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
         try (MutableStatusRegistry statusRegistry =
                      statusService.lockApplicationInstance_forCurrentThreadOnly(
@@ -355,10 +355,6 @@ 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 15ae69b3a0d..c2559bdd0da 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,7 +1,6 @@
 // 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.google.common.util.concurrent.UncheckedTimeoutException;
 import com.yahoo.vespa.orchestrator.OrchestratorContext;
 
 import java.io.IOException;
@@ -15,7 +14,6 @@ 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;
 
@@ -23,7 +21,6 @@ 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 3e387012d2c..a2732bca88a 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,7 +1,6 @@
 // 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;
@@ -75,13 +74,6 @@ 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 a781fd2358a..e1664466283 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,7 +28,6 @@ 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 76d9398c44e..c3b1ceb66ec 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -2,6 +2,7 @@
 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;
@@ -77,7 +78,8 @@ public class OrchestratorImplTest {
                 clustercontroller,
                 new InMemoryStatusService(),
                 new OrchestratorConfig(new OrchestratorConfig.Builder()),
-                new DummyInstanceLookupService());
+                new DummyInstanceLookupService(),
+                mock(Timer.class));
 
         clustercontroller.setAllDummyNodesAsUp();
     }
@@ -307,7 +309,8 @@ public class OrchestratorImplTest {
                 clusterControllerClientFactory,
                 statusService,
                 new OrchestratorConfig(new OrchestratorConfig.Builder()),
-                lookupService);
+                lookupService,
+                mock(Timer.class));
 
         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 45ba862c8f1..49f1a33febb 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,7 +40,6 @@ 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;
@@ -62,7 +61,7 @@ import static org.mockito.Mockito.when;
  * @author hakonhall
  */
 public class HostResourceTest {
-    private static final Clock clock = mock(Clock.class);
+    private static final Timer timer = mock(Timer.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");
@@ -137,7 +136,7 @@ public class HostResourceTest {
             new ClusterControllerClientFactoryMock(),
             EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, mockInstanceLookupService,
             SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-            clock
+            timer
     );
 
     private static final OrchestratorImpl hostNotFoundOrchestrator = new OrchestratorImpl(
@@ -145,7 +144,7 @@ public class HostResourceTest {
             new ClusterControllerClientFactoryMock(),
             EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, alwaysEmptyInstanceLookUpService,
             SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-            clock
+            timer
     );
 
     private final UriInfo uriInfo = mock(UriInfo.class);
@@ -243,7 +242,7 @@ public class HostResourceTest {
                 new ClusterControllerClientFactoryMock(),
                 EVERY_HOST_IS_UP_HOST_STATUS_SERVICE,mockInstanceLookupService,
                 SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-                clock);
+                timer);
 
         try {
             HostResource hostResource = new HostResource(alwaysRejectResolver, uriInfo);
@@ -262,7 +261,7 @@ public class HostResourceTest {
                 EVERY_HOST_IS_UP_HOST_STATUS_SERVICE,
                 mockInstanceLookupService,
                 SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS,
-                clock);
+                timer);
 
         try {
             HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysRejectResolver);