diff options
author | jonmv <venstad@gmail.com> | 2022-04-17 15:49:00 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-04-17 15:49:00 +0200 |
commit | 0773124206f4836c02a2b28c8aabbb2224d6d554 (patch) | |
tree | 68d41117056d88d4348b8bd7c210ca6fe483bc2e | |
parent | 5dd851d5c5fce2e95b236cd1f5385270077f0450 (diff) |
Preserve reentrancy
18 files changed, 115 insertions, 92 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 5b61996083f..3be5345b377 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -13,6 +13,7 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.log.LogLevel; import com.yahoo.text.Text; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzPrincipal; @@ -402,7 +403,7 @@ public class ApplicationController { * @throws IllegalArgumentException if the application already exists */ public Application createApplication(TenantAndApplicationId id, Credentials credentials) { - try (Lock lock = lock(id)) { + try (Mutex lock = lock(id)) { if (getApplication(id).isPresent()) throw new IllegalArgumentException("Could not create '" + id + "': Application already exists"); if (getApplication(dashToUnderscore(id)).isPresent()) // VESPA-1945 @@ -456,7 +457,7 @@ public class ApplicationController { ZoneId zone = job.type().zone(controller.system()); DeploymentId deployment = new DeploymentId(job.application(), zone); - try (Lock deploymentLock = lockForDeployment(job.application(), zone)) { + try (Mutex deploymentLock = lockForDeployment(job.application(), zone)) { Set<ContainerEndpoint> containerEndpoints; Optional<EndpointCertificateMetadata> endpointCertificateMetadata; @@ -470,7 +471,7 @@ public class ApplicationController { RevisionId revision = run.versions().sourceRevision().filter(__ -> deploySourceVersions).orElse(run.versions().targetRevision()); ApplicationPackage applicationPackage = new ApplicationPackage(applicationStore.get(deployment, revision)); - try (Lock lock = lock(applicationId)) { + try (Mutex lock = lock(applicationId)) { LockedApplication application = new LockedApplication(requireApplication(applicationId), lock); Instance instance = application.get().require(job.application().instance()); @@ -742,7 +743,7 @@ public class ApplicationController { * @param action Function which acts on the locked application. */ public void lockApplicationIfPresent(TenantAndApplicationId applicationId, Consumer<LockedApplication> action) { - try (Lock lock = lock(applicationId)) { + try (Mutex lock = lock(applicationId)) { getApplication(applicationId).map(application -> new LockedApplication(application, lock)).ifPresent(action); } } @@ -755,7 +756,7 @@ public class ApplicationController { * @throws IllegalArgumentException when application does not exist. */ public void lockApplicationOrThrow(TenantAndApplicationId applicationId, Consumer<LockedApplication> action) { - try (Lock lock = lock(applicationId)) { + try (Mutex lock = lock(applicationId)) { action.accept(new LockedApplication(requireApplication(applicationId), lock)); } } @@ -828,14 +829,14 @@ public class ApplicationController { * Any operation which stores an application need to first acquire this lock, then read, modify * and store the application, and finally release (close) the lock. */ - Lock lock(TenantAndApplicationId application) { + Mutex lock(TenantAndApplicationId application) { return curator.lock(application); } /** * Returns a lock which provides exclusive rights to deploying this application to the given zone. */ - private Lock lockForDeployment(ApplicationId application, ZoneId zone) { + private Mutex lockForDeployment(ApplicationId application, ZoneId zone) { return curator.lockForDeployment(application, zone); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index be88aede1eb..52f1a5e0ab0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -12,6 +12,7 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.jdisc.Metric; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; @@ -197,7 +198,7 @@ public class Controller extends AbstractComponent { /** Remove confidence override for versions matching given filter */ public void removeConfidenceOverride(Predicate<Version> filter) { - try (Lock lock = curator.lockConfidenceOverrides()) { + try (Mutex lock = curator.lockConfidenceOverrides()) { Map<Version, VespaVersion.Confidence> overrides = new LinkedHashMap<>(curator.readConfidenceOverrides()); overrides.keySet().removeIf(filter); curator.writeConfidenceOverrides(overrides); @@ -236,7 +237,7 @@ public class Controller extends AbstractComponent { throw new IllegalArgumentException("Cloud '" + cloudName + "' does not exist in this system"); } Instant scheduledAt = clock.instant(); - try (Lock lock = curator.lockOsVersions()) { + try (Mutex lock = curator.lockOsVersions()) { Map<CloudName, OsVersionTarget> targets = curator.readOsVersionTargets().stream() .collect(Collectors.toMap(t -> t.osVersion().cloud(), Function.identity())); @@ -266,7 +267,7 @@ public class Controller extends AbstractComponent { /** Replace the current OS version status with a new one */ public void updateOsVersionStatus(OsVersionStatus newStatus) { - try (Lock lock = curator.lockOsVersionStatus()) { + try (Mutex lock = curator.lockOsVersionStatus()) { OsVersionStatus currentStatus = curator.readOsVersionStatus(); for (CloudName cloud : clouds()) { Set<Version> newVersions = newStatus.versionsIn(cloud); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 551327fd2e5..3e822415e96 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.InstanceName; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -30,7 +31,7 @@ import java.util.function.UnaryOperator; */ public class LockedApplication { - private final Lock lock; + private final Mutex lock; private final TenantAndApplicationId id; private final Instant createdAt; private final DeploymentSpec deploymentSpec; @@ -51,7 +52,7 @@ public class LockedApplication { * @param application The application to lock. * @param lock The lock for the application. */ - LockedApplication(Application application, Lock lock) { + LockedApplication(Application application, Mutex lock) { this(Objects.requireNonNull(lock, "lock cannot be null"), application.id(), application.createdAt(), application.deploymentSpec(), application.validationOverrides(), application.deploymentIssueId(), application.ownershipIssueId(), @@ -59,7 +60,7 @@ public class LockedApplication { application.projectId(), application.instances(), application.revisions()); } - private LockedApplication(Lock lock, TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, + private LockedApplication(Mutex lock, TenantAndApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides, Optional<IssueId> deploymentIssueId, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Set<PublicKey> deployKeys, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index bcc3b9b54c7..7a0e60aacb4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -6,6 +6,7 @@ import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableBiMap; import com.yahoo.config.provision.TenantName; import com.yahoo.security.KeyUtils; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.identifiers.Property; @@ -46,7 +47,7 @@ public abstract class LockedTenant { this.lastLoginInfo = requireNonNull(lastLoginInfo); } - static LockedTenant of(Tenant tenant, Lock lock) { + static LockedTenant of(Tenant tenant, Mutex lock) { switch (tenant.type()) { case athenz: return new Athenz((AthenzTenant) tenant); case cloud: return new Cloud((CloudTenant) tenant); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java index 138bf547655..beba1cdb358 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller; import com.yahoo.config.provision.TenantName; import com.yahoo.text.Text; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; import com.yahoo.vespa.hosted.controller.application.SystemApplication; @@ -74,7 +75,7 @@ public class TenantController { /** Locks a tenant for modification and applies the given action. */ public <T extends LockedTenant> void lockIfPresent(TenantName name, Class<T> token, Consumer<T> action) { - try (Lock lock = lock(name)) { + try (Mutex lock = lock(name)) { get(name).map(tenant -> LockedTenant.of(tenant, lock)) .map(token::cast) .ifPresent(action); @@ -83,7 +84,7 @@ public class TenantController { /** Lock a tenant for modification and apply action. Throws if the tenant does not exist */ public <T extends LockedTenant> void lockOrThrow(TenantName name, Class<T> token, Consumer<T> action) { - try (Lock lock = lock(name)) { + try (Mutex lock = lock(name)) { action.accept(token.cast(LockedTenant.of(require(name), lock))); } } @@ -111,7 +112,7 @@ public class TenantController { /** Create a tenant, provided the given credentials are valid. */ public void create(TenantSpec tenantSpec, Credentials credentials) { - try (Lock lock = lock(tenantSpec.tenant())) { + try (Mutex lock = lock(tenantSpec.tenant())) { TenantId.validate(tenantSpec.tenant().value()); requireNonExistent(tenantSpec.tenant()); curator.writeTenant(accessControl.createTenant(tenantSpec, controller.clock().instant(), credentials, asList())); @@ -137,7 +138,7 @@ public class TenantController { /** Updates the tenant contained in the given tenant spec with new data. */ public void update(TenantSpec tenantSpec, Credentials credentials) { - try (Lock lock = lock(tenantSpec.tenant())) { + try (Mutex lock = lock(tenantSpec.tenant())) { curator.writeTenant(accessControl.updateTenant(tenantSpec, credentials, asList(), controller.applications().asList(tenantSpec.tenant()))); } @@ -148,7 +149,7 @@ public class TenantController { * new instant is later */ public void updateLastLogin(TenantName tenantName, List<LastLoginInfo.UserLevel> userLevels, Instant loggedInAt) { - try (Lock lock = lock(tenantName)) { + try (Mutex lock = lock(tenantName)) { Tenant tenant = require(tenantName); LastLoginInfo loginInfo = tenant.lastLoginInfo(); for (LastLoginInfo.UserLevel userLevel : userLevels) @@ -161,7 +162,7 @@ public class TenantController { /** Deletes the given tenant. */ public void delete(TenantName tenant, Optional<Credentials> credentials, boolean forget) { - try (Lock lock = lock(tenant)) { + try (Mutex lock = lock(tenant)) { Tenant oldTenant = get(tenant, true) .orElseThrow(() -> new NotExistsException("Could not delete tenant '" + tenant + "': Tenant not found")); @@ -202,7 +203,7 @@ public class TenantController { * Any operation which stores a tenant need to first acquire this lock, then read, modify * and store the tenant, and finally release (close) the lock. */ - private Lock lock(TenantName tenant) { + private Mutex lock(TenantName tenant) { return curator.lock(tenant); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLogger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLogger.java index 78d65766075..34e7955e02a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLogger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/auditlog/AuditLogger.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.auditlog; import com.yahoo.container.jdisc.HttpRequest; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; @@ -70,7 +71,7 @@ public class AuditLogger { Instant now = clock.instant(); AuditLog.Entry entry = new AuditLog.Entry(now, principal.getName(), method.get(), pathAndQueryOf(request.getUri()), Optional.of(new String(data, StandardCharsets.UTF_8))); - try (Lock lock = db.lockAuditLog()) { + try (Mutex lock = db.lockAuditLog()) { AuditLog auditLog = db.readAuditLog() .pruneBefore(now.minus(entryTtl)) .with(entry) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 54703308102..c98b3b76292 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -8,6 +8,7 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.InstanceName; import com.yahoo.text.Text; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; @@ -275,7 +276,7 @@ public class DeploymentTrigger { if (existingRun.isPresent()) { Run run = existingRun.get(); - try (Lock lock = controller.curator().lockDeploymentRetriggerQueue()) { + try (Mutex lock = controller.curator().lockDeploymentRetriggerQueue()) { List<RetriggerEntry> retriggerEntries = controller.curator().readRetriggerEntries(); List<RetriggerEntry> newList = new ArrayList<>(retriggerEntries); RetriggerEntry requiredEntry = new RetriggerEntry(new JobId(deployment.applicationId(), jobType), run.id().number() + 1); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index d1c19476929..4b8339cccb1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -6,6 +6,7 @@ import com.yahoo.component.Version; import com.yahoo.component.VersionCompatibility; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.ApplicationController; @@ -132,7 +133,7 @@ public class JobController { /** Returns the logged entries for the given run, which are after the given id threshold. */ public Optional<RunLog> details(RunId id, long after) { - try (Lock __ = curator.lock(id.application(), id.type())) { + try (Mutex __ = curator.lock(id.application(), id.type())) { Run run = runs(id.application(), id.type()).get(id); if (run == null) return Optional.empty(); @@ -378,7 +379,7 @@ public class JobController { * Throws TimeoutException if some step in this job is still being run. */ public void finish(RunId id) throws TimeoutException { - List<Lock> locks = new ArrayList<>(); + List<Mutex> locks = new ArrayList<>(); try { // Ensure no step is still running before we finish the run — report depends transitively on all the other steps. Run unlockedRun = run(id).get(); @@ -426,7 +427,7 @@ public class JobController { }); } finally { - for (Lock lock : locks) + for (Mutex lock : locks) lock.close(); } } @@ -658,7 +659,7 @@ public class JobController { TesterId tester = TesterId.of(id); for (JobType type : jobs(id)) locked(id, type, deactivateTester, __ -> { - try (Lock ___ = curator.lock(id, type)) { + try (Mutex ___ = curator.lock(id, type)) { try { deactivateTester(tester, type); } @@ -684,7 +685,7 @@ public class JobController { /** Locks all runs and modifies the list of historic runs for the given application and job type. */ private void locked(ApplicationId id, JobType type, Consumer<SortedMap<RunId, Run>> modifications) { Application application = controller.applications().requireApplication(TenantAndApplicationId.from(id)); - try (Lock __ = curator.lock(id, type)) { + try (Mutex __ = curator.lock(id, type)) { SortedMap<RunId, Run> runs = new TreeMap<>(curator.readHistoricRuns(id, type)); modifications.accept(runs); curator.writeHistoricRuns(id, type, runs.values(), application); @@ -694,7 +695,7 @@ public class JobController { /** Locks and modifies the run with the given id, provided it is still active. */ public void locked(RunId id, UnaryOperator<Run> modifications) { Application application = controller.applications().requireApplication(TenantAndApplicationId.from(id.application())); - try (Lock __ = curator.lock(id.application(), id.type())) { + try (Mutex __ = curator.lock(id.application(), id.type())) { active(id).ifPresent(run -> { run = modifications.apply(run); curator.writeLastRun(run, application); @@ -704,9 +705,9 @@ public class JobController { /** Locks the given step and checks none of its prerequisites are running, then performs the given actions. */ public void locked(ApplicationId id, JobType type, Step step, Consumer<LockedStep> action) throws TimeoutException { - try (Lock lock = curator.lock(id, type, step)) { + try (Mutex lock = curator.lock(id, type, step)) { for (Step prerequisite : step.allPrerequisites(last(id, type).get().steps().keySet())) // Check that no prerequisite is still running. - try (Lock __ = curator.lock(id, type, prerequisite)) { ; } + try (Mutex __ = curator.lock(id, type, prerequisite)) { ; } action.accept(new LockedStep(lock, step)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/LockedStep.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/LockedStep.java index d46516582be..8147ccb3180 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/LockedStep.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/LockedStep.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.deployment; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; /** @@ -9,7 +10,7 @@ import com.yahoo.vespa.curator.Lock; public class LockedStep { private final Step step; - LockedStep(Lock lock, Step step) { this.step = step; } + LockedStep(Mutex lock, Step step) { this.step = step; } public Step get() { return step; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java index aecb1e7a2c1..540e8489e6d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/dns/NameServiceForwarder.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.dns; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.integration.dns.AliasTarget; import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService; @@ -79,7 +80,7 @@ public class NameServiceForwarder { } protected void forward(NameServiceRequest request, NameServiceQueue.Priority priority) { - try (Lock lock = db.lockNameServiceQueue()) { + try (Mutex lock = db.lockNameServiceQueue()) { NameServiceQueue queue = db.readNameServiceQueue(); var queued = queue.requests().size(); if (queued >= QUEUE_CAPACITY) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index 15f8d6380c0..faa42e5caef 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -6,6 +6,7 @@ import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; import com.yahoo.container.jdisc.secretstore.SecretNotFoundException; import com.yahoo.container.jdisc.secretstore.SecretStore; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; @@ -82,7 +83,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { var refreshedCertificateMetadata = endpointCertificateMetadata .withVersion(latestAvailableVersion.getAsInt()) .withLastRefreshed(clock.instant().getEpochSecond()); - try (Lock lock = lock(applicationId)) { + try (Mutex lock = lock(applicationId)) { if (Optional.of(endpointCertificateMetadata).equals(curator.readEndpointCertificateMetadata(applicationId))) { curator.writeEndpointCertificateMetadata(applicationId, refreshedCertificateMetadata); // Certificate not validated here, but on deploy. } @@ -128,7 +129,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { curator.readAllEndpointCertificateMetadata().forEach((applicationId, storedMetaData) -> { var lastRequested = Instant.ofEpochSecond(storedMetaData.lastRequested()); if (lastRequested.isBefore(oneMonthAgo) && hasNoDeployments(applicationId)) { - try (Lock lock = lock(applicationId)) { + try (Mutex lock = lock(applicationId)) { if (Optional.of(storedMetaData).equals(curator.readEndpointCertificateMetadata(applicationId))) { log.log(Level.INFO, "Cert for app " + applicationId.serializedForm() + " has not been requested in a month and app has no deployments, deleting from provider and ZK"); @@ -140,7 +141,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { }); } - private Lock lock(ApplicationId applicationId) { + private Mutex lock(ApplicationId applicationId) { return curator.lock(TenantAndApplicationId.from(applicationId)); } @@ -169,7 +170,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { EndpointCertificateMetadata storedAppMetadata = storedAppEntry.getValue(); if (storedAppMetadata.certName().equals(unknownCertDetails.cert_key_keyname())) { matchFound = true; - try (Lock lock = lock(storedApp)) { + try (Mutex lock = lock(storedApp)) { if (Optional.of(storedAppMetadata).equals(curator.readEndpointCertificateMetadata(storedApp))) { log.log(Level.INFO, "Cert for app " + storedApp.serializedForm() + " has a new leafRequestId " + unknownCertDetails.request_id() + ", updating in ZK"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index fa5afa3a216..4ed34a91029 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; @@ -176,7 +177,7 @@ public class Upgrader extends ControllerMaintainer { " for version " + version.toFullString() + ": Version may be in use by applications"); } - try (Lock lock = curator.lockConfidenceOverrides()) { + try (Mutex lock = curator.lockConfidenceOverrides()) { Map<Version, Confidence> overrides = new LinkedHashMap<>(curator.readConfidenceOverrides()); overrides.put(version, confidence); curator.writeConfidenceOverrides(overrides); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java index aa62028749b..7876099cb21 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java @@ -5,6 +5,7 @@ import com.yahoo.collections.Pair; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.TenantName; import com.yahoo.text.Text; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -65,7 +66,7 @@ public class NotificationsDb { */ public void setNotification(NotificationSource source, Type type, Level level, List<String> messages) { Optional<Notification> changed = Optional.empty(); - try (Lock lock = curatorDb.lockNotifications(source.tenant())) { + try (Mutex lock = curatorDb.lockNotifications(source.tenant())) { var existingNotifications = curatorDb.readNotifications(source.tenant()); List<Notification> notifications = existingNotifications.stream() .filter(notification -> !source.equals(notification.source()) || type != notification.type()) @@ -82,7 +83,7 @@ public class NotificationsDb { /** Remove the notification with the given source and type */ public void removeNotification(NotificationSource source, Type type) { - try (Lock lock = curatorDb.lockNotifications(source.tenant())) { + try (Mutex lock = curatorDb.lockNotifications(source.tenant())) { List<Notification> initial = curatorDb.readNotifications(source.tenant()); List<Notification> filtered = initial.stream() .filter(notification -> !source.equals(notification.source()) || type != notification.type()) @@ -94,7 +95,7 @@ public class NotificationsDb { /** Remove all notifications for this source or sources contained by this source */ public void removeNotifications(NotificationSource source) { - try (Lock lock = curatorDb.lockNotifications(source.tenant())) { + try (Mutex lock = curatorDb.lockNotifications(source.tenant())) { if (source.application().isEmpty()) { // Source is tenant curatorDb.deleteNotifications(source.tenant()); return; @@ -130,7 +131,7 @@ public class NotificationsDb { .collect(Collectors.toUnmodifiableList()); NotificationSource deploymentSource = NotificationSource.from(deploymentId); - try (Lock lock = curatorDb.lockNotifications(deploymentSource.tenant())) { + try (Mutex lock = curatorDb.lockNotifications(deploymentSource.tenant())) { List<Notification> initial = curatorDb.readNotifications(deploymentSource.tenant()); List<Notification> updated = Stream.concat( initial.stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 0bfdd53eeab..d3b5c3754d5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -13,6 +13,7 @@ import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.curator.MultiplePathsLock; @@ -149,14 +150,15 @@ public class CuratorDb { // -------------- Locks --------------------------------------------------- - public Lock lock(TenantName name) { + public Mutex lock(TenantName name) { return curator.lock(lockPath(name), defaultLockTimeout.multipliedBy(2)); } - public Lock lock(TenantAndApplicationId id) { + public Mutex lock(TenantAndApplicationId id) { switch (lockScheme.value()) { case "BOTH": - return new MultiplePathsLock(lockPath(id), legacyLockPath(id), defaultLockTimeout.multipliedBy(2), curator); + return new MultiplePathsLock(curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2)), + curator.lock(legacyLockPath(id), defaultLockTimeout.multipliedBy(2))); case "OLD": return curator.lock(legacyLockPath(id), defaultLockTimeout.multipliedBy(2)); case "NEW": @@ -166,10 +168,11 @@ public class CuratorDb { } } - public Lock lockForDeployment(ApplicationId id, ZoneId zone) { + public Mutex lockForDeployment(ApplicationId id, ZoneId zone) { switch (lockScheme.value()) { case "BOTH": - return new MultiplePathsLock(lockPath(id, zone), legacyLockPath(id, zone), deployLockTimeout, curator); + return new MultiplePathsLock(curator.lock(lockPath(id, zone), defaultLockTimeout), + curator.lock(legacyLockPath(id, zone), defaultLockTimeout)); case "OLD": return curator.lock(legacyLockPath(id, zone), deployLockTimeout); case "NEW": @@ -179,10 +182,11 @@ public class CuratorDb { } } - public Lock lock(ApplicationId id, JobType type) { + public Mutex lock(ApplicationId id, JobType type) { switch (lockScheme.value()) { case "BOTH": - return new MultiplePathsLock(lockPath(id, type), legacyLockPath(id, type), defaultLockTimeout, curator); + return new MultiplePathsLock(curator.lock(lockPath(id, type), defaultLockTimeout), + curator.lock(legacyLockPath(id, type), defaultLockTimeout)); case "OLD": return curator.lock(legacyLockPath(id, type), defaultLockTimeout); case "NEW": @@ -192,7 +196,7 @@ public class CuratorDb { } } - public Lock lock(ApplicationId id, JobType type, Step step) throws TimeoutException { + public Mutex lock(ApplicationId id, JobType type, Step step) throws TimeoutException { switch (lockScheme.value()) { case "BOTH": return tryLock(lockPath(id, type, step), legacyLockPath(id, type, step)); @@ -205,15 +209,15 @@ public class CuratorDb { } } - public Lock lockRotations() { + public Mutex lockRotations() { return curator.lock(lockRoot.append("rotations"), defaultLockTimeout); } - public Lock lockConfidenceOverrides() { + public Mutex lockConfidenceOverrides() { return curator.lock(lockRoot.append("confidenceOverrides"), defaultLockTimeout); } - public Lock lockMaintenanceJob(String jobName) { + public Mutex lockMaintenanceJob(String jobName) { try { return tryLock(lockRoot.append("maintenanceJobLocks").append(jobName)); } catch (TimeoutException e) { @@ -221,52 +225,51 @@ public class CuratorDb { } } - @SuppressWarnings("unused") // Called by internal code - public Lock lockProvisionState(String provisionStateId) { + public Mutex lockProvisionState(String provisionStateId) { return curator.lock(lockPath(provisionStateId), Duration.ofSeconds(1)); } - public Lock lockOsVersions() { + public Mutex lockOsVersions() { return curator.lock(lockRoot.append("osTargetVersion"), defaultLockTimeout); } - public Lock lockOsVersionStatus() { + public Mutex lockOsVersionStatus() { return curator.lock(lockRoot.append("osVersionStatus"), defaultLockTimeout); } - public Lock lockRoutingPolicies() { + public Mutex lockRoutingPolicies() { return curator.lock(lockRoot.append("routingPolicies"), defaultLockTimeout); } - public Lock lockAuditLog() { + public Mutex lockAuditLog() { return curator.lock(lockRoot.append("auditLog"), defaultLockTimeout); } - public Lock lockNameServiceQueue() { + public Mutex lockNameServiceQueue() { return curator.lock(lockRoot.append("nameServiceQueue"), defaultLockTimeout); } - public Lock lockMeteringRefreshTime() throws TimeoutException { + public Mutex lockMeteringRefreshTime() throws TimeoutException { return tryLock(lockRoot.append("meteringRefreshTime")); } - public Lock lockArchiveBuckets(ZoneId zoneId) { + public Mutex lockArchiveBuckets(ZoneId zoneId) { return curator.lock(lockRoot.append("archiveBuckets").append(zoneId.value()), defaultLockTimeout); } - public Lock lockChangeRequests() { + public Mutex lockChangeRequests() { return curator.lock(lockRoot.append("changeRequests"), defaultLockTimeout); } - public Lock lockNotifications(TenantName tenantName) { + public Mutex lockNotifications(TenantName tenantName) { return curator.lock(lockRoot.append("notifications").append(tenantName.value()), defaultLockTimeout); } - public Lock lockSupportAccess(DeploymentId deploymentId) { + public Mutex lockSupportAccess(DeploymentId deploymentId) { return curator.lock(lockRoot.append("supportAccess").append(deploymentId.dottedString()), defaultLockTimeout); } - public Lock lockDeploymentRetriggerQueue() { + public Mutex lockDeploymentRetriggerQueue() { return curator.lock(lockRoot.append("deploymentRetriggerQueue"), defaultLockTimeout); } @@ -276,7 +279,7 @@ public class CuratorDb { * * Useful for maintenance jobs, where there is no point in running the jobs back to back. */ - private Lock tryLock(Path path) throws TimeoutException { + private Mutex tryLock(Path path) throws TimeoutException { try { return curator.lock(path, tryLockTimeout); } @@ -289,9 +292,9 @@ public class CuratorDb { * * Useful for maintenance jobs, where there is no point in running the jobs back to back. */ - private Lock tryLock(Path path, Path path2) throws TimeoutException { + private Mutex tryLock(Path path, Path path2) throws TimeoutException { try { - return new MultiplePathsLock(path, path2, tryLockTimeout, curator); + return new MultiplePathsLock(curator.lock(path, tryLockTimeout), curator.lock(path2, tryLockTimeout)); } catch (UncheckedTimeoutException e) { throw new TimeoutException(e.getMessage()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 850a8cab123..a5a09ab6551 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneId; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; @@ -152,7 +153,7 @@ public class RoutingPolicies { } /** Update global DNS records for given policies */ - private void updateGlobalDnsOf(RoutingPolicyList instancePolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) { + private void updateGlobalDnsOf(RoutingPolicyList instancePolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Mutex lock) { Map<RoutingId, List<RoutingPolicy>> routingTable = instancePolicies.asInstanceRoutingTable(); for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) { RoutingId routingId = routeEntry.getKey(); @@ -225,7 +226,7 @@ public class RoutingPolicies { } - private void updateApplicationDnsOf(RoutingPolicyList routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Lock lock) { + private void updateApplicationDnsOf(RoutingPolicyList routingPolicies, Set<ZoneId> inactiveZones, @SuppressWarnings("unused") Mutex lock) { // In the context of single deployment (which this is) there is only one routing policy per routing ID. I.e. // there is no scenario where more than one deployment within an instance can be a member the same // application-level endpoint. However, to allow this in the future the routing table remains @@ -307,7 +308,7 @@ public class RoutingPolicies { * * @return the updated policies */ - private RoutingPolicyList storePoliciesOf(LoadBalancerAllocation allocation, RoutingPolicyList instancePolicies, @SuppressWarnings("unused") Lock lock) { + private RoutingPolicyList storePoliciesOf(LoadBalancerAllocation allocation, RoutingPolicyList instancePolicies, @SuppressWarnings("unused") Mutex lock) { Map<RoutingPolicyId, RoutingPolicy> policies = new LinkedHashMap<>(instancePolicies.asMap()); for (LoadBalancer loadBalancer : allocation.loadBalancers) { if (loadBalancer.hostname().isEmpty()) continue; @@ -343,7 +344,7 @@ public class RoutingPolicies { * * @return the updated policies */ - private RoutingPolicyList removePoliciesUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList instancePolicies, @SuppressWarnings("unused") Lock lock) { + private RoutingPolicyList removePoliciesUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList instancePolicies, @SuppressWarnings("unused") Mutex lock) { Map<RoutingPolicyId, RoutingPolicy> newPolicies = new LinkedHashMap<>(instancePolicies.asMap()); Set<RoutingPolicyId> activeIds = allocation.asPolicyIds(); RoutingPolicyList removable = instancePolicies.deployment(allocation.deployment) @@ -363,7 +364,7 @@ public class RoutingPolicies { } /** Remove unreferenced instance endpoints from DNS */ - private void removeGlobalDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, @SuppressWarnings("unused") Lock lock) { + private void removeGlobalDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, @SuppressWarnings("unused") Mutex lock) { Set<RoutingId> removalCandidates = new HashSet<>(deploymentPolicies.asInstanceRoutingTable().keySet()); Set<RoutingId> activeRoutingIds = instanceRoutingIds(allocation); removalCandidates.removeAll(activeRoutingIds); @@ -380,7 +381,7 @@ public class RoutingPolicies { } /** Remove unreferenced application endpoints in given allocation from DNS */ - private void removeApplicationDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, @SuppressWarnings("unused") Lock lock) { + private void removeApplicationDnsUnreferencedBy(LoadBalancerAllocation allocation, RoutingPolicyList deploymentPolicies, @SuppressWarnings("unused") Mutex lock) { Map<RoutingId, List<RoutingPolicy>> routingTable = deploymentPolicies.asApplicationRoutingTable(); Set<RoutingId> removalCandidates = new HashSet<>(routingTable.keySet()); Set<RoutingId> activeRoutingIds = applicationRoutingIds(allocation); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationLock.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationLock.java index 36a43f80e9a..39fc70aac64 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationLock.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationLock.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.routing.rotation; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Lock; import java.util.Objects; @@ -12,9 +13,9 @@ import java.util.Objects; */ public class RotationLock implements AutoCloseable { - private final Lock lock; + private final Mutex lock; - RotationLock(Lock lock) { + RotationLock(Mutex lock) { this.lock = Objects.requireNonNull(lock, "lock cannot be null"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java index cd6e3a49c46..27b61a4fd17 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/support/access/SupportAccessControl.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.support.access; +import com.yahoo.transaction.Mutex; import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; @@ -35,7 +36,7 @@ public class SupportAccessControl { } public SupportAccess disallow(DeploymentId deployment, String by) { - try (Lock lock = controller.curator().lockSupportAccess(deployment)) { + try (Mutex lock = controller.curator().lockSupportAccess(deployment)) { var now = controller.clock().instant(); SupportAccess supportAccess = forDeployment(deployment); if (supportAccess.currentStatus(now).state() == NOT_ALLOWED) { @@ -49,7 +50,7 @@ public class SupportAccessControl { } public SupportAccess allow(DeploymentId deployment, Instant until, String by) { - try (Lock lock = controller.curator().lockSupportAccess(deployment)) { + try (Mutex lock = controller.curator().lockSupportAccess(deployment)) { var now = controller.clock().instant(); if (until.isAfter(now.plus(MAX_SUPPORT_ACCESS_TIME))) { throw new IllegalArgumentException("Support access cannot be allowed for more than 10 days"); @@ -61,7 +62,7 @@ public class SupportAccessControl { } public SupportAccess registerGrant(DeploymentId deployment, String by, X509Certificate certificate) { - try (Lock lock = controller.curator().lockSupportAccess(deployment)) { + try (Mutex lock = controller.curator().lockSupportAccess(deployment)) { var now = controller.clock().instant(); SupportAccess supportAccess = forDeployment(deployment); if (certificate.getNotAfter().toInstant().isBefore(now)) { diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java index 2520a0682ed..f71f9c6149e 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java @@ -2,8 +2,10 @@ package com.yahoo.vespa.curator; import com.yahoo.path.Path; +import com.yahoo.transaction.Mutex; import java.time.Duration; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -14,26 +16,28 @@ import java.util.logging.Logger; * * @author hmusum */ -public class MultiplePathsLock extends Lock { +public class MultiplePathsLock implements Mutex { private static final Logger log = Logger.getLogger(MultiplePathsLock.class.getName()); - private final Lock oldLock; + private final List<Lock> locks; - public MultiplePathsLock(Path newLockPath, Path oldLockPath, Duration timeout, Curator curator) { - super(newLockPath.getAbsolute(), curator); - log.log(Level.INFO, "Acquiring lock " + oldLockPath); - this.oldLock = curator.lock(oldLockPath, timeout);; - log.log(Level.INFO, "Acquiring lock " + lockPath()); - super.acquire(timeout); + /** Wrapped locks, in acquisition order. */ + public MultiplePathsLock(Lock... locks) { + this.locks = List.of(locks); } @Override public void close() { - log.log(Level.INFO, "Closing lock " + lockPath()); - super.close(); - log.log(Level.INFO, "Closing lock " + oldLock.lockPath()); - oldLock.close(); + close(0); + } + + private void close(int i) { + if (i < locks.size()) + try (Lock lock = locks.get(i)) { + close(i + 1); + log.log(Level.INFO, "Closing lock " + lock.lockPath()); + } } } |