diff options
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(); |