diff options
author | Håkon Hallingstad <hakon@verizonmedia.com> | 2020-03-02 14:38:00 +0100 |
---|---|---|
committer | Håkon Hallingstad <hakon@verizonmedia.com> | 2020-03-02 14:38:00 +0100 |
commit | 09486a9adff55af8a615dd51cbb230bf64653c1c (patch) | |
tree | 8bb3dec71115289a2bb633d19d3f218bd08a7d71 /orchestrator | |
parent | 4b6a2a891680dd778ccd5cf8761a888d44b3421a (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')
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(); |