aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHÃ¥kon Hallingstad <hakon@oath.com>2018-10-29 10:50:07 +0100
committerGitHub <noreply@github.com>2018-10-29 10:50:07 +0100
commitd7e75c63c3c25474e825488c0daaa328e27472d8 (patch)
tree30f813e6cd54e73180e90affb29dc4a279104211
parent06056362f9190cf74634e4cce5b4ac0da5a6855c (diff)
Revert "Revert "Enforce CC timeouts in Orchestrator 2""
-rw-r--r--clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequest.java27
-rw-r--r--clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStatesForClusterRequest.java5
-rw-r--r--clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java3
-rw-r--r--clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/SetNodeStateTest.java9
-rw-r--r--clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java41
-rw-r--r--clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/requests/SetUnitStateRequest.java3
-rw-r--r--clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java10
-rw-r--r--clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java2
-rw-r--r--configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java5
-rw-r--r--jaxrs_client_utils/pom.xml6
-rw-r--r--jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java44
-rw-r--r--jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java4
-rw-r--r--jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java22
-rw-r--r--jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java28
-rw-r--r--jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java26
-rw-r--r--jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java14
-rw-r--r--jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java3
-rw-r--r--jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java48
-rw-r--r--node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java6
-rw-r--r--orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java8
-rw-r--r--orchestrator/pom.xml6
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java9
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java70
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java37
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java41
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java125
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java34
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactory.java54
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/NodeGroup.java2
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java7
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java8
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java29
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java16
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java8
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java151
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java54
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java116
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java10
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusServiceTest.java21
-rw-r--r--vespajlib/src/main/java/com/yahoo/time/TimeBudget.java12
40 files changed, 733 insertions, 391 deletions
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 4d6738940a8..eecdcc75228 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
@@ -37,6 +37,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
private final SetUnitStateRequest.ResponseWait responseWait;
private final WantedStateSetter wantedState;
private final TimeBudget timeBudget;
+ private final boolean probe;
public SetNodeStateRequest(Id.Node id, SetUnitStateRequest setUnitStateRequest) {
this(id, setUnitStateRequest, SetNodeStateRequest::setWantedState);
@@ -51,6 +52,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
this.responseWait = setUnitStateRequest.getResponseWait();
this.wantedState = wantedState;
this.timeBudget = setUnitStateRequest.timeBudget();
+ this.probe = setUnitStateRequest.isProbe();
}
@Override
@@ -61,7 +63,8 @@ public class SetNodeStateRequest extends Request<SetResponse> {
newStates,
id.getNode(),
context.nodeStateOrHostInfoChangeHandler,
- context.currentConsolidatedState);
+ context.currentConsolidatedState,
+ probe);
}
static NodeState getRequestedNodeState(Map<String, UnitState> newStates, Node n) throws StateRestApiException {
@@ -100,7 +103,8 @@ public class SetNodeStateRequest extends Request<SetResponse> {
Map<String, UnitState> newStates,
Node node,
NodeStateOrHostInfoChangeHandler stateListener,
- ClusterState currentClusterState) throws StateRestApiException {
+ ClusterState currentClusterState,
+ boolean probe) throws StateRestApiException {
if ( ! cluster.hasConfiguredNode(node.getIndex())) {
throw new MissingIdException(cluster.getName(), node);
}
@@ -126,7 +130,8 @@ public class SetNodeStateRequest extends Request<SetResponse> {
condition,
nodeInfo,
cluster,
- stateListener);
+ stateListener,
+ probe);
// If the state was successfully set, just return an "ok" message back.
String reason = success ? "ok" : result.getReason();
@@ -143,9 +148,10 @@ public class SetNodeStateRequest extends Request<SetResponse> {
SetUnitStateRequest.Condition condition,
NodeInfo nodeInfo,
ContentCluster cluster,
- NodeStateOrHostInfoChangeHandler stateListener) {
+ NodeStateOrHostInfoChangeHandler stateListener,
+ boolean probe) {
if (result.settingWantedStateIsAllowed()) {
- setNewWantedState(nodeInfo, newWantedState, stateListener);
+ setNewWantedState(nodeInfo, newWantedState, stateListener, probe);
}
// True if the wanted state was or has just been set to newWantedState
@@ -156,7 +162,7 @@ public class SetNodeStateRequest extends Request<SetResponse> {
// of the distributor. E.g. setting the storage node to maintenance may cause
// feeding issues unless distributor is also set down.
- setDistributorWantedState(cluster, nodeInfo.getNodeIndex(), newWantedState, stateListener);
+ setDistributorWantedState(cluster, nodeInfo.getNodeIndex(), newWantedState, stateListener, probe);
}
return success;
@@ -169,7 +175,8 @@ public class SetNodeStateRequest extends Request<SetResponse> {
private static void setDistributorWantedState(ContentCluster cluster,
int index,
NodeState newStorageWantedState,
- NodeStateOrHostInfoChangeHandler stateListener) {
+ NodeStateOrHostInfoChangeHandler stateListener,
+ boolean probe) {
Node distributorNode = new Node(NodeType.DISTRIBUTOR, index);
NodeInfo nodeInfo = cluster.getNodeInfo(distributorNode);
if (nodeInfo == null) {
@@ -200,13 +207,15 @@ public class SetNodeStateRequest extends Request<SetResponse> {
if (newWantedState.getState() != currentWantedState.getState() ||
!Objects.equals(newWantedState.getDescription(),
currentWantedState.getDescription())) {
- setNewWantedState(nodeInfo, newWantedState, stateListener);
+ setNewWantedState(nodeInfo, newWantedState, stateListener, probe);
}
}
private static void setNewWantedState(NodeInfo nodeInfo,
NodeState newWantedState,
- NodeStateOrHostInfoChangeHandler stateListener) {
+ NodeStateOrHostInfoChangeHandler stateListener,
+ boolean probe) {
+ if (probe) return;
nodeInfo.setWantedState(newWantedState);
stateListener.handleNewWantedNodeState(nodeInfo, newWantedState);
}
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 b4d189bcd55..d7820722887 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
@@ -27,6 +27,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> {
private final Map<String, UnitState> newStates;
private final SetUnitStateRequest.Condition condition;
private final TimeBudget timeBudget;
+ private final boolean probe;
public SetNodeStatesForClusterRequest(Id.Cluster cluster, SetUnitStateRequest request) {
@@ -35,6 +36,7 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> {
this.newStates = request.getNewState();
this.condition = request.getCondition();
this.timeBudget = request.timeBudget();
+ this.probe = request.isProbe();
}
@Override
@@ -69,7 +71,8 @@ public class SetNodeStatesForClusterRequest extends Request<SetResponse> {
newStates,
node,
context.nodeStateOrHostInfoChangeHandler,
- context.currentConsolidatedState);
+ context.currentConsolidatedState,
+ probe);
if (!setResponse.getWasModified()) {
throw new InternalFailure("We have not yet implemented the meaning of " +
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java
index 6fa7d536c67..c3090a5e832 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/WantedStateSetter.java
@@ -22,5 +22,6 @@ public interface WantedStateSetter {
Map<String, UnitState> newStates,
Node node,
NodeStateOrHostInfoChangeHandler stateListener,
- ClusterState currentClusterState) throws StateRestApiException;
+ ClusterState currentClusterState,
+ boolean probe) throws StateRestApiException;
}
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 f3a4be5ac2f..6cf4b7989e7 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
@@ -33,6 +33,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -46,6 +47,7 @@ public class SetNodeStateTest extends StateRestApiTest {
private Condition condition = Condition.FORCE;
private ResponseWait responseWait = ResponseWait.WAIT_UNTIL_CLUSTER_ACKED;
private TimeBudget timeBudget = TimeBudget.fromNow(Clock.systemUTC(), Duration.ofSeconds(10));
+ private boolean probe = false;
public SetUnitStateRequestImpl(String req) {
super(req, 0);
@@ -98,6 +100,11 @@ public class SetNodeStateTest extends StateRestApiTest {
public TimeBudget timeBudget() {
return timeBudget;
}
+
+ @Override
+ public boolean isProbe() {
+ return probe;
+ }
}
private void verifyStateSet(String state, String reason) throws Exception {
@@ -458,7 +465,7 @@ public class SetNodeStateTest extends StateRestApiTest {
new SetUnitStateRequestImpl("music/storage/1").setNewState("user", "maintenance", "whatever reason."),
wantedStateSetter);
SetResponse response = new SetResponse("some reason", wasModified);
- when(wantedStateSetter.set(any(), any(), any(), any(), any(), any())).thenReturn(response);
+ when(wantedStateSetter.set(any(), any(), any(), any(), any(), any(), anyBoolean())).thenReturn(response);
RemoteClusterControllerTask.Context context = mock(RemoteClusterControllerTask.Context.class);
MasterInterface masterInterface = mock(MasterInterface.class);
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java
index 9239b8774b0..7161fb1be79 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/restapiv2/requests/SetNodeStateRequestTest.java
@@ -23,20 +23,23 @@ import java.util.Map;
import java.util.Optional;
import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class SetNodeStateRequestTest {
- public static final String REASON = "operator";
- ContentCluster cluster = mock(ContentCluster.class);
- SetUnitStateRequest.Condition condition = SetUnitStateRequest.Condition.SAFE;
- Map<String, UnitState> newStates = new HashMap<>();
- UnitState unitState = mock(UnitState.class);
+ private static final String REASON = "operator";
+ private ContentCluster cluster = mock(ContentCluster.class);
+ private SetUnitStateRequest.Condition condition = SetUnitStateRequest.Condition.SAFE;
+ private Map<String, UnitState> newStates = new HashMap<>();
+ private UnitState unitState = mock(UnitState.class);
private final int NODE_INDEX = 2;
- Node storageNode = new Node(NodeType.STORAGE, NODE_INDEX);
- NodeStateOrHostInfoChangeHandler stateListener = mock(NodeStateOrHostInfoChangeHandler.class);
- ClusterState currentClusterState = mock(ClusterState.class);
+ private Node storageNode = new Node(NodeType.STORAGE, NODE_INDEX);
+ private NodeStateOrHostInfoChangeHandler stateListener = mock(NodeStateOrHostInfoChangeHandler.class);
+ private ClusterState currentClusterState = mock(ClusterState.class);
+ private boolean probe = false;
@Before
public void setUp() {
@@ -53,6 +56,16 @@ public class SetNodeStateRequestTest {
}
@Test
+ public void testProbingDoesntChangeState() throws StateRestApiException {
+ probe = true;
+ testSetStateRequest(
+ "maintenance",
+ State.UP, State.UP,
+ NodeStateChangeChecker.Result.allowSettingOfWantedState(),
+ Optional.empty(), Optional.empty());
+ }
+
+ @Test
public void testUpToDown() throws StateRestApiException {
testSetStateRequest(
"down",
@@ -124,6 +137,9 @@ public class SetNodeStateRequestTest {
when(cluster.getNodeInfo(distributorNode)).thenReturn(distributorNodeInfo);
NodeState distributorNodeState = new NodeState(distributorNode.getType(), distributorWantedState);
+ if (distributorWantedState != State.UP) {
+ distributorNodeState.setDescription(REASON);
+ }
when(distributorNodeInfo.getUserWantedState()).thenReturn(distributorNodeState);
setWantedState();
@@ -133,6 +149,9 @@ public class SetNodeStateRequestTest {
new NodeState(NodeType.STORAGE, expectedNewStorageWantedState.get());
verify(storageNodeInfo).setWantedState(expectedNewStorageNodeState);
verify(stateListener).handleNewWantedNodeState(storageNodeInfo, expectedNewStorageNodeState);
+ } else {
+ verify(storageNodeInfo, times(0)).setWantedState(any());
+ verify(stateListener, times(0)).handleNewWantedNodeState(eq(storageNodeInfo), any());
}
if (expectedNewDistributorWantedState.isPresent()) {
@@ -140,6 +159,9 @@ public class SetNodeStateRequestTest {
new NodeState(NodeType.DISTRIBUTOR, expectedNewDistributorWantedState.get());
verify(distributorNodeInfo).setWantedState(expectedNewDistributorNodeState);
verify(stateListener).handleNewWantedNodeState(distributorNodeInfo, expectedNewDistributorNodeState);
+ } else {
+ verify(distributorNodeInfo, times(0)).setWantedState(any());
+ verify(stateListener, times(0)).handleNewWantedNodeState(eq(distributorNodeInfo), any());
}
}
@@ -150,6 +172,7 @@ public class SetNodeStateRequestTest {
newStates,
storageNode,
stateListener,
- currentClusterState);
+ currentClusterState,
+ probe);
}
} \ No newline at end of file
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 a28ddb3539b..27f18c3664b 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
@@ -64,4 +64,7 @@ public interface SetUnitStateRequest extends UnitRequest {
ResponseWait getResponseWait();
TimeBudget timeBudget();
+
+ /** A probe request is a non-committal request to see if an identical (but non-probe) request would have succeeded. */
+ boolean isProbe();
}
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 d871a8ed6bc..dab6895cc9d 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
@@ -33,13 +33,16 @@ public class JsonReader {
}
static class SetRequestData {
+ final boolean probe;
final Map<String, UnitState> stateMap;
final SetUnitStateRequest.Condition condition;
final SetUnitStateRequest.ResponseWait responseWait;
- public SetRequestData(Map<String, UnitState> stateMap,
+ public SetRequestData(boolean probe,
+ Map<String, UnitState> stateMap,
SetUnitStateRequest.Condition condition,
SetUnitStateRequest.ResponseWait responseWait) {
+ this.probe = probe;
this.stateMap = stateMap;
this.condition = condition;
this.responseWait = responseWait;
@@ -49,8 +52,9 @@ public class JsonReader {
public SetRequestData getStateRequestData(HttpRequest request) throws Exception {
JSONObject json = new JSONObject(request.getPostContent().toString());
- final SetUnitStateRequest.Condition condition;
+ final boolean probe = json.has("probe") && json.getBoolean("probe");
+ final SetUnitStateRequest.Condition condition;
if (json.has("condition")) {
condition = SetUnitStateRequest.Condition.fromString(json.getString("condition"));
} else {
@@ -100,6 +104,6 @@ public class JsonReader {
stateMap.put(type, new UnitStateImpl(code, reason));
}
- return new SetRequestData(stateMap, condition, responseWait);
+ return new SetRequestData(probe, stateMap, condition, responseWait);
}
}
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 c38f7aec8c6..46f5d964245 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
@@ -97,6 +97,8 @@ public class RestApiHandler implements HttpRequestHandler {
public ResponseWait getResponseWait() { return setRequestData.responseWait; }
@Override
public TimeBudget timeBudget() { return TimeBudget.from(clock, start, timeout); }
+ @Override
+ public boolean isProbe() { return setRequestData.probe; }
});
return new JsonHttpResult().setJson(jsonWriter.createJson(setResponse));
}
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java
index e61d3710fac..73abd70a5ae 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java
@@ -47,11 +47,6 @@ public class OrchestratorMock implements Orchestrator {
}
@Override
- public void suspendGroup(NodeGroup nodeGroup) {
- nodeGroup.getHostNames().forEach(this::suspend);
- }
-
- @Override
public ApplicationInstanceStatus getApplicationInstanceStatus(ApplicationId appId) {
return suspendedApplications.contains(appId)
? ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN : ApplicationInstanceStatus.NO_REMARKS;
diff --git a/jaxrs_client_utils/pom.xml b/jaxrs_client_utils/pom.xml
index e3de5b8163d..43fbc66a9e6 100644
--- a/jaxrs_client_utils/pom.xml
+++ b/jaxrs_client_utils/pom.xml
@@ -18,6 +18,12 @@
<dependencies>
<dependency>
<groupId>com.yahoo.vespa</groupId>
+ <artifactId>vespajlib</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.yahoo.vespa</groupId>
<artifactId>application-model</artifactId>
<version>${project.version}</version>
<scope>provided</scope>
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java
index d004ac3af45..78076bdc04b 100644
--- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java
@@ -3,11 +3,55 @@ package com.yahoo.vespa.jaxrs.client;
import com.yahoo.vespa.applicationmodel.HostName;
+import java.net.URI;
+import java.time.Duration;
+
/**
* Interface for creating a JAX-RS client API instance for a single server endpoint.
*
* @author bakksjo
*/
public interface JaxRsClientFactory {
+ class Params<T> {
+ private final Class<T> apiClass;
+ private final URI uri;
+
+ private Duration connectTimeout = Duration.ofSeconds(30);
+ private Duration readTimeout = Duration.ofSeconds(30);
+
+ public Params(Class<T> apiClass, URI uri) {
+ this.apiClass = apiClass;
+ this.uri = uri;
+ }
+
+ public Class<T> apiClass() {
+ return apiClass;
+ }
+
+ public URI uri() {
+ return uri;
+ }
+
+ public void setConnectTimeout(Duration timeout) {
+ this.connectTimeout = timeout;
+ }
+
+ public Duration connectTimeout() {
+ return connectTimeout;
+ }
+
+ public void setReadTimeout(Duration timeout) {
+ readTimeout = timeout;
+ }
+
+ public Duration readTimeout() {
+ return readTimeout;
+ }
+ }
+
+ default <T> T createClient(Params<T> params) {
+ throw new UnsupportedOperationException("Not implemented");
+ }
+
<T> T createClient(Class<T> apiClass, HostName hostName, int port, String pathPrefix, String scheme);
}
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java
index 72af76fe54c..cd7d8684cbc 100644
--- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java
@@ -11,4 +11,8 @@ import java.util.function.Function;
*/
public interface JaxRsStrategy<T> {
<R> R apply(final Function<T, R> function) throws IOException;
+
+ default <R> R apply(final Function<T, R> function, JaxRsTimeouts timeouts) throws IOException {
+ return apply(function);
+ }
}
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java
new file mode 100644
index 00000000000..6758d6f94d6
--- /dev/null
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java
@@ -0,0 +1,22 @@
+package com.yahoo.vespa.jaxrs.client;
+
+import java.time.Duration;
+
+/**
+ * @author hakonhall
+ */
+public interface JaxRsTimeouts {
+ /**
+ * The connect timeout, which must be at least 1ms.
+ *
+ * Throws com.google.common.util.concurrent.UncheckedTimeoutException on timeout.
+ */
+ Duration getConnectTimeoutOrThrow();
+
+ /**
+ * The read timeout, which must be at least 1ms.
+ *
+ * Throws com.google.common.util.concurrent.UncheckedTimeoutException on timeout.
+ */
+ Duration getReadTimeoutOrThrow();
+}
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java
index 9321f8e290d..8aa880fb0e4 100644
--- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java
@@ -23,34 +23,20 @@ import java.util.Collections;
*/
public class JerseyJaxRsClientFactory implements JaxRsClientFactory {
- private static final int DEFAULT_CONNECT_TIMEOUT_MS = 30000;
- private static final int DEFAULT_READ_TIMEOUT_MS = 30000;
-
// Client is a heavy-weight object with a finalizer so we create only one and re-use it
private final Client client;
public JerseyJaxRsClientFactory() {
- this(DEFAULT_CONNECT_TIMEOUT_MS, DEFAULT_READ_TIMEOUT_MS);
+ this(null, null, null);
}
public JerseyJaxRsClientFactory(SSLContext sslContext, HostnameVerifier hostnameVerifier, String userAgent) {
- this(DEFAULT_CONNECT_TIMEOUT_MS, DEFAULT_READ_TIMEOUT_MS, sslContext, hostnameVerifier, userAgent);
- }
-
- public JerseyJaxRsClientFactory(int connectTimeoutMs, int readTimeoutMs) {
- this(connectTimeoutMs, readTimeoutMs, null, null, null);
- }
-
- public JerseyJaxRsClientFactory(int connectTimeoutMs, int readTimeoutMs, SSLContext sslContext,
- HostnameVerifier hostnameVerifier, String userAgent) {
/*
* Configure client with some workarounds for HTTP/JAX-RS/Jersey issues. See:
* https://jersey.java.net/apidocs/latest/jersey/org/glassfish/jersey/client/ClientProperties.html#SUPPRESS_HTTP_COMPLIANCE_VALIDATION
* https://jersey.java.net/apidocs/latest/jersey/org/glassfish/jersey/client/HttpUrlConnectorProvider.html#SET_METHOD_WORKAROUND
*/
ClientBuilder builder = ClientBuilder.newBuilder()
- .property(ClientProperties.CONNECT_TIMEOUT, connectTimeoutMs)
- .property(ClientProperties.READ_TIMEOUT, readTimeoutMs)
.property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true) // Allow empty PUT. TODO: Fix API.
.property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true) // Allow e.g. PATCH method.
.property(ClientProperties.FOLLOW_REDIRECTS, true);
@@ -67,10 +53,16 @@ public class JerseyJaxRsClientFactory implements JaxRsClientFactory {
}
@Override
+ public <T> T createClient(Params<T> params) {
+ WebTarget target = client.target(params.uri());
+ target.property(ClientProperties.CONNECT_TIMEOUT, (int) params.connectTimeout().toMillis());
+ target.property(ClientProperties.READ_TIMEOUT, (int) params.readTimeout().toMillis());
+ return WebResourceFactory.newResource(params.apiClass(), target);
+ }
+
+ @Override
public <T> T createClient(Class<T> apiClass, HostName hostName, int port, String pathPrefix, String scheme) {
UriBuilder uriBuilder = UriBuilder.fromPath(pathPrefix).host(hostName.s()).port(port).scheme(scheme);
- WebTarget target = client.target(uriBuilder);
- return WebResourceFactory.newResource(apiClass, target);
+ return createClient(new Params<>(apiClass, uriBuilder.build()));
}
-
}
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java
new file mode 100644
index 00000000000..3f2139f6bf0
--- /dev/null
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java
@@ -0,0 +1,26 @@
+package com.yahoo.vespa.jaxrs.client;
+
+import java.time.Duration;
+
+/**
+ * Legacy defaults for timeouts.
+ *
+ * Clients should instead define their own JaxRsTimeouts tailored to their use-case.
+ *
+ * @author hakonhall
+ */
+// Immutable
+public class LegacyJaxRsTimeouts implements JaxRsTimeouts {
+ private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(30);
+ private static final Duration READ_TIMEOUT = Duration.ofSeconds(30);
+
+ @Override
+ public Duration getConnectTimeoutOrThrow() {
+ return CONNECT_TIMEOUT;
+ }
+
+ @Override
+ public Duration getReadTimeoutOrThrow() {
+ return READ_TIMEOUT;
+ }
+}
diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java
index 65b302ef4ff..4c97147d61e 100644
--- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java
+++ b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java
@@ -4,7 +4,9 @@ package com.yahoo.vespa.jaxrs.client;
import com.yahoo.vespa.applicationmodel.HostName;
import javax.ws.rs.ProcessingException;
+import javax.ws.rs.core.UriBuilder;
import java.io.IOException;
+import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -65,11 +67,21 @@ public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> {
@Override
public <R> R apply(final Function<T, R> function) throws IOException {
+ return apply(function, new LegacyJaxRsTimeouts());
+ }
+
+
+ @Override
+ public <R> R apply(final Function<T, R> function, JaxRsTimeouts timeouts) throws IOException {
ProcessingException sampleException = null;
for (int i = 0; i < maxIterations; ++i) {
for (final HostName hostName : hostNames) {
- final T jaxRsClient = jaxRsClientFactory.createClient(apiClass, hostName, port, pathPrefix, scheme);
+ URI uri = UriBuilder.fromPath(pathPrefix).port(port).scheme(scheme).host(hostName.s()).build();
+ JaxRsClientFactory.Params<T> params = new JaxRsClientFactory.Params<>(apiClass, uri);
+ params.setConnectTimeout(timeouts.getConnectTimeoutOrThrow());
+ params.setReadTimeout(timeouts.getReadTimeoutOrThrow());
+ final T jaxRsClient = jaxRsClientFactory.createClient(params);
try {
return function.apply(jaxRsClient);
} catch (ProcessingException e) {
diff --git a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java
index 63e2b814c24..8161602cdac 100644
--- a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java
+++ b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java
@@ -74,8 +74,7 @@ public class HttpPatchTest extends JerseyTest {
final JaxRsStrategy<TestResourceApi> client = factory.apiNoRetries(TestResourceApi.class, apiPath);
final String responseBody;
- responseBody = client.apply(api ->
- api.doPatch(REQUEST_BODY));
+ responseBody = client.apply(api -> api.doPatch(REQUEST_BODY));
assertThat(testResourceSingleton.invocation.get(60, TimeUnit.SECONDS), is(REQUEST_BODY));
assertThat(responseBody, is(REQUEST_BODY));
diff --git a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java
index e31920febd6..dbe886b7896 100644
--- a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java
+++ b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java
@@ -5,7 +5,10 @@ import com.yahoo.vespa.applicationmodel.HostName;
import com.yahoo.vespa.defaults.Defaults;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.OngoingStubbing;
import javax.ws.rs.GET;
@@ -15,24 +18,26 @@ import java.io.IOException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
+import java.util.stream.Collectors;
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+@RunWith(MockitoJUnitRunner.class)
public class RetryingJaxRsStrategyTest {
private static final String API_PATH = "/";
+ @Captor
+ ArgumentCaptor<JaxRsClientFactory.Params<TestJaxRsApi>> paramsCaptor;
+
@Path(API_PATH)
private interface TestJaxRsApi {
@GET
@@ -53,8 +58,7 @@ public class RetryingJaxRsStrategyTest {
@Before
public void setup() {
- when(jaxRsClientFactory.createClient(eq(TestJaxRsApi.class), any(HostName.class), anyInt(), anyString(), anyString()))
- .thenReturn(mockApi);
+ when(jaxRsClientFactory.createClient(any())).thenReturn(mockApi);
}
@Test
@@ -63,11 +67,12 @@ public class RetryingJaxRsStrategyTest {
verify(mockApi, times(1)).doSomething();
- // Check that one of the supplied hosts is contacted.
- final ArgumentCaptor<HostName> hostNameCaptor = ArgumentCaptor.forClass(HostName.class);
- verify(jaxRsClientFactory, times(1))
- .createClient(eq(TestJaxRsApi.class), hostNameCaptor.capture(), eq(REST_PORT), eq(API_PATH), eq("http"));
- assertThat(SERVER_HOSTS.contains(hostNameCaptor.getValue()), is(true));
+ verify(jaxRsClientFactory, times(1)).createClient(paramsCaptor.capture());
+ JaxRsClientFactory.Params<TestJaxRsApi> params = paramsCaptor.getValue();
+ assertEquals(REST_PORT, params.uri().getPort());
+ assertEquals(API_PATH, params.uri().getPath());
+ assertEquals("http", params.uri().getScheme());
+ assertThat(SERVER_HOSTS, hasItem(new HostName(params.uri().getHost())));
}
@Test
@@ -99,10 +104,10 @@ public class RetryingJaxRsStrategyTest {
@Test
public void testRetryLoopsOverAvailableServers() throws Exception {
when(mockApi.doSomething())
- .thenThrow(new ProcessingException("Fake timeout 1 induced by test"))
- .thenThrow(new ProcessingException("Fake timeout 2 induced by test"))
- .thenThrow(new ProcessingException("Fake timeout 3 induced by test"))
- .thenThrow(new ProcessingException("Fake timeout 4 induced by test"))
+ .thenThrow(new ProcessingException("Fake socket timeout 1 induced by test"))
+ .thenThrow(new ProcessingException("Fake socket timeout 2 induced by test"))
+ .thenThrow(new ProcessingException("Fake socket timeout 3 induced by test"))
+ .thenThrow(new ProcessingException("Fake socket timeout 4 induced by test"))
.thenReturn("a response");
jaxRsStrategy.apply(TestJaxRsApi::doSomething);
@@ -142,12 +147,9 @@ public class RetryingJaxRsStrategyTest {
verifyAllServersContacted(jaxRsClientFactory);
}
- private static void verifyAllServersContacted(
- final JaxRsClientFactory jaxRsClientFactory) {
- final ArgumentCaptor<HostName> hostNameCaptor = ArgumentCaptor.forClass(HostName.class);
- verify(jaxRsClientFactory, atLeast(SERVER_HOSTS.size()))
- .createClient(eq(TestJaxRsApi.class), hostNameCaptor.capture(), eq(REST_PORT), eq(API_PATH), eq("http"));
- final Set<HostName> actualServerHostsContacted = new HashSet<>(hostNameCaptor.getAllValues());
- assertThat(actualServerHostsContacted, equalTo(SERVER_HOSTS));
+ private void verifyAllServersContacted(final JaxRsClientFactory jaxRsClientFactory) {
+ verify(jaxRsClientFactory, atLeast(SERVER_HOSTS.size())).createClient(paramsCaptor.capture());
+ final Set<JaxRsClientFactory.Params<TestJaxRsApi>> actualServerHostsContacted = new HashSet<>(paramsCaptor.getAllValues());
+ assertEquals(actualServerHostsContacted.stream().map(x -> new HostName(x.uri().getHost())).collect(Collectors.toSet()), SERVER_HOSTS);
}
}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
index c71b0783d69..70750dd6672 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java
@@ -5,7 +5,6 @@ import com.yahoo.config.provision.ApplicationId;
import com.yahoo.vespa.applicationmodel.HostName;
import com.yahoo.vespa.orchestrator.Host;
import com.yahoo.vespa.orchestrator.Orchestrator;
-import com.yahoo.vespa.orchestrator.model.NodeGroup;
import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
import com.yahoo.vespa.orchestrator.status.HostStatus;
@@ -46,11 +45,6 @@ public class OrchestratorMock implements Orchestrator {
}
@Override
- public void suspendGroup(NodeGroup nodeGroup) {
- nodeGroup.getHostNames().forEach(this::suspend);
- }
-
- @Override
public ApplicationInstanceStatus getApplicationInstanceStatus(ApplicationId appId) {
return suspendedApplications.contains(appId)
? ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN : ApplicationInstanceStatus.NO_REMARKS;
diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java
index 603d6a1adac..9535096af4f 100644
--- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java
+++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java
@@ -22,13 +22,9 @@ public interface HostSuspensionApi {
String PATH_PREFIX = "/v1/suspensions/hosts";
/**
- * Ask for permission to temporarily suspend all services on a set of hosts.
+ * Ask for permission to temporarily suspend all services on a set of hosts (nodes).
*
- * See HostApi::suspend for semantics of suspending a host.
- *
- * On failure, it tries to resume ALL hosts. It needs to try to resume all hosts because any or all hosts
- * may have been suspended in an earlier attempt. Ending with resumption of all hosts makes sure other
- * batch-requests for suspension of hosts succeed.
+ * See HostApi::suspend for semantics of suspending a node.
*/
@PUT
@Path("/{hostname}")
diff --git a/orchestrator/pom.xml b/orchestrator/pom.xml
index ae05a1908c9..37d308111ae 100644
--- a/orchestrator/pom.xml
+++ b/orchestrator/pom.xml
@@ -117,6 +117,12 @@
<scope>provided</scope>
</dependency>
<dependency>
+ <groupId>com.yahoo.vespa</groupId>
+ <artifactId>testutil</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
<groupId>org.apache.curator</groupId>
<artifactId>curator-test</artifactId>
<scope>test</scope>
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java
index b2be4fe52ec..df124f2f690 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java
@@ -75,15 +75,6 @@ public interface Orchestrator {
void acquirePermissionToRemove(HostName hostName) throws OrchestrationException;
/**
- * Suspend normal operations for a group of nodes in the same application.
- *
- * @param nodeGroup The group of nodes in an application.
- * @throws HostStateChangeDeniedException if the request cannot be meet due to policy constraints.
- * @throws HostNameNotFoundException if any hostnames in the node group is not recognized
- */
- void suspendGroup(NodeGroup nodeGroup) throws HostStateChangeDeniedException, HostNameNotFoundException;
-
- /**
* Suspend several hosts. On failure, all hosts are resumed before exiting the method with an exception.
*/
void suspendAll(HostName parentHostname, List<HostName> hostNames)
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 6577b4b96cc..25c6c780130 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java
@@ -1,40 +1,74 @@
// 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.vespa.orchestrator.controller.ClusterControllerClientTimeouts;
import java.time.Clock;
import java.time.Duration;
+import java.time.Instant;
+import java.util.Optional;
/**
- * Context for the Orchestrator, e.g. timeout management.
+ * Context for an operation (or suboperation) of the Orchestrator that needs to pass through to the backend,
+ * e.g. timeout management and probing.
*
- * @author hakon
+ * @author hakonhall
*/
public class OrchestratorContext {
- private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(10);
+ private static final Duration DEFAULT_TIMEOUT_FOR_SINGLE_OP = Duration.ofSeconds(10);
+ private static final Duration DEFAULT_TIMEOUT_FOR_BATCH_OP = Duration.ofSeconds(60);
- private TimeBudget timeBudget;
+ private final Clock clock;
+ private final TimeBudget timeBudget;
+ private boolean probe;
- public OrchestratorContext(Clock clock) {
- this.timeBudget = TimeBudget.fromNow(clock, DEFAULT_TIMEOUT);
+ public static OrchestratorContext createContextForMultiAppOp(Clock clock) {
+ return new OrchestratorContext(clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_BATCH_OP), true);
}
- /** Get the original timeout in seconds. */
- public long getOriginalTimeoutInSeconds() {
- return timeBudget.originalTimeout().get().getSeconds();
+ public static OrchestratorContext createContextForSingleAppOp(Clock clock) {
+ return new OrchestratorContext(clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_SINGLE_OP), true);
}
- /**
- * Get timeout for a suboperation that should take up {@code shareOfRemaining} of the
- * remaining time, or throw an {@link UncheckedTimeoutException} if timed out.
- */
- public float getSuboperationTimeoutInSeconds(float shareOfRemaining) {
- if (!(0f <= shareOfRemaining && shareOfRemaining <= 1.0f)) {
- throw new IllegalArgumentException("Share of remaining time must be between 0 and 1: " + shareOfRemaining);
+ private OrchestratorContext(Clock clock, TimeBudget timeBudget, boolean probe) {
+ this.clock = clock;
+ this.timeBudget = timeBudget;
+ this.probe = probe;
+ }
+
+ public Duration getTimeLeft() {
+ return timeBudget.timeLeftOrThrow().get();
+ }
+
+ public ClusterControllerClientTimeouts getClusterControllerTimeouts(String clusterName) {
+ return new ClusterControllerClientTimeouts(clusterName, timeBudget.timeLeftAsTimeBudget());
+ }
+
+
+ /** Mark this operation as a non-committal probe. */
+ public OrchestratorContext markAsProbe() {
+ this.probe = true;
+ return this;
+ }
+
+ /** Whether the operation is a no-op probe to test whether it would have succeeded, if it had been committal. */
+ public boolean isProbe() {
+ return probe;
+ }
+
+ /** Create an OrchestratorContext to use within an application lock. */
+ public OrchestratorContext createSubcontextForApplication() {
+ Instant now = clock.instant();
+ Instant deadline = timeBudget.deadline().get();
+ Instant maxDeadline = now.plus(DEFAULT_TIMEOUT_FOR_SINGLE_OP);
+ if (maxDeadline.compareTo(deadline) < 0) {
+ deadline = maxDeadline;
}
- return shareOfRemaining * timeBudget.timeLeftOrThrow().get().toMillis() / 1000.0f;
+ return new OrchestratorContext(
+ clock,
+ TimeBudget.from(clock, now, Optional.of(Duration.between(now, deadline))),
+ probe);
}
}
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..3a2673d9dbc 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -105,7 +105,9 @@ public class OrchestratorImpl implements Orchestrator {
@Override
public void setNodeStatus(HostName hostName, HostStatus status) throws OrchestrationException {
ApplicationInstanceReference reference = getApplicationInstance(hostName).reference();
- try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(reference)) {
+ OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
+ try (MutableStatusRegistry statusRegistry = statusService
+ .lockApplicationInstance_forCurrentThreadOnly(reference, context.getTimeLeft())) {
statusRegistry.setHostState(hostName, status);
}
}
@@ -127,10 +129,10 @@ public class OrchestratorImpl implements Orchestrator {
ApplicationInstance appInstance = getApplicationInstance(hostName);
- OrchestratorContext context = new OrchestratorContext(clock);
+ OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(
appInstance.reference(),
- context.getOriginalTimeoutInSeconds())) {
+ context.getTimeLeft())) {
final HostStatus currentHostState = statusRegistry.getHostStatus(hostName);
if (HostStatus.NO_REMARKS == currentHostState) {
@@ -148,7 +150,7 @@ public class OrchestratorImpl implements Orchestrator {
public void suspend(HostName hostName) throws HostStateChangeDeniedException, HostNameNotFoundException {
ApplicationInstance appInstance = getApplicationInstance(hostName);
NodeGroup nodeGroup = new NodeGroup(appInstance, hostName);
- suspendGroup(nodeGroup);
+ suspendGroup(OrchestratorContext.createContextForSingleAppOp(clock), nodeGroup);
}
@Override
@@ -156,10 +158,10 @@ public class OrchestratorImpl implements Orchestrator {
ApplicationInstance appInstance = getApplicationInstance(hostName);
NodeGroup nodeGroup = new NodeGroup(appInstance, hostName);
- OrchestratorContext context = new OrchestratorContext(clock);
+ OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(
appInstance.reference(),
- context.getOriginalTimeoutInSeconds())) {
+ context.getTimeLeft())) {
ApplicationApi applicationApi = new ApplicationApiImpl(
nodeGroup,
statusRegistry,
@@ -169,16 +171,19 @@ public class OrchestratorImpl implements Orchestrator {
}
}
- // Public for testing purposes
- @Override
- public void suspendGroup(NodeGroup nodeGroup) throws HostStateChangeDeniedException, HostNameNotFoundException {
+ /**
+ * Suspend normal operations for a group of nodes in the same application.
+ *
+ * @param nodeGroup The group of nodes in an application.
+ * @throws HostStateChangeDeniedException if the request cannot be meet due to policy constraints.
+ */
+ void suspendGroup(OrchestratorContext context, NodeGroup nodeGroup) throws HostStateChangeDeniedException {
ApplicationInstanceReference applicationReference = nodeGroup.getApplicationReference();
- OrchestratorContext context = new OrchestratorContext(clock);
try (MutableStatusRegistry hostStatusRegistry =
statusService.lockApplicationInstance_forCurrentThreadOnly(
applicationReference,
- context.getOriginalTimeoutInSeconds())) {
+ context.getTimeLeft())) {
ApplicationInstanceStatus appStatus = statusService.forApplicationInstance(applicationReference).getApplicationInstanceStatus();
if (appStatus == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) {
return;
@@ -224,14 +229,12 @@ public class OrchestratorImpl implements Orchestrator {
throw new BatchHostNameNotFoundException(parentHostname, hostNames, e);
}
+ OrchestratorContext context = OrchestratorContext.createContextForMultiAppOp(clock);
for (NodeGroup nodeGroup : nodeGroupsOrderedByApplication) {
try {
- suspendGroup(nodeGroup);
+ suspendGroup(context.createSubcontextForApplication(), nodeGroup);
} catch (HostStateChangeDeniedException e) {
throw new BatchHostStateChangeDeniedException(parentHostname, nodeGroup, e);
- } catch (HostNameNotFoundException e) {
- // Should never get here since since we would have received HostNameNotFoundException earlier.
- throw new BatchHostNameNotFoundException(parentHostname, hostNames, e);
} catch (RuntimeException e) {
throw new BatchInternalErrorException(parentHostname, nodeGroup, e);
}
@@ -301,12 +304,12 @@ public class OrchestratorImpl implements Orchestrator {
private void setApplicationStatus(ApplicationId appId, ApplicationInstanceStatus status)
throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException{
- OrchestratorContext context = new OrchestratorContext(clock);
+ OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
try (MutableStatusRegistry statusRegistry =
statusService.lockApplicationInstance_forCurrentThreadOnly(
appRef,
- context.getOriginalTimeoutInSeconds())) {
+ context.getTimeLeft())) {
// Short-circuit if already in wanted state
if (status == statusRegistry.getApplicationInstanceStatus()) return;
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 467b534f809..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
@@ -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.google.common.util.concurrent.UncheckedTimeoutException;
import com.yahoo.vespa.jaxrs.client.JaxRsStrategy;
import com.yahoo.vespa.orchestrator.OrchestratorContext;
@@ -15,23 +16,6 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{
public static final String REQUEST_REASON = "Orchestrator";
- // On setNodeState calls against the CC ensemble.
- //
- // We'd like to set a timeout for the request to the first CC such that if the first
- // CC is faulty, there's sufficient time to send the request to the second and third CC.
- // The timeouts would be:
- // timeout(1. request) = SHARE_REMAINING_TIME * T
- // timeout(2. request) = SHARE_REMAINING_TIME * T * (1 - SHARE_REMAINING_TIME)
- // timeout(3. request) = SHARE_REMAINING_TIME * T * (1 - SHARE_REMAINING_TIME)^2
- //
- // Using a share of 50% gives approximately:
- // timeout(1. request) = T * 0.5
- // timeout(2. request) = T * 0.25
- // timeout(3. request) = T * 0.125
- //
- // which seems fine
- public static final float SHARE_REMAINING_TIME = 0.5f;
-
private final JaxRsStrategy<ClusterControllerJaxRsApi> clusterControllerApi;
private final String clusterName;
@@ -52,15 +36,16 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{
ClusterControllerNodeState wantedState) throws IOException {
ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON);
ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest(state, ClusterControllerStateRequest.Condition.SAFE);
+ ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(clusterName);
try {
return clusterControllerApi.apply(api -> api.setNodeState(
clusterName,
storageNodeIndex,
- context.getSuboperationTimeoutInSeconds(SHARE_REMAINING_TIME),
- stateRequest)
- );
- } catch (IOException e) {
+ timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f,
+ stateRequest),
+ timeouts);
+ } catch (IOException | UncheckedTimeoutException e) {
String message = String.format(
"Giving up setting %s for storage node with index %d in cluster %s",
stateRequest,
@@ -79,16 +64,18 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{
@Override
public ClusterControllerStateResponse setApplicationState(
OrchestratorContext context,
- final ClusterControllerNodeState wantedState) throws IOException {
- final ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON);
- final ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest(state, ClusterControllerStateRequest.Condition.FORCE);
+ ClusterControllerNodeState wantedState) throws IOException {
+ ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON);
+ ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest(state, ClusterControllerStateRequest.Condition.FORCE);
+ ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(clusterName);
try {
return clusterControllerApi.apply(api -> api.setClusterState(
clusterName,
- context.getSuboperationTimeoutInSeconds(SHARE_REMAINING_TIME),
- stateRequest));
- } catch (IOException e) {
+ timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f,
+ stateRequest),
+ timeouts);
+ } catch (IOException | UncheckedTimeoutException e) {
final String message = String.format(
"Giving up setting %s for cluster %s",
stateRequest,
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
new file mode 100644
index 00000000000..5b0685e88a0
--- /dev/null
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java
@@ -0,0 +1,125 @@
+package com.yahoo.vespa.orchestrator.controller;
+
+import com.google.common.util.concurrent.UncheckedTimeoutException;
+import com.yahoo.time.TimeBudget;
+import com.yahoo.vespa.jaxrs.client.JaxRsTimeouts;
+
+import java.time.Duration;
+
+/**
+ * Calculates various timeouts associated with a REST call from the Orchestrator to the Cluster Controller.
+ *
+ * <p>Timeout handling of HTTP messaging is fundamentally flawed in various Java implementations.
+ * We would like to specify a max time for the whole operation (connect, send request, and receive response).
+ * Jersey JAX-RS implementation and the Apache HTTP client library provides a way to set the connect timeout C
+ * and read timeout R. So if the operation takes NR reads, and the writes takes TW time,
+ * the theoretical max time is: T = C + R * NR + TW. With both NR and TW unknown, there's no way to
+ * set a proper C and R.</p>
+ *
+ * <p>The various timeouts is set according to the following considerations:</p>
+ *
+ * <ol>
+ * <li>Some time is reserved for the execution in this process, e.g. execution leading to the REST call,
+ * handling of the response, exception handling, etc, such that we can finish processing this request
+ * before the {@link #timeBudget} deadline. This is typically in the order of ms.</li>
+ * <li>A timeout will be passed to the Cluster Controller backend. We'll give a timeout such that if one
+ * CC times out, the next CC will be given exactly the same timeout. This may or may not be a good strategy:
+ * (A) There's typically a 3rd CC. But if the first 2 fails with timeout, the chance the last is OK
+ * is negligible. (B) If picking the CC is random, then giving the full timeout to the first
+ * should be sufficient since a later retry will hit the healthy CC. (C) Because we have been using
+ * DROP in networking rules, clients may time out (host out of app or whatever). This would suggest
+ * allowing more than 1 full request.</li>
+ * <li>The timeout passed to the CC backend should be such that if it honors that, the Orchestrator
+ * should not time out. This means some kernel and network overhead should be subtracted from the timeout
+ * passed to the CC.</li>
+ * <li>We're only able to set the connect and read/write timeouts(!) Since we're communicating within
+ * data center, assume connections are in the order of ms, while a single read may stall close up to the CC
+ * timeout.</li>
+ * </ol>
+ *
+ * @author hakonhall
+ */
+public class ClusterControllerClientTimeouts implements JaxRsTimeouts {
+ // In data center connect timeout
+ static final Duration CONNECT_TIMEOUT = Duration.ofMillis(50);
+ // Per call overhead
+ static final Duration IN_PROCESS_OVERHEAD_PER_CALL = Duration.ofMillis(50);
+ // In data center kernel and network overhead.
+ static final Duration NETWORK_OVERHEAD_PER_CALL = CONNECT_TIMEOUT;
+ // Minimum time reserved for post-RPC processing to finish BEFORE the deadline, including ZK write.
+ static final Duration IN_PROCESS_OVERHEAD = Duration.ofMillis(100);
+ // Number of JAX-RS RPC calls to account for within the time budget.
+ static final int NUM_CALLS = 2;
+ // Minimum server-side timeout
+ static final Duration MIN_SERVER_TIMEOUT = Duration.ofMillis(10);
+
+ private final String clusterName;
+ private final TimeBudget timeBudget;
+ private final Duration maxClientTimeout;
+
+ /**
+ * Creates a timeouts instance.
+ *
+ * The {@link #timeBudget} SHOULD be the time budget for a single logical call to the Cluster Controller.
+ * A logical call to CC may in fact call the CC several times, if the first onces are down and/or not
+ * the master.
+ *
+ * @param clusterName The name of the content cluster this request is for.
+ * @param timeBudget The time budget for a single logical call to the the Cluster Controller.
+ */
+ public ClusterControllerClientTimeouts(String clusterName, TimeBudget timeBudget) {
+ this.clusterName = clusterName;
+ this.timeBudget = timeBudget;
+
+ // timeLeft = inProcessOverhead + numCalls * clientTimeout
+ maxClientTimeout = timeBudget.originalTimeout().get().minus(IN_PROCESS_OVERHEAD).dividedBy(NUM_CALLS);
+ }
+
+ @Override
+ public Duration getConnectTimeoutOrThrow() {
+ return CONNECT_TIMEOUT;
+ }
+
+ @Override
+ public Duration getReadTimeoutOrThrow() {
+ Duration timeLeft = timeBudget.timeLeft().get();
+ if (timeLeft.toMillis() <= 0) {
+ throw new UncheckedTimeoutException("Exceeded the timeout " + timeBudget.originalTimeout().get() +
+ " against content cluster '" + clusterName + "' by " + timeLeft.negated());
+ }
+
+ Duration clientTimeout = min(timeLeft, maxClientTimeout);
+ verifyPositive(timeLeft, clientTimeout);
+
+ // clientTimeout = overheadPerCall + connectTimeout + readTimeout
+ Duration readTimeout = clientTimeout.minus(IN_PROCESS_OVERHEAD_PER_CALL).minus(CONNECT_TIMEOUT);
+ verifyPositive(timeLeft, readTimeout);
+
+ return readTimeout;
+ }
+
+ public Duration getServerTimeoutOrThrow() {
+ // readTimeout = networkOverhead + serverTimeout
+ 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());
+ }
+
+ return serverTimeout;
+ }
+
+ private static Duration min(Duration a, Duration b) {
+ return a.compareTo(b) < 0 ? a : b;
+ }
+
+ private void verifyLargerThan(Duration timeLeft, Duration availableDuration) {
+ if (availableDuration.toMillis() <= 0) {
+ throw new UncheckedTimeoutException("Too little time left (" + timeLeft +
+ ") to call content cluster '" + clusterName +
+ "', original timeout was " + timeBudget.originalTimeout().get());
+ }
+ }
+
+ private void verifyPositive(Duration timeLeft, Duration duration) { verifyLargerThan(timeLeft, duration); }
+}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java
index 5571eedeec6..7a0b96c2231 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java
@@ -8,9 +8,8 @@ import com.yahoo.vespa.jaxrs.client.JaxRsStrategy;
import com.yahoo.vespa.jaxrs.client.JaxRsStrategyFactory;
import com.yahoo.vespa.jaxrs.client.JerseyJaxRsClientFactory;
+import java.util.HashSet;
import java.util.List;
-import java.util.Set;
-import java.util.stream.Collectors;
/**
* @author bakksjo
@@ -21,14 +20,12 @@ public class RetryingClusterControllerClientFactory implements ClusterController
public static final int HARDCODED_CLUSTERCONTROLLER_PORT = 19050;
public static final String CLUSTERCONTROLLER_API_PATH = "/";
public static final String CLUSTERCONTROLLER_SCHEME = "http";
- private static final int CLUSTER_CONTROLLER_CONNECT_TIMEOUT_MS = 1000;
- private static final int CLUSTER_CONTROLLER_READ_TIMEOUT_MS = 1000;
private JaxRsClientFactory jaxRsClientFactory;
@Inject
public RetryingClusterControllerClientFactory() {
- this(new JerseyJaxRsClientFactory(CLUSTER_CONTROLLER_CONNECT_TIMEOUT_MS, CLUSTER_CONTROLLER_READ_TIMEOUT_MS));
+ this(new JerseyJaxRsClientFactory());
}
public RetryingClusterControllerClientFactory(JaxRsClientFactory jaxRsClientFactory) {
@@ -36,19 +33,20 @@ public class RetryingClusterControllerClientFactory implements ClusterController
}
@Override
- public ClusterControllerClient createClient(List<HostName> clusterControllers,
- String clusterName) {
- Set<HostName> clusterControllerSet = clusterControllers.stream().collect(Collectors.toSet());
- JaxRsStrategy<ClusterControllerJaxRsApi> jaxRsApi
- = new JaxRsStrategyFactory(clusterControllerSet, HARDCODED_CLUSTERCONTROLLER_PORT, jaxRsClientFactory, CLUSTERCONTROLLER_SCHEME)
- .apiWithRetries(ClusterControllerJaxRsApi.class, CLUSTERCONTROLLER_API_PATH)
- // Use max iteration 1. The JaxRsStrategyFactory will try host 1, 2, then 3:
- // - If host 1 responds, it will redirect to master if necessary. Otherwise
- // - If host 2 responds, it will redirect to master if necessary. Otherwise
- // - If host 3 responds, it may redirect to master if necessary (if they're up
- // after all), but more likely there's no quorum and this will fail too.
- .setMaxIterations(1);
+ public ClusterControllerClient createClient(List<HostName> clusterControllers, String clusterName) {
+ JaxRsStrategy<ClusterControllerJaxRsApi> jaxRsApi =
+ new JaxRsStrategyFactory(
+ new HashSet<>(clusterControllers),
+ HARDCODED_CLUSTERCONTROLLER_PORT,
+ jaxRsClientFactory,
+ CLUSTERCONTROLLER_SCHEME)
+ .apiWithRetries(ClusterControllerJaxRsApi.class, CLUSTERCONTROLLER_API_PATH)
+ // Use max iteration 1. The JaxRsStrategyFactory will try host 1, 2, then 3:
+ // - If host 1 responds, it will redirect to master if necessary. Otherwise
+ // - If host 2 responds, it will redirect to master if necessary. Otherwise
+ // - If host 3 responds, it may redirect to master if necessary (if they're up
+ // after all), but more likely there's no quorum and this will fail too.
+ .setMaxIterations(1);
return new ClusterControllerClientImpl(jaxRsApi, clusterName);
}
-
}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactory.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactory.java
deleted file mode 100644
index 7459f0a6b11..00000000000
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactory.java
+++ /dev/null
@@ -1,54 +0,0 @@
-// 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.log.LogLevel;
-import com.yahoo.vespa.applicationmodel.HostName;
-import com.yahoo.vespa.jaxrs.client.JaxRsClientFactory;
-import com.yahoo.vespa.jaxrs.client.JaxRsStrategy;
-import com.yahoo.vespa.jaxrs.client.NoRetryJaxRsStrategy;
-
-import java.util.List;
-import java.util.logging.Logger;
-
-/**
- * @author bakksjo
- */
-public class SingleInstanceClusterControllerClientFactory implements ClusterControllerClientFactory {
-
- public static final int CLUSTERCONTROLLER_HARDCODED_PORT = 19050;
- public static final String CLUSTERCONTROLLER_HARDCODED_SCHEME = "http";
- public static final String CLUSTERCONTROLLER_API_PATH = "/";
-
- private static final Logger log = Logger.getLogger(SingleInstanceClusterControllerClientFactory.class.getName());
-
- private JaxRsClientFactory jaxRsClientFactory;
-
- public SingleInstanceClusterControllerClientFactory(JaxRsClientFactory jaxRsClientFactory) {
- this.jaxRsClientFactory = jaxRsClientFactory;
- }
-
- @Override
- public ClusterControllerClient createClient(List<HostName> clusterControllers,
- String clusterName) {
- if (clusterControllers.isEmpty()) {
- throw new IllegalArgumentException("No cluster controller instances found");
- }
- HostName controllerHostName = clusterControllers.iterator().next();
- int port = CLUSTERCONTROLLER_HARDCODED_PORT; // TODO: Get this from service monitor.
-
- log.log(LogLevel.DEBUG, () ->
- "For cluster '" + clusterName + "' with controllers " + clusterControllers
- + ", creating api client for " + controllerHostName.s() + ":" + port);
-
- JaxRsStrategy<ClusterControllerJaxRsApi> strategy = new NoRetryJaxRsStrategy<>(
- controllerHostName,
- port,
- jaxRsClientFactory,
- ClusterControllerJaxRsApi.class,
- CLUSTERCONTROLLER_API_PATH,
- CLUSTERCONTROLLER_HARDCODED_SCHEME);
-
- return new ClusterControllerClientImpl(strategy, clusterName);
- }
-
-}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/NodeGroup.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/NodeGroup.java
index ed506c82079..74b4b534acc 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/NodeGroup.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/NodeGroup.java
@@ -43,7 +43,7 @@ public class NodeGroup {
}
public List<HostName> getHostNames() {
- return hostNames.stream().collect(Collectors.toList()).stream().sorted().collect(Collectors.toList());
+ return hostNames.stream().sorted().collect(Collectors.toList());
}
public String toCommaSeparatedString() {
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java
index c5ae553a98c..bd5eb6f3e29 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.orchestrator.status;
import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
import com.yahoo.vespa.applicationmodel.HostName;
+import java.time.Duration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -42,15 +43,11 @@ public class InMemoryStatusService implements StatusService {
};
}
- @Override
- public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference applicationInstanceReference) {
- return lockApplicationInstance_forCurrentThreadOnly(applicationInstanceReference, 10);
- }
@Override
public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(
ApplicationInstanceReference applicationInstanceReference,
- long timeoutSeconds) {
+ Duration timeout) {
Lock lock = instanceLockService.get(applicationInstanceReference);
return new InMemoryMutableStatusRegistry(lock, applicationInstanceReference);
}
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 c47be096242..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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.orchestrator.status;
import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
+import java.time.Duration;
import java.util.Set;
/**
@@ -24,7 +25,7 @@ public interface StatusService {
* possibly inconsistent snapshot values. It is not recommended that this method is used for anything other
* than monitoring, logging, debugging, etc. It should never be used for multi-step operations (e.g.
* read-then-write) where consistency is required. For those cases, use
- * {@link #lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference)}.
+ * {@link #lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference, Duration)}.
*/
ReadOnlyStatusRegistry forApplicationInstance(ApplicationInstanceReference applicationInstanceReference);
@@ -52,12 +53,9 @@ public interface StatusService {
* this case, subsequent mutating operations will fail, but previous mutating operations are NOT rolled back.
* This may leave the registry in an inconsistent state (as judged by the client code).
*/
- MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference applicationInstanceReference);
-
- /** Lock application instance with timeout. */
MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(
ApplicationInstanceReference applicationInstanceReference,
- long timeoutSeconds);
+ Duration timeout);
/**
* Returns all application instances that are allowed to be down. The intention is to use this
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
index deece6a4a65..7df29e038c1 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
@@ -56,21 +56,6 @@ public class ZookeeperStatusService implements StatusService {
};
}
- /**
- * 1) locks the status service for an application instance.
- * 2) fails all operations in this thread when the session is lost,
- * since session loss might cause the lock to be lost.
- * Since it only fails operations in this thread,
- * all operations depending on a lock, including the locking itself, must be done in this thread.
- * Note that since it is the thread that fails, all status operations in this thread will fail
- * even if they're not supposed to be guarded by this lock
- * (i.e. the request is for another applicationInstanceReference)
- */
- @Override
- public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference applicationInstanceReference) {
- return lockApplicationInstance_forCurrentThreadOnly(applicationInstanceReference, 10);
- }
-
@Override
public Set<ApplicationInstanceReference> getAllSuspendedApplications() {
try {
@@ -93,13 +78,23 @@ public class ZookeeperStatusService implements StatusService {
}
}
+ /**
+ * 1) locks the status service for an application instance.
+ * 2) fails all operations in this thread when the session is lost,
+ * since session loss might cause the lock to be lost.
+ * Since it only fails operations in this thread,
+ * all operations depending on a lock, including the locking itself, must be done in this thread.
+ * Note that since it is the thread that fails, all status operations in this thread will fail
+ * even if they're not supposed to be guarded by this lock
+ * (i.e. the request is for another applicationInstanceReference)
+ */
@Override
public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(
ApplicationInstanceReference applicationInstanceReference,
- long timeoutSeconds) {
+ Duration timeout) {
String lockPath = applicationInstanceLock2Path(applicationInstanceReference);
Lock lock = new Lock(lockPath, curator);
- lock.acquire(Duration.ofSeconds(timeoutSeconds));
+ lock.acquire(timeout);
try {
return new ZkMutableStatusRegistry(lock, applicationInstanceReference);
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..80174d05a54 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -43,6 +43,8 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
@@ -245,6 +247,8 @@ public class OrchestratorImplTest {
// A spy is preferential because suspendAll() relies on delegating the hard work to suspend() and resume().
OrchestratorImpl orchestrator = spy(this.orchestrator);
+ OrchestratorContext context = mock(OrchestratorContext.class);
+
orchestrator.suspendAll(
new HostName("parentHostname"),
Arrays.asList(
@@ -257,9 +261,9 @@ public class OrchestratorImplTest {
// TEST6: tenant-id-3:application-instance-3:default
// TEST1: test-tenant-id:application:instance
InOrder order = inOrder(orchestrator);
- order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST3_NODE_GROUP);
- order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST6_NODE_GROUP);
- order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST1_NODE_GROUP);
+ order.verify(orchestrator).suspendGroup(any(), eq(DummyInstanceLookupService.TEST3_NODE_GROUP));
+ order.verify(orchestrator).suspendGroup(any(), eq(DummyInstanceLookupService.TEST6_NODE_GROUP));
+ order.verify(orchestrator).suspendGroup(any(), eq(DummyInstanceLookupService.TEST1_NODE_GROUP));
order.verifyNoMoreInteractions();
}
@@ -272,7 +276,7 @@ public class OrchestratorImplTest {
DummyInstanceLookupService.TEST6_HOST_NAME,
"some-constraint",
"error message");
- doThrow(supensionFailure).when(orchestrator).suspendGroup(DummyInstanceLookupService.TEST6_NODE_GROUP);
+ doThrow(supensionFailure).when(orchestrator).suspendGroup(any(), eq(DummyInstanceLookupService.TEST6_NODE_GROUP));
try {
orchestrator.suspendAll(
@@ -291,8 +295,8 @@ public class OrchestratorImplTest {
}
InOrder order = inOrder(orchestrator);
- order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST3_NODE_GROUP);
- order.verify(orchestrator).suspendGroup(DummyInstanceLookupService.TEST6_NODE_GROUP);
+ order.verify(orchestrator).suspendGroup(any(), eq(DummyInstanceLookupService.TEST3_NODE_GROUP));
+ order.verify(orchestrator).suspendGroup(any(), eq(DummyInstanceLookupService.TEST6_NODE_GROUP));
order.verifyNoMoreInteractions();
}
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 228174a9b3d..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
@@ -6,7 +6,9 @@ import com.yahoo.vespa.jaxrs.client.LocalPassThroughJaxRsStrategy;
import com.yahoo.vespa.orchestrator.OrchestratorContext;
import org.junit.Test;
-import static org.mockito.Matchers.anyFloat;
+import java.time.Duration;
+
+import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -28,7 +30,9 @@ public class ClusterControllerClientTest {
final ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE;
OrchestratorContext context = mock(OrchestratorContext.class);
- when(context.getSuboperationTimeoutInSeconds(anyFloat())).thenReturn(1.0f);
+ ClusterControllerClientTimeouts timeouts = mock(ClusterControllerClientTimeouts.class);
+ when(context.getClusterControllerTimeouts(any())).thenReturn(timeouts);
+ 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
new file mode 100644
index 00000000000..9fe65447ac1
--- /dev/null
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java
@@ -0,0 +1,151 @@
+package com.yahoo.vespa.orchestrator.controller;
+
+import com.google.common.util.concurrent.UncheckedTimeoutException;
+import com.yahoo.test.ManualClock;
+import com.yahoo.time.TimeBudget;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.Optional;
+
+import static com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts.CONNECT_TIMEOUT;
+import static com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts.IN_PROCESS_OVERHEAD;
+import static com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts.IN_PROCESS_OVERHEAD_PER_CALL;
+import static com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts.MIN_SERVER_TIMEOUT;
+import static com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts.NETWORK_OVERHEAD_PER_CALL;
+import static com.yahoo.vespa.orchestrator.controller.ClusterControllerClientTimeouts.NUM_CALLS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class ClusterControllerClientTimeoutsTest {
+ // The minimum time left for any invocation of prepareForImmediateJaxRsCall().
+ private static final Duration MINIMUM_TIME_LEFT = IN_PROCESS_OVERHEAD_PER_CALL
+ .plus(CONNECT_TIMEOUT)
+ .plus(NETWORK_OVERHEAD_PER_CALL)
+ .plus(MIN_SERVER_TIMEOUT);
+ static {
+ assertEquals(Duration.ofMillis(160), MINIMUM_TIME_LEFT);
+ }
+
+ // The minimum time left (= original time) which is required to allow any requests to the CC.
+ private static final Duration MINIMUM_ORIGINAL_TIMEOUT = MINIMUM_TIME_LEFT
+ .multipliedBy(NUM_CALLS)
+ .plus(IN_PROCESS_OVERHEAD);
+ static {
+ assertEquals(Duration.ofMillis(420), MINIMUM_ORIGINAL_TIMEOUT);
+ }
+
+ private final ManualClock clock = new ManualClock();
+
+ private Duration originalTimeout;
+ private TimeBudget timeBudget;
+ private ClusterControllerClientTimeouts timeouts;
+
+ private void makeTimeouts(Duration originalTimeout) {
+ this.originalTimeout = originalTimeout;
+ this.timeBudget = TimeBudget.from(clock, clock.instant(), Optional.of(originalTimeout));
+ this.timeouts = new ClusterControllerClientTimeouts("clustername", timeBudget);
+ }
+
+ @Before
+ public void setUp() {
+ makeTimeouts(Duration.ofSeconds(3));
+ }
+
+ @Test
+ public void makes2RequestsWithMaxProcessingTime() {
+ assertStandardTimeouts();
+
+ Duration maxProcessingTime = IN_PROCESS_OVERHEAD_PER_CALL
+ .plus(CONNECT_TIMEOUT)
+ .plus(timeouts.getReadTimeoutOrThrow());
+ assertEquals(1450, maxProcessingTime.toMillis());
+ clock.advance(maxProcessingTime);
+
+ assertStandardTimeouts();
+
+ clock.advance(maxProcessingTime);
+
+ try {
+ timeouts.getServerTimeoutOrThrow();
+ fail();
+ } catch (UncheckedTimeoutException e) {
+ assertEquals(
+ "Too little time left (PT0.1S) to call content cluster 'clustername', original timeout was PT3S",
+ e.getMessage());
+ }
+ }
+
+ @Test
+ public void makesAtLeast3RequestsWithShortProcessingTime() {
+ assertStandardTimeouts();
+
+ Duration shortProcessingTime = Duration.ofMillis(200);
+ clock.advance(shortProcessingTime);
+
+ assertStandardTimeouts();
+
+ clock.advance(shortProcessingTime);
+
+ assertStandardTimeouts();
+ }
+
+ private void assertStandardTimeouts() {
+ assertEquals(Duration.ofMillis(50), timeouts.getConnectTimeoutOrThrow());
+ assertEquals(Duration.ofMillis(1350), timeouts.getReadTimeoutOrThrow());
+ assertEquals(Duration.ofMillis(1300), timeouts.getServerTimeoutOrThrow());
+ }
+
+ @Test
+ public void alreadyTimedOut() {
+ clock.advance(Duration.ofSeconds(4));
+
+ try {
+ timeouts.getServerTimeoutOrThrow();
+ fail();
+ } catch (UncheckedTimeoutException e) {
+ assertEquals(
+ "Exceeded the timeout PT3S against content cluster 'clustername' by PT1S",
+ e.getMessage());
+ }
+ }
+
+ @Test
+ public void justTooLittleTime() {
+ clock.advance(originalTimeout.minus(MINIMUM_TIME_LEFT).plus(Duration.ofMillis(1)));
+ try {
+ timeouts.getServerTimeoutOrThrow();
+ fail();
+ } catch (UncheckedTimeoutException e) {
+ assertEquals(
+ "Server would be given too little time to complete: PT0.009S. Original timeout was PT3S",
+ e.getMessage());
+ }
+ }
+
+ @Test
+ public void justEnoughTime() {
+ clock.advance(originalTimeout.minus(MINIMUM_TIME_LEFT));
+ timeouts.getServerTimeoutOrThrow();
+ }
+
+ @Test
+ public void justTooLittleInitialTime() {
+ makeTimeouts(MINIMUM_ORIGINAL_TIMEOUT.minus(Duration.ofMillis(1)));
+ try {
+ timeouts.getServerTimeoutOrThrow();
+ fail();
+ } catch (UncheckedTimeoutException e) {
+ assertEquals(
+ "Server would be given too little time to complete: PT0.0095S. Original timeout was PT0.419S",
+ e.getMessage());
+ }
+ }
+
+ @Test
+ public void justEnoughInitialTime() {
+ makeTimeouts(MINIMUM_ORIGINAL_TIMEOUT);
+ timeouts.getServerTimeoutOrThrow();
+ }
+} \ No newline at end of file
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java
new file mode 100644
index 00000000000..1f505991476
--- /dev/null
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java
@@ -0,0 +1,54 @@
+package com.yahoo.vespa.orchestrator.controller;
+
+import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.applicationmodel.HostName;
+import com.yahoo.vespa.jaxrs.client.JaxRsClientFactory;
+import com.yahoo.vespa.orchestrator.OrchestratorContext;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.io.IOException;
+import java.time.Clock;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class RetryingClusterControllerClientFactoryTest {
+ private final Clock clock = new ManualClock();
+
+ @Test
+ public void verifyJerseyCallForSetNodeState() throws IOException {
+ JaxRsClientFactory clientFactory = mock(JaxRsClientFactory.class);
+ ClusterControllerJaxRsApi api = mock(ClusterControllerJaxRsApi.class);
+ when(clientFactory.createClient(any())).thenReturn(api);
+ RetryingClusterControllerClientFactory factory = new RetryingClusterControllerClientFactory(clientFactory);
+ String clusterName = "clustername";
+ HostName host1 = new HostName("host1");
+ HostName host2 = new HostName("host2");
+ HostName host3 = new HostName("host3");
+ List<HostName> clusterControllers = Arrays.asList(host1, host2, host3);
+ ClusterControllerClient client = factory.createClient(clusterControllers, clusterName);
+ OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
+ int storageNode = 2;
+ ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE;
+ client.setNodeState(context, storageNode, wantedState);
+
+ ArgumentCaptor<ClusterControllerStateRequest> requestCaptor = ArgumentCaptor.forClass(ClusterControllerStateRequest.class);
+
+ verify(api, times(1)).setNodeState(eq(clusterName), eq(storageNode), eq(4.8f), requestCaptor.capture());
+ ClusterControllerStateRequest request = requestCaptor.getValue();
+ assertEquals(ClusterControllerStateRequest.Condition.SAFE, request.condition);
+ Map<String, Object> expectedState = new HashMap<>();
+ expectedState.put("user", new ClusterControllerStateRequest.State(wantedState, "Orchestrator"));
+ assertEquals(expectedState, request.state);
+ }
+} \ No newline at end of file
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
deleted file mode 100644
index 4dabae14add..00000000000
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java
+++ /dev/null
@@ -1,116 +0,0 @@
-// 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.vespa.applicationmodel.ConfigId;
-import com.yahoo.vespa.applicationmodel.HostName;
-import com.yahoo.vespa.jaxrs.client.JaxRsClientFactory;
-import com.yahoo.vespa.orchestrator.OrchestratorContext;
-import org.junit.Before;
-import org.junit.Test;
-
-import java.util.Arrays;
-import java.util.List;
-
-import static org.hamcrest.CoreMatchers.anyOf;
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.fail;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyFloat;
-import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.argThat;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-public class SingleInstanceClusterControllerClientFactoryTest {
- private static final int PORT = SingleInstanceClusterControllerClientFactory.CLUSTERCONTROLLER_HARDCODED_PORT;
- private static final String PATH = SingleInstanceClusterControllerClientFactory.CLUSTERCONTROLLER_API_PATH;
-
- private static final HostName HOST_NAME_1 = new HostName("host1");
- private static final HostName HOST_NAME_2 = new HostName("host2");
- private static final HostName HOST_NAME_3 = new HostName("host3");
-
- OrchestratorContext context = mock(OrchestratorContext.class);
- private final ClusterControllerJaxRsApi mockApi = mock(ClusterControllerJaxRsApi.class);
- private final JaxRsClientFactory jaxRsClientFactory = mock(JaxRsClientFactory.class);
- private final ClusterControllerClientFactory clientFactory
- = new SingleInstanceClusterControllerClientFactory(jaxRsClientFactory);
-
- @Before
- public void setup() {
- when(
- jaxRsClientFactory.createClient(
- eq(ClusterControllerJaxRsApi.class),
- any(HostName.class),
- anyInt(),
- anyString(),
- anyString()))
- .thenReturn(mockApi);
- }
-
- @Test
- public void testCreateClientWithNoClusterControllerInstances() throws Exception {
- final List<HostName> clusterControllers = Arrays.asList();
-
- try {
- clientFactory.createClient(clusterControllers, "clusterName");
- fail();
- } catch (IllegalArgumentException e) {
- // As expected.
- }
- }
-
- @Test
- public void testCreateClientWithSingleClusterControllerInstance() throws Exception {
- final List<HostName> clusterControllers = Arrays.asList(HOST_NAME_1);
-
- when(context.getSuboperationTimeoutInSeconds(anyFloat())).thenReturn(1.0f);
- clientFactory.createClient(clusterControllers, "clusterName")
- .setNodeState(context, 0, ClusterControllerNodeState.MAINTENANCE);
-
- verify(jaxRsClientFactory).createClient(
- ClusterControllerJaxRsApi.class,
- HOST_NAME_1,
- PORT,
- PATH,
- "http");
- }
-
- @Test
- public void testCreateClientWithoutClusterControllerInstances() throws Exception {
- final List<HostName> clusterControllers = Arrays.asList();
-
- try {
- clientFactory.createClient(clusterControllers, "clusterName");
- fail();
- } catch (IllegalArgumentException e) {
- // As expected.
- }
- }
-
- @Test
- public void testCreateClientWithThreeClusterControllerInstances() throws Exception {
- final List<HostName> clusterControllers = Arrays.asList(HOST_NAME_1, HOST_NAME_2, HOST_NAME_3);
-
- when(context.getSuboperationTimeoutInSeconds(anyFloat())).thenReturn(1.0f);
- clientFactory.createClient(clusterControllers, "clusterName")
- .setNodeState(context, 0, ClusterControllerNodeState.MAINTENANCE);
-
- verify(jaxRsClientFactory).createClient(
- eq(ClusterControllerJaxRsApi.class),
- argThat(is(anyOf(
- equalTo(HOST_NAME_1),
- equalTo(HOST_NAME_2),
- equalTo(HOST_NAME_3)))),
- eq(PORT),
- eq(PATH),
- eq("http"));
- }
-
- private static ConfigId clusterControllerConfigId(final int index) {
- return new ConfigId("admin/cluster-controllers/" + index);
- }
-}
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..a9b8127e7fe 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
@@ -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.resources;
-import com.yahoo.jdisc.Timer;
import com.yahoo.vespa.applicationmodel.ApplicationInstance;
import com.yahoo.vespa.applicationmodel.ApplicationInstanceId;
import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
@@ -32,6 +31,7 @@ import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
import com.yahoo.vespa.orchestrator.status.HostStatus;
import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry;
import com.yahoo.vespa.orchestrator.status.StatusService;
+import org.junit.Before;
import org.junit.Test;
import javax.ws.rs.BadRequestException;
@@ -41,6 +41,7 @@ import javax.ws.rs.core.UriBuilder;
import javax.ws.rs.core.UriInfo;
import java.net.URI;
import java.time.Clock;
+import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
import java.util.Optional;
@@ -74,7 +75,7 @@ public class HostResourceTest {
static {
when(EVERY_HOST_IS_UP_HOST_STATUS_SERVICE.forApplicationInstance(eq(APPLICATION_INSTANCE_REFERENCE)))
.thenReturn(EVERY_HOST_IS_UP_MUTABLE_HOST_STATUS_REGISTRY);
- when(EVERY_HOST_IS_UP_HOST_STATUS_SERVICE.lockApplicationInstance_forCurrentThreadOnly(eq(APPLICATION_INSTANCE_REFERENCE)))
+ when(EVERY_HOST_IS_UP_HOST_STATUS_SERVICE.lockApplicationInstance_forCurrentThreadOnly(eq(APPLICATION_INSTANCE_REFERENCE), any()))
.thenReturn(EVERY_HOST_IS_UP_MUTABLE_HOST_STATUS_REGISTRY);
when(EVERY_HOST_IS_UP_MUTABLE_HOST_STATUS_REGISTRY.getHostStatus(any()))
.thenReturn(HostStatus.NO_REMARKS);
@@ -150,6 +151,11 @@ public class HostResourceTest {
private final UriInfo uriInfo = mock(UriInfo.class);
+ @Before
+ public void setUp() {
+ when(clock.instant()).thenReturn(Instant.now());
+ }
+
@Test
public void returns_200_on_success() {
HostResource hostResource =
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusServiceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusServiceTest.java
index 2e914718e20..44847666670 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusServiceTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusServiceTest.java
@@ -17,6 +17,7 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -81,7 +82,8 @@ public class ZookeeperStatusServiceTest {
@Test
public void setting_host_state_is_idempotent() {
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE,
+ Duration.ofSeconds(10))) {
//shuffling to catch "clean database" failures for all cases.
for (HostStatus hostStatus: shuffledList(HostStatus.values())) {
@@ -105,11 +107,12 @@ public class ZookeeperStatusServiceTest {
final CompletableFuture<Void> lockedSuccessfullyFuture;
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE,
+ Duration.ofSeconds(10))) {
lockedSuccessfullyFuture = CompletableFuture.runAsync(() -> {
try (MutableStatusRegistry statusRegistry2 = zookeeperStatusService2
- .lockApplicationInstance_forCurrentThreadOnly(TestIds.APPLICATION_INSTANCE_REFERENCE))
+ .lockApplicationInstance_forCurrentThreadOnly(TestIds.APPLICATION_INSTANCE_REFERENCE, Duration.ofSeconds(10)))
{
}
});
@@ -131,13 +134,13 @@ public class ZookeeperStatusServiceTest {
ZookeeperStatusService zookeeperStatusService2 = new ZookeeperStatusService(curator);
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE, Duration.ofSeconds(10))) {
//must run in separate thread, since having 2 locks in the same thread fails
CompletableFuture<Void> resultOfZkOperationAfterLockFailure = CompletableFuture.runAsync(() -> {
try {
zookeeperStatusService2.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE,1);
+ TestIds.APPLICATION_INSTANCE_REFERENCE, Duration.ofSeconds(1));
fail("Both zookeeper host status services locked simultaneously for the same application instance");
} catch (RuntimeException e) {
}
@@ -211,7 +214,7 @@ public class ZookeeperStatusServiceTest {
// Suspend
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE, Duration.ofSeconds(10))) {
statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
}
@@ -223,7 +226,7 @@ public class ZookeeperStatusServiceTest {
// Resume
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE, Duration.ofSeconds(10))) {
statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.NO_REMARKS);
}
@@ -241,12 +244,12 @@ public class ZookeeperStatusServiceTest {
assertThat(suspendedApps.size(), is(0));
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE, Duration.ofSeconds(10))) {
statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
}
try (MutableStatusRegistry statusRegistry = zookeeperStatusService.lockApplicationInstance_forCurrentThreadOnly(
- TestIds.APPLICATION_INSTANCE_REFERENCE2)) {
+ TestIds.APPLICATION_INSTANCE_REFERENCE2, Duration.ofSeconds(10))) {
statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
}
diff --git a/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java b/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java
index a7963df0208..449b0d6bd05 100644
--- a/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java
+++ b/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java
@@ -66,6 +66,18 @@ public class TimeBudget {
});
}
+ /** Returns the time left, possibly negative if the deadline has passed. */
+ public Optional<Duration> timeLeft() {
+ return timeout.map(timeout -> timeout.minus(timePassed()));
+ }
+
+ /** Returns the time left as a new TimeBudget. */
+ public TimeBudget timeLeftAsTimeBudget() {
+ Instant now = clock.instant();
+ Optional<Instant> deadline = deadline();
+ return new TimeBudget(clock, now, deadline.map(d -> Duration.between(now, d)));
+ }
+
private static Duration nonNegativeBetween(Instant start, Instant end) {
return makeNonNegative(Duration.between(start, end));
}