summaryrefslogtreecommitdiffstats
path: root/orchestrator
diff options
context:
space:
mode:
authorHåkon Hallingstad <hakon@verizonmedia.com>2020-03-02 14:38:00 +0100
committerHåkon Hallingstad <hakon@verizonmedia.com>2020-03-02 14:38:00 +0100
commit09486a9adff55af8a615dd51cbb230bf64653c1c (patch)
tree8bb3dec71115289a2bb633d19d3f218bd08a7d71 /orchestrator
parent4b6a2a891680dd778ccd5cf8761a888d44b3421a (diff)
Rename MutableStatusService to ApplicationLock
The result of acquiring the application lock in the status service is now named ApplicationLock instead of MutableStatusService.
Diffstat (limited to 'orchestrator')
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java69
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiFactory.java6
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java14
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java6
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java4
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ApplicationLock.java36
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusService.java36
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java21
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkApplicationLock.java (renamed from orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkMutableStatusService.java)25
-rw-r--r--orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkStatusService.java11
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java15
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java6
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java6
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusService2Test.java8
-rw-r--r--orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java31
15 files changed, 136 insertions, 158 deletions
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 964d2875496..be1f83da094 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -34,7 +34,7 @@ import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
import com.yahoo.vespa.orchestrator.status.HostInfo;
import com.yahoo.vespa.orchestrator.status.HostInfos;
import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
import com.yahoo.vespa.orchestrator.status.StatusService;
import java.io.IOException;
@@ -74,14 +74,16 @@ public class OrchestratorImpl implements Orchestrator {
ConfigserverConfig configServerConfig,
FlagSource flagSource)
{
- this(new HostedVespaPolicy(new HostedVespaClusterPolicy(), clusterControllerClientFactory, new ApplicationApiFactory(configServerConfig.zookeeperserver().size())),
- clusterControllerClientFactory,
- statusService,
- instanceLookupService,
- orchestratorConfig.serviceMonitorConvergenceLatencySeconds(),
- Clock.systemUTC(),
- new ApplicationApiFactory(configServerConfig.zookeeperserver().size()),
- flagSource);
+ this(new HostedVespaPolicy(new HostedVespaClusterPolicy(),
+ clusterControllerClientFactory,
+ new ApplicationApiFactory(configServerConfig.zookeeperserver().size())),
+ clusterControllerClientFactory,
+ statusService,
+ instanceLookupService,
+ orchestratorConfig.serviceMonitorConvergenceLatencySeconds(),
+ Clock.systemUTC(),
+ new ApplicationApiFactory(configServerConfig.zookeeperserver().size()),
+ flagSource);
}
public OrchestratorImpl(Policy policy,
@@ -127,17 +129,17 @@ public class OrchestratorImpl implements Orchestrator {
@Override
public Function<HostName, Optional<HostInfo>> getHostResolver() {
- return hostName -> instanceLookupService.findInstanceByHost(hostName)
- .map(application -> statusService.getHostInfo(application.reference(), hostName));
+ return hostName -> instanceLookupService
+ .findInstanceByHost(hostName)
+ .map(application -> statusService.getHostInfo(application.reference(), hostName));
}
@Override
public void setNodeStatus(HostName hostName, HostStatus status) throws OrchestrationException {
ApplicationInstanceReference reference = getApplicationInstance(hostName).reference();
OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
- try (MutableStatusService statusRegistry = statusService
- .lockApplication(context, reference)) {
- statusRegistry.setHostState(hostName, status);
+ try (ApplicationLock lock = statusService.lockApplication(context, reference)) {
+ lock.setHostState(hostName, status);
}
}
@@ -165,9 +167,8 @@ public class OrchestratorImpl implements Orchestrator {
ApplicationInstance appInstance = getApplicationInstance(hostName);
OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
- try (MutableStatusService statusRegistry = statusService
- .lockApplication(context, appInstance.reference())) {
- HostStatus currentHostState = statusRegistry.getHostInfos().getOrNoRemarks(hostName).status();
+ try (ApplicationLock lock = statusService.lockApplication(context, appInstance.reference())) {
+ HostStatus currentHostState = lock.getHostInfos().getOrNoRemarks(hostName).status();
if (currentHostState == HostStatus.NO_REMARKS) {
return;
}
@@ -178,11 +179,11 @@ public class OrchestratorImpl implements Orchestrator {
// 2. The whole application is down: the content cluster states are set to maintenance,
// and the host may be taken down manually at any moment.
if (currentHostState == HostStatus.PERMANENTLY_DOWN ||
- statusRegistry.getStatus() == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) {
+ lock.getApplicationInstanceStatus() == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) {
return;
}
- policy.releaseSuspensionGrant(context.createSubcontextWithinLock(), appInstance, hostName, statusRegistry);
+ policy.releaseSuspensionGrant(context.createSubcontextWithinLock(), appInstance, hostName, lock);
}
}
@@ -202,10 +203,8 @@ public class OrchestratorImpl implements Orchestrator {
.with(FetchVector.Dimension.HOSTNAME, hostName.s())
.value();
OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock, usePermanentlyDownStatus);
- try (MutableStatusService statusRegistry = statusService
- .lockApplication(context, appInstance.reference())) {
- ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, statusRegistry,
- clusterControllerClientFactory);
+ try (ApplicationLock lock = statusService.lockApplication(context, appInstance.reference())) {
+ ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, lock, clusterControllerClientFactory);
policy.acquirePermissionToRemove(context.createSubcontextWithinLock(), applicationApi);
}
@@ -220,23 +219,22 @@ public class OrchestratorImpl implements Orchestrator {
void suspendGroup(OrchestratorContext context, NodeGroup nodeGroup) throws HostStateChangeDeniedException {
ApplicationInstanceReference applicationReference = nodeGroup.getApplicationReference();
- try (MutableStatusService hostStatusRegistry =
- statusService.lockApplication(context, applicationReference)) {
- ApplicationInstanceStatus appStatus = hostStatusRegistry.getStatus();
+ try (ApplicationLock lock = statusService.lockApplication(context, applicationReference)) {
+ ApplicationInstanceStatus appStatus = lock.getApplicationInstanceStatus();
if (appStatus == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) {
return;
}
- ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, hostStatusRegistry,
- clusterControllerClientFactory);
+ ApplicationApi applicationApi = applicationApiFactory.create(
+ nodeGroup, lock, clusterControllerClientFactory);
policy.grantSuspensionRequest(context.createSubcontextWithinLock(), applicationApi);
}
}
@Override
public ApplicationInstanceStatus getApplicationInstanceStatus(ApplicationId appId) throws ApplicationIdNotFoundException {
- ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
- return statusService.getApplicationInstanceStatus(appRef);
+ ApplicationInstanceReference reference = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
+ return statusService.getApplicationInstanceStatus(reference);
}
@Override
@@ -354,17 +352,16 @@ public class OrchestratorImpl implements Orchestrator {
throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException{
OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock);
ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
- try (MutableStatusService statusRegistry =
- statusService.lockApplication(context, appRef)) {
+ try (ApplicationLock lock = statusService.lockApplication(context, appRef)) {
// Short-circuit if already in wanted state
- if (status == statusRegistry.getStatus()) return;
+ if (status == lock.getApplicationInstanceStatus()) return;
// Set content clusters for this application in maintenance on suspend
if (status == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) {
ApplicationInstance application = getApplicationInstance(appRef);
- HostInfos hostInfosSnapshot = statusRegistry.getHostInfos();
+ HostInfos hostInfosSnapshot = lock.getHostInfos();
// Mark it allowed to be down before we manipulate the clustercontroller
OrchestratorUtil.getHostsUsedByApplicationInstance(application)
@@ -372,14 +369,14 @@ public class OrchestratorImpl implements Orchestrator {
// This filter also ensures host status is not modified if a suspended host
// has status != ALLOWED_TO_BE_DOWN.
.filter(hostname -> !hostInfosSnapshot.getOrNoRemarks(hostname).status().isSuspended())
- .forEach(hostname -> statusRegistry.setHostState(hostname, HostStatus.ALLOWED_TO_BE_DOWN));
+ .forEach(hostname -> lock.setHostState(hostname, HostStatus.ALLOWED_TO_BE_DOWN));
// If the clustercontroller throws an error the nodes will be marked as allowed to be down
// and be set back up on next resume invocation.
setClusterStateInController(context.createSubcontextWithinLock(), application, ClusterControllerNodeState.MAINTENANCE);
}
- statusRegistry.setApplicationInstanceStatus(status);
+ lock.setApplicationInstanceStatus(status);
}
}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiFactory.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiFactory.java
index ef14a99d7db..fb52eed2048 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiFactory.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiFactory.java
@@ -2,7 +2,7 @@
package com.yahoo.vespa.orchestrator.model;
import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
/**
* @author mpolden
@@ -16,9 +16,9 @@ public class ApplicationApiFactory {
}
public ApplicationApi create(NodeGroup nodeGroup,
- MutableStatusService hostStatusService,
+ ApplicationLock lock,
ClusterControllerClientFactory clusterControllerClientFactory) {
- return new ApplicationApiImpl(nodeGroup, hostStatusService, clusterControllerClientFactory, numberOfConfigServers);
+ return new ApplicationApiImpl(nodeGroup, lock, clusterControllerClientFactory, numberOfConfigServers);
}
}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java
index 92867cf3258..ccc89fa9191 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java
@@ -12,7 +12,7 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory;
import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
import com.yahoo.vespa.orchestrator.status.HostInfos;
import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
import java.util.Collection;
import java.util.Comparator;
@@ -32,19 +32,19 @@ public class ApplicationApiImpl implements ApplicationApi {
private final ApplicationInstance applicationInstance;
private final NodeGroup nodeGroup;
- private final MutableStatusService hostStatusService;
+ private final ApplicationLock lock;
private final List<ClusterApi> clusterInOrder;
private final HostInfos hostInfos;
public ApplicationApiImpl(NodeGroup nodeGroup,
- MutableStatusService hostStatusService,
+ ApplicationLock lock,
ClusterControllerClientFactory clusterControllerClientFactory,
int numberOfConfigServers) {
this.applicationInstance = nodeGroup.getApplication();
this.nodeGroup = nodeGroup;
- this.hostStatusService = hostStatusService;
+ this.lock = lock;
Collection<HostName> hosts = getHostsUsedByApplicationInstance(applicationInstance);
- this.hostInfos = hostStatusService.getHostInfos();
+ this.hostInfos = lock.getHostInfos();
this.clusterInOrder = makeClustersInOrder(nodeGroup, hostInfos, clusterControllerClientFactory, numberOfConfigServers);
}
@@ -95,12 +95,12 @@ public class ApplicationApiImpl implements ApplicationApi {
@Override
public ApplicationInstanceStatus getApplicationStatus() {
- return hostStatusService.getStatus();
+ return lock.getApplicationInstanceStatus();
}
@Override
public void setHostState(OrchestratorContext context, HostName hostName, HostStatus status) {
- hostStatusService.setHostState(hostName, status);
+ lock.setHostState(hostName, status);
}
@Override
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
index 95ce34a8a61..8b74d8a40ef 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java
@@ -13,7 +13,7 @@ import com.yahoo.vespa.orchestrator.model.NodeGroup;
import com.yahoo.vespa.orchestrator.model.StorageNode;
import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus;
import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
/**
* @author oyving
@@ -113,9 +113,9 @@ public class HostedVespaPolicy implements Policy {
OrchestratorContext context,
ApplicationInstance applicationInstance,
HostName hostName,
- MutableStatusService hostStatusService) throws HostStateChangeDeniedException {
+ ApplicationLock lock) throws HostStateChangeDeniedException {
NodeGroup nodeGroup = new NodeGroup(applicationInstance, hostName);
- ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, hostStatusService, clusterControllerClientFactory);
+ ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, lock, clusterControllerClientFactory);
releaseSuspensionGrant(context, applicationApi);
}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java
index 6e7a3552828..c410cda23a8 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java
@@ -5,7 +5,7 @@ import com.yahoo.vespa.applicationmodel.ApplicationInstance;
import com.yahoo.vespa.applicationmodel.HostName;
import com.yahoo.vespa.orchestrator.OrchestratorContext;
import com.yahoo.vespa.orchestrator.model.ApplicationApi;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
/**
* @author oyving
@@ -32,6 +32,6 @@ public interface Policy {
void releaseSuspensionGrant(
OrchestratorContext context, ApplicationInstance applicationInstance,
HostName hostName,
- MutableStatusService hostStatusService) throws HostStateChangeDeniedException;
+ ApplicationLock hostStatusService) throws HostStateChangeDeniedException;
}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ApplicationLock.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ApplicationLock.java
new file mode 100644
index 00000000000..4f6c0547ed6
--- /dev/null
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ApplicationLock.java
@@ -0,0 +1,36 @@
+// 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.status;
+
+import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
+import com.yahoo.vespa.applicationmodel.HostName;
+
+/**
+ * The exclusive lock of an application in the status service, and methods to mutate host status
+ * and data structures guarded by such a lock.
+ *
+ * @author oyving
+ * @author Tony Vaagenes
+ * @author bakksjo
+ */
+public interface ApplicationLock extends AutoCloseable {
+
+ /** The reference of the locked application. */
+ ApplicationInstanceReference getApplicationInstanceReference();
+
+ /** Returns all host infos for this application. */
+ HostInfos getHostInfos();
+
+ /** Sets the state for the given host. */
+ void setHostState(HostName hostName, HostStatus status);
+
+ /** Returns the application status. */
+ ApplicationInstanceStatus getApplicationInstanceStatus();
+
+ /** Sets the orchestration status for the application instance. */
+ void setApplicationInstanceStatus(ApplicationInstanceStatus applicationInstanceStatus);
+
+ /** implNote: Must not throw an exception. */
+ @Override
+ void close();
+
+}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusService.java
deleted file mode 100644
index ad68652e968..00000000000
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusService.java
+++ /dev/null
@@ -1,36 +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.status;
-
-import com.yahoo.vespa.applicationmodel.HostName;
-
-/**
- * Registry of the suspension and host statuses for an application instance.
- *
- * @author oyving
- * @author Tony Vaagenes
- * @author bakksjo
- */
-public interface MutableStatusService extends AutoCloseable {
-
- /** Returns the status of this application. */
- ApplicationInstanceStatus getStatus();
-
- /** Returns a snapshot of all host infos for this application. */
- HostInfos getHostInfos();
-
- /** Sets the state for the given host. */
- void setHostState(HostName hostName, HostStatus status);
-
- /**
- * Sets the orchestration status for the application instance.
- */
- void setApplicationInstanceStatus(ApplicationInstanceStatus applicationInstanceStatus);
-
- /**
- * We don't want {@link AutoCloseable#close()} to throw an exception (what to do about it anyway?),
- * so we override it here to strip the exception from the signature.
- */
- @Override
- void close();
-
-}
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 346d0523333..0d4076f3d59 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
@@ -24,27 +24,8 @@ public interface StatusService {
* Returns a mutable host status registry for a locked application instance. All operations performed on
* the returned registry are executed in the context of a lock, including read operations. Hence, multi-step
* operations (e.g. read-then-write) are guaranteed to be consistent.
- *
- * Some limitations/caveats apply for certain implementations, and since clients of this API must be aware of
- * these limitations/caveats when using those implementations, they are expressed here, at interface level
- * rather than at implementation level, because the interface represents the lowest common denominator
- * of guarantees offered by implementations. Specifically, it is the zookeeper-based implementation's semantics
- * that "leak through" in this spec. Now, to the specific caveats:
- *
- * Locking this application instance only guarantees that the holder is the only one that can mutate host statuses
- * for the application instance.
- * It is _not_ safe to assume that there is only one entity holding the lock for a given application instance
- * reference at any given time.
- *
- * You cannot have multiple locks in a single thread, even if they are for different application instances,
- * (i.e. different HostStatusRegistry instances). (This is due to a limitation in SessionFailRetryLoop.)
- *
- * While read-then-write-operations are consistent (i.e. the current value doesn't change between the read
- * and the write), it is possible that the lock is lost before it is explicitly released by the code. In
- * 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).
*/
- MutableStatusService lockApplication(OrchestratorContext context, ApplicationInstanceReference reference)
+ ApplicationLock lockApplication(OrchestratorContext context, ApplicationInstanceReference reference)
throws UncheckedTimeoutException;
/**
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkMutableStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkApplicationLock.java
index e855f78a03c..08cbe1f45d7 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkMutableStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkApplicationLock.java
@@ -10,9 +10,9 @@ import org.apache.zookeeper.KeeperException;
import java.util.logging.Logger;
-class ZkMutableStatusService implements MutableStatusService {
+class ZkApplicationLock implements ApplicationLock {
- private static final Logger log = Logger.getLogger(ZkMutableStatusService.class.getName());
+ private static final Logger log = Logger.getLogger(ZkApplicationLock.class.getName());
private final ZkStatusService statusService;
private final Curator curator;
@@ -21,12 +21,12 @@ class ZkMutableStatusService implements MutableStatusService {
private final boolean probe;
private final HostInfosCache hostInfosCache;
- ZkMutableStatusService(ZkStatusService statusService,
- Curator curator,
- Runnable onClose,
- ApplicationInstanceReference reference,
- boolean probe,
- HostInfosCache hostInfosCache) {
+ ZkApplicationLock(ZkStatusService statusService,
+ Curator curator,
+ Runnable onClose,
+ ApplicationInstanceReference reference,
+ boolean probe,
+ HostInfosCache hostInfosCache) {
this.statusService = statusService;
this.curator = curator;
this.onClose = onClose;
@@ -36,7 +36,12 @@ class ZkMutableStatusService implements MutableStatusService {
}
@Override
- public ApplicationInstanceStatus getStatus() {
+ public ApplicationInstanceReference getApplicationInstanceReference() {
+ return reference;
+ }
+
+ @Override
+ public ApplicationInstanceStatus getApplicationInstanceStatus() {
return statusService.getApplicationInstanceStatus(reference);
}
@@ -77,7 +82,7 @@ class ZkMutableStatusService implements MutableStatusService {
// We may want to avoid logging some exceptions that may be expected, like when session expires.
log.log(LogLevel.WARNING,
"Failed close application lock in " +
- ZkMutableStatusService.class.getSimpleName() + ", will ignore and continue",
+ ZkApplicationLock.class.getSimpleName() + ", will ignore and continue",
e);
}
}
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkStatusService.java
index 4be629c6d21..c1ca04aea75 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZkStatusService.java
@@ -110,9 +110,9 @@ public class ZkStatusService implements StatusService {
* (i.e. the request is for another applicationInstanceReference)
*/
@Override
- public MutableStatusService lockApplication(
- OrchestratorContext context,
- ApplicationInstanceReference reference) throws UncheckedTimeoutException {
+ public ApplicationLock lockApplication(OrchestratorContext context, ApplicationInstanceReference reference)
+ throws UncheckedTimeoutException {
+
Runnable onRegistryClose;
// A multi-application operation, aka batch suspension, will first issue a probe
@@ -131,7 +131,7 @@ public class ZkStatusService implements StatusService {
}
try {
- return new ZkMutableStatusService(
+ return new ZkApplicationLock(
this,
curator,
onRegistryClose,
@@ -145,8 +145,7 @@ public class ZkStatusService implements StatusService {
}
}
- private Runnable acquireLock(OrchestratorContext context,
- ApplicationInstanceReference reference)
+ private Runnable acquireLock(OrchestratorContext context, ApplicationInstanceReference reference)
throws UncheckedTimeoutException {
ApplicationId applicationId = OrchestratorUtil.toApplicationId(reference);
String app = applicationId.application().value() + "." + applicationId.instance().value();
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 3f560a3e274..f7c5ab15927 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -27,7 +27,7 @@ import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException;
import com.yahoo.vespa.orchestrator.policy.HostedVespaClusterPolicy;
import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy;
import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
import com.yahoo.vespa.orchestrator.status.StatusService;
import com.yahoo.vespa.orchestrator.status.ZkStatusService;
import com.yahoo.vespa.service.model.ServiceModelCache;
@@ -336,13 +336,12 @@ public class OrchestratorImplTest {
var clusterControllerClientFactory = mock(ClusterControllerClientFactory.class);
var clock = new ManualClock();
var applicationApiFactory = mock(ApplicationApiFactory.class);
- var hostStatusRegistry = mock(MutableStatusService.class);
+ var lock = mock(ApplicationLock.class);
when(instanceLookupService.findInstanceByHost(any())).thenReturn(Optional.of(applicationInstance));
when(applicationInstance.reference()).thenReturn(applicationInstanceReference);
- when(zookeeperStatusService.lockApplication(any(), any()))
- .thenReturn(hostStatusRegistry);
- when(hostStatusRegistry.getStatus()).thenReturn(NO_REMARKS);
+ when(zookeeperStatusService.lockApplication(any(), any())).thenReturn(lock);
+ when(lock.getApplicationInstanceStatus()).thenReturn(NO_REMARKS);
var orchestrator = new OrchestratorImpl(
policy,
@@ -372,17 +371,17 @@ public class OrchestratorImplTest {
verify(applicationApiFactory, times(2)).create(any(), any(), any());
verify(policy, times(2)).grantSuspensionRequest(any(), any());
verify(instanceLookupService, atLeastOnce()).findInstanceByHost(any());
- verify(hostStatusRegistry, times(2)).getStatus();
+ verify(lock, times(2)).getApplicationInstanceStatus();
// Each zookeeperStatusService that is created, is closed.
verify(zookeeperStatusService, times(2)).lockApplication(any(), any());
- verify(hostStatusRegistry, times(2)).close();
+ verify(lock, times(2)).close();
verifyNoMoreInteractions(
policy,
clusterControllerClientFactory,
zookeeperStatusService,
- hostStatusRegistry,
+ lock,
instanceLookupService,
applicationApiFactory);
}
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java
index b37d3fc3850..24a85bfa244 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java
@@ -29,7 +29,7 @@ import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy;
import com.yahoo.vespa.orchestrator.status.HostInfo;
import com.yahoo.vespa.orchestrator.status.HostInfos;
import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
import com.yahoo.vespa.orchestrator.status.StatusService;
import com.yahoo.vespa.orchestrator.status.ZkStatusService;
import com.yahoo.vespa.service.model.ServiceModelCache;
@@ -114,10 +114,10 @@ class ModelTestUtils {
ApplicationInstance applicationInstance,
HostName... hostnames) {
NodeGroup nodeGroup = new NodeGroup(applicationInstance, hostnames);
- MutableStatusService registry = statusService.lockApplication(
+ ApplicationLock lock = statusService.lockApplication(
OrchestratorContext.createContextForSingleAppOp(Clock.systemUTC()),
applicationInstance.reference());
- return applicationApiFactory().create(nodeGroup, registry, clusterControllerClientFactory);
+ return applicationApiFactory().create(nodeGroup, lock, clusterControllerClientFactory);
}
ApplicationInstance createApplicationInstance(
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 df5f31c8e47..9133a3bbf2c 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
@@ -39,7 +39,7 @@ import com.yahoo.vespa.orchestrator.restapi.wire.PatchHostResponse;
import com.yahoo.vespa.orchestrator.restapi.wire.UpdateHostResponse;
import com.yahoo.vespa.orchestrator.status.HostInfo;
import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.MutableStatusService;
+import com.yahoo.vespa.orchestrator.status.ApplicationLock;
import com.yahoo.vespa.orchestrator.status.StatusService;
import com.yahoo.vespa.orchestrator.status.ZkStatusService;
import org.junit.Before;
@@ -134,7 +134,7 @@ public class HostResourceTest {
public void releaseSuspensionGrant(
OrchestratorContext context, ApplicationInstance applicationInstance,
HostName hostName,
- MutableStatusService hostStatusRegistry) {
+ ApplicationLock hostStatusRegistry) {
}
}
@@ -236,7 +236,7 @@ public class HostResourceTest {
public void releaseSuspensionGrant(
OrchestratorContext context, ApplicationInstance applicationInstance,
HostName hostName,
- MutableStatusService hostStatusRegistry) throws HostStateChangeDeniedException {
+ ApplicationLock hostStatusRegistry) throws HostStateChangeDeniedException {
doThrow();
}
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusService2Test.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusService2Test.java
index 10556a15c51..c5d390050ee 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusService2Test.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusService2Test.java
@@ -50,7 +50,7 @@ public class ZkStatusService2Test {
when(context.getTimeLeft()).thenReturn(Duration.ofSeconds(12));
- try (MutableStatusService registry = zkStatusService.lockApplication(context, reference)) {
+ try (ApplicationLock lock = zkStatusService.lockApplication(context, reference)) {
// nothing
}
@@ -65,7 +65,7 @@ public class ZkStatusService2Test {
when(context.isProbe()).thenReturn(false);
- try (MutableStatusService registry = zkStatusService.lockApplication(context, reference)) {
+ try (ApplicationLock lock = zkStatusService.lockApplication(context, reference)) {
// nothing
}
@@ -88,7 +88,7 @@ public class ZkStatusService2Test {
when(context.getTimeLeft()).thenReturn(Duration.ofSeconds(12));
- try (MutableStatusService registry = zkStatusService.lockApplication(context, reference)) {
+ try (ApplicationLock lock = zkStatusService.lockApplication(context, reference)) {
// nothing
}
@@ -105,7 +105,7 @@ public class ZkStatusService2Test {
when(context.hasLock(any())).thenReturn(true);
when(context.registerLockAcquisition(any(), any())).thenReturn(false);
- try (MutableStatusService registry = zkStatusService.lockApplication(context, reference)) {
+ try (ApplicationLock lock = zkStatusService.lockApplication(context, reference)) {
// nothing
}
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java
index 48d50946e51..a6d7f09d69b 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/status/ZkStatusServiceTest.java
@@ -107,17 +107,14 @@ public class ZkStatusServiceTest {
Instant.ofEpochMilli((3)),
Instant.ofEpochMilli(6));
- try (MutableStatusService statusRegistry = zkStatusService
- .lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
+ try (ApplicationLock lock = zkStatusService.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
//shuffling to catch "clean database" failures for all cases.
for (HostStatus hostStatus: shuffledList(HostStatus.NO_REMARKS, HostStatus.ALLOWED_TO_BE_DOWN)) {
for (int i = 0; i < 2; i++) {
- statusRegistry.setHostState(
- TestIds.HOST_NAME1,
- hostStatus);
+ lock.setHostState(TestIds.HOST_NAME1, hostStatus);
- assertThat(statusRegistry.getHostInfos().getOrNoRemarks(TestIds.HOST_NAME1).status(),
+ assertThat(lock.getHostInfos().getOrNoRemarks(TestIds.HOST_NAME1).status(),
is(hostStatus));
}
}
@@ -144,11 +141,11 @@ public class ZkStatusServiceTest {
ZkStatusService zkStatusService2 = new ZkStatusService(curator, mock(Metric.class), new TestTimer());
final CompletableFuture<Void> lockedSuccessfullyFuture;
- try (MutableStatusService statusRegistry = zkStatusService
+ try (ApplicationLock lock = zkStatusService
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
lockedSuccessfullyFuture = CompletableFuture.runAsync(() -> {
- try (MutableStatusService statusRegistry2 = zkStatusService2
+ try (ApplicationLock lock2 = zkStatusService2
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE))
{
}
@@ -168,7 +165,7 @@ public class ZkStatusServiceTest {
public void failing_to_get_lock_closes_SessionFailRetryLoop() throws Exception {
ZkStatusService zkStatusService2 = new ZkStatusService(curator, mock(Metric.class), new TestTimer());
- try (MutableStatusService statusRegistry = zkStatusService
+ try (ApplicationLock lock = zkStatusService
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
//must run in separate thread, since having 2 locks in the same thread fails
@@ -182,7 +179,7 @@ public class ZkStatusServiceTest {
killSession(curator.framework(), testingServer);
//Throws SessionFailedException if the SessionFailRetryLoop has not been closed.
- statusRegistry.getHostInfos().getOrNoRemarks(TestIds.HOST_NAME1);
+ lock.getHostInfos().getOrNoRemarks(TestIds.HOST_NAME1);
});
assertThat(resultOfZkOperationAfterLockFailure, notHoldsException());
@@ -244,9 +241,9 @@ public class ZkStatusServiceTest {
is(ApplicationInstanceStatus.NO_REMARKS));
// Suspend
- try (MutableStatusService statusRegistry = zkStatusService
+ try (ApplicationLock lock = zkStatusService
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
- statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
+ lock.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
}
assertThat(
@@ -255,9 +252,9 @@ public class ZkStatusServiceTest {
is(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN));
// Resume
- try (MutableStatusService statusRegistry = zkStatusService
+ try (ApplicationLock lock = zkStatusService
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
- statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.NO_REMARKS);
+ lock.setApplicationInstanceStatus(ApplicationInstanceStatus.NO_REMARKS);
}
assertThat(
@@ -272,14 +269,14 @@ public class ZkStatusServiceTest {
= zkStatusService.getAllSuspendedApplications();
assertThat(suspendedApps.size(), is(0));
- try (MutableStatusService statusRegistry = zkStatusService
+ try (ApplicationLock statusRegistry = zkStatusService
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE)) {
statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
}
- try (MutableStatusService statusRegistry = zkStatusService
+ try (ApplicationLock lock = zkStatusService
.lockApplication(context, TestIds.APPLICATION_INSTANCE_REFERENCE2)) {
- statusRegistry.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
+ lock.setApplicationInstanceStatus(ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN);
}
suspendedApps = zkStatusService.getAllSuspendedApplications();