diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-06-22 11:01:22 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-06-22 11:01:22 +0200 |
commit | da8720586341957f15bf6a42b291d879c8569538 (patch) | |
tree | 29bb114ffbc6bb5bdd4a048e53845df00f449e08 | |
parent | d17e36f062c38550a96ccee3e41d7ff5266efecb (diff) |
set-node-state timeout in CC
17 files changed, 186 insertions, 43 deletions
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java index 6b40577b1b0..56fe679fc6a 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java @@ -882,11 +882,11 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd */ private void scheduleVersionDependentTasksForFutureCompletion(int completeAtVersion) { // TODO expose and use monotonic clock instead of system clock - final long deadlineTimePointMs = timer.getCurrentTimeInMillis() + options.getMaxDeferredTaskVersionWaitTime().toMillis(); + final long maxDeadlineTimePointMs = timer.getCurrentTimeInMillis() + options.getMaxDeferredTaskVersionWaitTime().toMillis(); for (RemoteClusterControllerTask task : tasksPendingStateRecompute) { log.finest(() -> String.format("Adding task of type '%s' to be completed at version %d", task.getClass().getName(), completeAtVersion)); - taskCompletionQueue.add(new VersionDependentTaskCompletion(completeAtVersion, task, deadlineTimePointMs)); + taskCompletionQueue.add(new VersionDependentTaskCompletion(completeAtVersion, task, maxDeadlineTimePointMs)); } tasksPendingStateRecompute.clear(); } diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/RemoteClusterControllerTask.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/RemoteClusterControllerTask.java index e96209c083a..8382e127e13 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/RemoteClusterControllerTask.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/RemoteClusterControllerTask.java @@ -5,6 +5,9 @@ import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vespa.clustercontroller.core.listeners.NodeAddedOrRemovedListener; import com.yahoo.vespa.clustercontroller.core.listeners.NodeStateOrHostInfoChangeHandler; +import java.time.Instant; +import java.util.Optional; + public abstract class RemoteClusterControllerTask { public static class Context { @@ -65,6 +68,10 @@ public abstract class RemoteClusterControllerTask { */ public void handleFailure(FailureCondition condition) {} + public Optional<Instant> getDeadline() { + return Optional.empty(); + } + public boolean isCompleted() { synchronized (monitor) { return completed; diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/VersionDependentTaskCompletion.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/VersionDependentTaskCompletion.java index 5d6a4f66467..28df5a8e35a 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/VersionDependentTaskCompletion.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/VersionDependentTaskCompletion.java @@ -16,10 +16,12 @@ class VersionDependentTaskCompletion { private final RemoteClusterControllerTask task; private final long deadlineTimePointMs; - VersionDependentTaskCompletion(long minimumVersion, RemoteClusterControllerTask task, long deadlineTimePointMs) { + VersionDependentTaskCompletion(long minimumVersion, RemoteClusterControllerTask task, long maxDeadlineTimePointMs) { this.minimumVersion = minimumVersion; this.task = task; - this.deadlineTimePointMs = deadlineTimePointMs; + this.deadlineTimePointMs = task.getDeadline().map(deadline -> + Math.max(0, Math.min(deadline.toEpochMilli(), maxDeadlineTimePointMs))) + .orElse(maxDeadlineTimePointMs); } long getMinimumVersion() { @@ -30,7 +32,9 @@ class VersionDependentTaskCompletion { return task; } - long getDeadlineTimePointMs() { return deadlineTimePointMs; } + long getDeadlineTimePointMs() { + return deadlineTimePointMs; + } @Override public boolean equals(Object o) { diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java index 849d8cc6e7b..4d6738940a8 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.clustercontroller.core.restapiv2.requests; import com.yahoo.log.LogLevel; +import com.yahoo.time.TimeBudget; import com.yahoo.vdslib.state.ClusterState; import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeState; @@ -21,8 +22,10 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStat import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; +import java.time.Instant; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.logging.Logger; public class SetNodeStateRequest extends Request<SetResponse> { @@ -33,6 +36,7 @@ public class SetNodeStateRequest extends Request<SetResponse> { private final SetUnitStateRequest.Condition condition; private final SetUnitStateRequest.ResponseWait responseWait; private final WantedStateSetter wantedState; + private final TimeBudget timeBudget; public SetNodeStateRequest(Id.Node id, SetUnitStateRequest setUnitStateRequest) { this(id, setUnitStateRequest, SetNodeStateRequest::setWantedState); @@ -46,6 +50,7 @@ public class SetNodeStateRequest extends Request<SetResponse> { this.condition = setUnitStateRequest.getCondition(); this.responseWait = setUnitStateRequest.getResponseWait(); this.wantedState = wantedState; + this.timeBudget = setUnitStateRequest.timeBudget(); } @Override @@ -79,6 +84,11 @@ public class SetNodeStateRequest extends Request<SetResponse> { } @Override + public Optional<Instant> getDeadline() { + return timeBudget.deadline(); + } + + @Override public boolean isFailed() { // Failure to set a node state is propagated as a 200 with wasModified false. return super.isFailed() || (resultSet && !result.getWasModified()); diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java index 1246ba62313..b4d189bcd55 100644 --- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java +++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.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.clustercontroller.core.restapiv2.requests; +import com.yahoo.time.TimeBudget; import com.yahoo.vdslib.distribution.ConfiguredNode; import com.yahoo.vdslib.state.Node; import com.yahoo.vdslib.state.NodeType; @@ -14,7 +15,9 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStat import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; +import java.time.Instant; import java.util.Map; +import java.util.Optional; import java.util.logging.Logger; public class SetNodeStatesForClusterRequest extends Request<SetResponse> { @@ -23,6 +26,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> { private final Id.Cluster cluster; private final Map<String, UnitState> newStates; private final SetUnitStateRequest.Condition condition; + private final TimeBudget timeBudget; public SetNodeStatesForClusterRequest(Id.Cluster cluster, SetUnitStateRequest request) { @@ -30,6 +34,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> { this.cluster = cluster; this.newStates = request.getNewState(); this.condition = request.getCondition(); + this.timeBudget = request.timeBudget(); } @Override @@ -80,6 +85,11 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> { } @Override + public Optional<Instant> getDeadline() { + return timeBudget.deadline(); + } + + @Override public boolean isFailed() { // Failure to set a node state is propagated as a 200 with wasModified false. return super.isFailed() || (resultSet && !result.getWasModified()); diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java index 3f977273054..f3a4be5ac2f 100644 --- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java +++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.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.clustercontroller.core.restapiv2; +import com.yahoo.time.TimeBudget; import com.yahoo.vdslib.state.NodeType; import com.yahoo.vespa.clustercontroller.core.MasterInterface; import com.yahoo.vespa.clustercontroller.core.RemoteClusterControllerTask; @@ -20,6 +21,8 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import java.time.Clock; +import java.time.Duration; import java.util.LinkedHashMap; import java.util.Map; @@ -42,6 +45,7 @@ public class SetNodeStateTest extends StateRestApiTest { private Map<String, UnitState> newStates = new LinkedHashMap<>(); private Condition condition = Condition.FORCE; private ResponseWait responseWait = ResponseWait.WAIT_UNTIL_CLUSTER_ACKED; + private TimeBudget timeBudget = TimeBudget.fromNow(Clock.systemUTC(), Duration.ofSeconds(10)); public SetUnitStateRequestImpl(String req) { super(req, 0); @@ -89,6 +93,11 @@ public class SetNodeStateTest extends StateRestApiTest { public ResponseWait getResponseWait() { return responseWait; } + + @Override + public TimeBudget timeBudget() { + return timeBudget; + } } private void verifyStateSet(String state, String reason) throws Exception { diff --git a/clustercontroller-utils/pom.xml b/clustercontroller-utils/pom.xml index ecfc6df3bd0..e176ce9d1e5 100644 --- a/clustercontroller-utils/pom.xml +++ b/clustercontroller-utils/pom.xml @@ -40,6 +40,11 @@ <version>${project.version}</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <scope>provided</scope> + </dependency> </dependencies> <build> <plugins> diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java index d583e4ecc27..a28ddb3539b 100644 --- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java +++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.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.clustercontroller.utils.staterestapi.requests; +import com.yahoo.time.TimeBudget; import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InvalidContentException; import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; @@ -62,4 +63,5 @@ public interface SetUnitStateRequest extends UnitRequest { } ResponseWait getResponseWait(); + TimeBudget timeBudget(); } diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java index 3d0856669f1..3c62d87098b 100644 --- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java +++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java @@ -8,10 +8,16 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; import org.codehaus.jettison.json.JSONArray; import org.codehaus.jettison.json.JSONObject; +import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.Optional; +import java.util.concurrent.TimeUnit; public class JsonReader { + public static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(30); + public static final Duration MAX_TIMEOUT = Duration.ofHours(1); + private static final long MICROS_IN_SECOND = TimeUnit.SECONDS.toMillis(1); private static class UnitStateImpl implements UnitState { private final String id; @@ -37,11 +43,16 @@ public class JsonReader { final Map<String, UnitState> stateMap; final SetUnitStateRequest.Condition condition; final SetUnitStateRequest.ResponseWait responseWait; - public SetRequestData(Map<String, UnitState> stateMap, SetUnitStateRequest.Condition condition, - SetUnitStateRequest.ResponseWait responseWait) { + final Optional<Duration> timeout; + + public SetRequestData(Map<String, UnitState> stateMap, + SetUnitStateRequest.Condition condition, + SetUnitStateRequest.ResponseWait responseWait, + Optional<Duration> timeout) { this.stateMap = stateMap; this.condition = condition; this.responseWait = responseWait; + this.timeout = timeout; } } @@ -98,7 +109,33 @@ public class JsonReader { } stateMap.put(type, new UnitStateImpl(code, reason)); } - return new SetRequestData(stateMap, condition, responseWait); + + final Optional<Duration> timeout = parseTimeout(request.getOption("timeout", null)); + + return new SetRequestData(stateMap, condition, responseWait, timeout); + } + + public static Optional<Duration> parseTimeout(String timeoutOption) throws InvalidContentException { + if (timeoutOption == null) { + return Optional.empty(); + } else { + float timeoutSeconds; + try { + timeoutSeconds = Float.parseFloat(timeoutOption); + } catch (NumberFormatException e) { + throw new InvalidContentException("value of timeout->" + timeoutOption + " is not a float"); + } + + if (timeoutSeconds <= 0.0) { + return Optional.of(Duration.ZERO); + } else if (timeoutSeconds <= MAX_TIMEOUT.getSeconds()) { + long micros = Math.round(timeoutSeconds * MICROS_IN_SECOND); + long nanoAdjustment = TimeUnit.MILLISECONDS.toNanos(micros % MICROS_IN_SECOND); + return Optional.of(Duration.ofSeconds(micros / MICROS_IN_SECOND, nanoAdjustment)); + } else { + throw new InvalidContentException("value of timeout->" + timeoutOption + " exceeds max timeout " + MAX_TIMEOUT); + } + } } } diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java index 71e6bc36de1..2ccfecf3d17 100644 --- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java +++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java @@ -1,19 +1,32 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.utils.staterestapi.server; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.log.LogLevel; -import com.yahoo.yolean.Exceptions; +import com.yahoo.time.TimeBudget; import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpRequest; import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpRequestHandler; import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpResult; import com.yahoo.vespa.clustercontroller.utils.communication.http.JsonHttpResult; import com.yahoo.vespa.clustercontroller.utils.staterestapi.StateRestAPI; -import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.*; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.DeadlineExceededException; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InvalidOptionValueException; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.OtherMasterException; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.StateRestApiException; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.UnknownMasterException; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStateRequest; import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.UnitStateRequest; -import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.*; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.SetResponse; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitResponse; +import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState; +import com.yahoo.yolean.Exceptions; -import java.util.*; +import java.time.Clock; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -24,10 +37,12 @@ public class RestApiHandler implements HttpRequestHandler { private final StateRestAPI restApi; private final JsonWriter jsonWriter; private final JsonReader jsonReader = new JsonReader(); + private final Clock clock; public RestApiHandler(StateRestAPI restApi) { this.restApi = restApi; this.jsonWriter = new JsonWriter(); + this.clock = Clock.systemUTC(); } public RestApiHandler setDefaultPathPrefix(String defaultPathPrefix) { @@ -43,6 +58,8 @@ public class RestApiHandler implements HttpRequestHandler { @Override public HttpResult handleRequest(HttpRequest request) { + Instant start = clock.instant(); + try{ final String[] unitPath = createUnitPath(request); if (request.getHttpOperation().equals(HttpRequest.HttpOp.GET)) { @@ -73,6 +90,8 @@ public class RestApiHandler implements HttpRequestHandler { public Condition getCondition() { return setRequestData.condition; } @Override public ResponseWait getResponseWait() { return setRequestData.responseWait; } + @Override + public TimeBudget timeBudget() { return TimeBudget.from(clock, start, setRequestData.timeout); } }); return new JsonHttpResult().setJson(jsonWriter.createJson(setResponse)); } @@ -89,7 +108,7 @@ public class RestApiHandler implements HttpRequestHandler { result.setHttpCode(503, "Service Unavailable"); result.setJson(jsonWriter.createErrorJson(exception.getMessage())); return result; - } catch (DeadlineExceededException exception) { + } catch (DeadlineExceededException | UncheckedTimeoutException exception) { logRequestException(request, exception, Level.WARNING); JsonHttpResult result = new JsonHttpResult(); result.setHttpCode(504, "Gateway Timeout"); diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReaderTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReaderTest.java new file mode 100644 index 00000000000..bf47a6605a2 --- /dev/null +++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReaderTest.java @@ -0,0 +1,20 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.clustercontroller.utils.staterestapi.server; + +import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InvalidContentException; +import org.junit.Test; + +import java.time.Duration; +import java.util.Optional; + +import static org.junit.Assert.assertEquals; + +public class JsonReaderTest { + @Test + public void testParsingOfTimeout() throws InvalidContentException { + assertEquals(Optional.empty(), JsonReader.parseTimeout(null)); + assertEquals(Optional.of(Duration.ofMillis(12500)), JsonReader.parseTimeout("12.5")); + assertEquals(Optional.of(Duration.ofMillis(0)), JsonReader.parseTimeout("-1")); + assertEquals(Optional.of(Duration.ofMillis(0)), JsonReader.parseTimeout("0.0001")); + } +}
\ No newline at end of file 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..e795e16813f 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java @@ -6,7 +6,6 @@ import com.yahoo.time.TimeBudget; import java.time.Clock; import java.time.Duration; -import java.util.Optional; /** * Context for the Orchestrator, e.g. timeout management. @@ -24,14 +23,14 @@ public class OrchestratorContext { /** Get the original timeout in seconds. */ public long getOriginalTimeoutInSeconds() { - return timeBudget.originalTimeout().getSeconds(); + return timeBudget.originalTimeout().get().getSeconds(); } /** * Get number of seconds until the deadline, or empty if there's no deadline, or throw * an {@link UncheckedTimeoutException} if timed out. */ - public Optional<Float> getSuboperationTimeoutInSeconds() { - return Optional.of((float) (timeBudget.timeLeftOrThrow().toMillis() / 1000.0)); + public float getSuboperationTimeoutInSeconds() { + return (float) (timeBudget.timeLeftOrThrow().get().toMillis() / 1000.0); } } 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 7d7a9b36ff9..ba469c1617e 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 @@ -40,7 +40,7 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{ return clusterControllerApi.apply(api -> api.setNodeState( clusterName, storageNodeIndex, - context.getSuboperationTimeoutInSeconds().orElse(null), + context.getSuboperationTimeoutInSeconds(), stateRequest) ); } catch (IOException e) { @@ -69,7 +69,7 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{ try { return clusterControllerApi.apply(api -> api.setClusterState( clusterName, - context.getSuboperationTimeoutInSeconds().orElse(null), + context.getSuboperationTimeoutInSeconds(), stateRequest)); } catch (IOException e) { final String message = String.format( 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 b3f80b5129f..c1dfc5d38f2 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 @@ -6,8 +6,6 @@ import com.yahoo.vespa.jaxrs.client.LocalPassThroughJaxRsStrategy; import com.yahoo.vespa.orchestrator.OrchestratorContext; import org.junit.Test; -import java.util.Optional; - import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -29,7 +27,7 @@ public class ClusterControllerClientTest { final ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE; OrchestratorContext context = mock(OrchestratorContext.class); - when(context.getSuboperationTimeoutInSeconds()).thenReturn(Optional.of(1.0f)); + when(context.getSuboperationTimeoutInSeconds()).thenReturn(1.0f); clusterControllerClient.setNodeState(context, STORAGE_NODE_INDEX, wantedState); final ClusterControllerStateRequest expectedNodeStateRequest = new ClusterControllerStateRequest( diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java index c3dbe5c8a92..87f07688c34 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java @@ -10,7 +10,6 @@ import org.junit.Test; import java.util.Arrays; import java.util.List; -import java.util.Optional; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.CoreMatchers.equalTo; @@ -67,7 +66,7 @@ public class SingleInstanceClusterControllerClientFactoryTest { public void testCreateClientWithSingleClusterControllerInstance() throws Exception { final List<HostName> clusterControllers = Arrays.asList(HOST_NAME_1); - when(context.getSuboperationTimeoutInSeconds()).thenReturn(Optional.of(1.0f)); + when(context.getSuboperationTimeoutInSeconds()).thenReturn(1.0f); clientFactory.createClient(clusterControllers, "clusterName") .setNodeState(context, 0, ClusterControllerNodeState.MAINTENANCE); @@ -95,7 +94,7 @@ public class SingleInstanceClusterControllerClientFactoryTest { public void testCreateClientWithThreeClusterControllerInstances() throws Exception { final List<HostName> clusterControllers = Arrays.asList(HOST_NAME_1, HOST_NAME_2, HOST_NAME_3); - when(context.getSuboperationTimeoutInSeconds()).thenReturn(Optional.of(1.0f)); + when(context.getSuboperationTimeoutInSeconds()).thenReturn(1.0f); clientFactory.createClient(clusterControllers, "clusterName") .setNodeState(context, 0, ClusterControllerNodeState.MAINTENANCE); diff --git a/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java b/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java index fa18cb5e467..a7963df0208 100644 --- a/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java +++ b/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java @@ -6,26 +6,31 @@ import com.google.common.util.concurrent.UncheckedTimeoutException; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.Optional; /** - * A TimeBudget can be used to track the time of an ongoing operation with a timeout. + * A TimeBudget can be used to track the time of an ongoing operation, possibly with a timeout. * * @author hakon */ public class TimeBudget { private final Clock clock; private final Instant start; - private final Duration timeout; + private final Optional<Duration> timeout; /** Returns a TimeBudget with a start time of now, and with the given timeout. */ public static TimeBudget fromNow(Clock clock, Duration timeout) { - return new TimeBudget(clock, clock.instant(), timeout); + return new TimeBudget(clock, clock.instant(), Optional.of(timeout)); } - private TimeBudget(Clock clock, Instant start, Duration timeout) { + public static TimeBudget from(Clock clock, Instant start, Optional<Duration> timeout) { + return new TimeBudget(clock, start, timeout); + } + + private TimeBudget(Clock clock, Instant start, Optional<Duration> timeout) { this.clock = clock; this.start = start; - this.timeout = makeNonNegative(timeout); + this.timeout = timeout.map(TimeBudget::makeNonNegative); } /** Returns time since start. */ @@ -33,26 +38,32 @@ public class TimeBudget { return nonNegativeBetween(start, clock.instant()); } - /** Returns the original timeout. */ - public Duration originalTimeout() { + /** Returns the original timeout, if any. */ + public Optional<Duration> originalTimeout() { return timeout; } + /** Returns the deadline, if present. */ + public Optional<Instant> deadline() { + return timeout.map(start::plus); + } + /** - * Returns the time until deadline. + * Returns the time until deadline, if there is one. * * @return time until deadline. It's toMillis() is guaranteed to be positive. * @throws UncheckedTimeoutException if the deadline has been reached or passed. */ - public Duration timeLeftOrThrow() { - Instant now = clock.instant(); - Duration left = Duration.between(now, start.plus(timeout)); - if (left.toMillis() <= 0) { - throw new UncheckedTimeoutException("Time since start " + nonNegativeBetween(start, now) + - " exceeds timeout " + timeout); - } + public Optional<Duration> timeLeftOrThrow() { + return timeout.map(timeout -> { + Duration passed = timePassed(); + Duration left = timeout.minus(passed); + if (left.toMillis() <= 0) { + throw new UncheckedTimeoutException("Time since start " + passed + " exceeds timeout " + this.timeout); + } - return left; + return left; + }); } private static Duration nonNegativeBetween(Instant start, Instant end) { diff --git a/vespajlib/src/test/java/com/yahoo/time/TimeBudgetTest.java b/vespajlib/src/test/java/com/yahoo/time/TimeBudgetTest.java index a51081067b7..f197cc34b32 100644 --- a/vespajlib/src/test/java/com/yahoo/time/TimeBudgetTest.java +++ b/vespajlib/src/test/java/com/yahoo/time/TimeBudgetTest.java @@ -8,8 +8,10 @@ import org.junit.Test; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.Optional; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; @@ -23,12 +25,12 @@ public class TimeBudgetTest { TimeBudget timeBudget = TimeBudget.fromNow(clock, Duration.ofSeconds(10)); clock.advance(Duration.ofSeconds(7)); - assertEquals(Duration.ofSeconds(3), timeBudget.timeLeftOrThrow()); + assertEquals(Duration.ofSeconds(3), timeBudget.timeLeftOrThrow().get()); // Verify that toMillis() of >=1 is fine, but 0 is not. clock.setInstant(Instant.ofEpochSecond(9, 999000000)); - assertEquals(1, timeBudget.timeLeftOrThrow().toMillis()); + assertEquals(1, timeBudget.timeLeftOrThrow().get().toMillis()); clock.setInstant(Instant.ofEpochSecond(9, 999000001)); try { timeBudget.timeLeftOrThrow(); @@ -37,4 +39,15 @@ public class TimeBudgetTest { // OK } } + + @Test + public void noDeadline() { + ManualClock clock = new ManualClock(); + clock.setInstant(Instant.ofEpochSecond(0)); + TimeBudget timeBudget = TimeBudget.from(clock, clock.instant(), Optional.empty()); + + assertFalse(timeBudget.originalTimeout().isPresent()); + assertFalse(timeBudget.timeLeftOrThrow().isPresent()); + assertFalse(timeBudget.deadline().isPresent()); + } }
\ No newline at end of file |