Merge pull request #22198 from vespa-engine/hmusum/use-new-controller-lock-scheme

Use new controller lock scheme by default

2 files changed, 9 insertions, 98 deletions

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 16398fb1137..c3c68f7596f 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
@@ -15,12 +15,8 @@ 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;
-import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.Flags;
-import com.yahoo.vespa.flags.StringFlag;
 import com.yahoo.vespa.hosted.controller.Application;
+import com.yahoo.vespa.hosted.controller.api.identifiers.ControllerVersion;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry;
 import com.yahoo.vespa.hosted.controller.api.integration.archive.ArchiveBucket;
@@ -28,7 +24,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.vcmr.VespaChangeRequest;
-import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.auditlog.AuditLog;
 import com.yahoo.vespa.hosted.controller.deployment.RetriggerEntry;
@@ -42,12 +37,10 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingStatus;
 import com.yahoo.vespa.hosted.controller.routing.ZoneRoutingPolicy;
 import com.yahoo.vespa.hosted.controller.support.access.SupportAccess;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
-import com.yahoo.vespa.hosted.controller.api.identifiers.ControllerVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
-
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.ByteBuffer;
@@ -121,7 +114,6 @@ public class CuratorDb {
 
     private final Curator curator;
     private final Duration tryLockTimeout;
-    private final StringFlag lockScheme;
 
     // For each application id (path), store the ZK node version and its deserialised data - update when version changes.
     // This will grow to keep all applications in memory, but this should be OK
@@ -131,14 +123,13 @@ public class CuratorDb {
     private final Map<Path, Pair<Integer, NavigableMap<RunId, Run>>> cachedHistoricRuns = new ConcurrentHashMap<>();
 
     @Inject
-    public CuratorDb(Curator curator, FlagSource flagSource, ServiceRegistry services) {
-        this(curator, defaultTryLockTimeout, flagSource, services.zoneRegistry().system());
+    public CuratorDb(Curator curator, ServiceRegistry services) {
+        this(curator, defaultTryLockTimeout, services.zoneRegistry().system());
     }
 
-    CuratorDb(Curator curator, Duration tryLockTimeout, FlagSource flagSource, SystemName system) {
+    CuratorDb(Curator curator, Duration tryLockTimeout, SystemName system) {
         this.curator = curator;
         this.tryLockTimeout = tryLockTimeout;
-        this.lockScheme = Flags.CONTROLLER_LOCK_SCHEME.bindTo(flagSource);
     }
 
     /** Returns all hostnames configured to be part of this ZooKeeper cluster */
@@ -156,58 +147,19 @@ public class CuratorDb {
     }
 
     public Mutex lock(TenantAndApplicationId id) {
-        switch (lockScheme.value()) {
-            case "BOTH":
-                return new MultiplePathsLock(curator.lock(legacyLockPath(id), defaultLockTimeout.multipliedBy(2)),
-                                             curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2)));
-            case "OLD":
-                return curator.lock(legacyLockPath(id), defaultLockTimeout.multipliedBy(2));
-            case "NEW":
-                return curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2));
-            default:
-                throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value());
-        }
+        return curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2));
     }
 
     public Mutex lockForDeployment(ApplicationId id, ZoneId zone) {
-        switch (lockScheme.value()) {
-            case "BOTH":
-                return new MultiplePathsLock(curator.lock(legacyLockPath(id, zone), defaultLockTimeout),
-                                             curator.lock(lockPath(id, zone), defaultLockTimeout));
-            case "OLD":
-                return curator.lock(legacyLockPath(id, zone), deployLockTimeout);
-            case "NEW":
-                return curator.lock(lockPath(id, zone), deployLockTimeout);
-            default:
-                throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value());
-        }
+        return curator.lock(lockPath(id, zone), deployLockTimeout);
     }
 
     public Mutex lock(ApplicationId id, JobType type) {
-        switch (lockScheme.value()) {
-            case "BOTH":
-                return new MultiplePathsLock(curator.lock(legacyLockPath(id, type), defaultLockTimeout),
-                                             curator.lock(lockPath(id, type), defaultLockTimeout));
-            case "OLD":
-                return curator.lock(legacyLockPath(id, type), defaultLockTimeout);
-            case "NEW":
-                return curator.lock(lockPath(id, type), defaultLockTimeout);
-            default:
-                throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value());
-        }
+        return curator.lock(lockPath(id, type), defaultLockTimeout);
     }
 
     public Mutex lock(ApplicationId id, JobType type, Step step) throws TimeoutException {
-        switch (lockScheme.value()) {
-            case "BOTH":
-                return tryLock(legacyLockPath(id, type, step), lockPath(id, type, step));
-            case "OLD":
-                return tryLock(legacyLockPath(id, type, step));
-            case "NEW":
-                return tryLock(lockPath(id, type, step));
-            default:
-        throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value());
-        }
+        return tryLock(lockPath(id, type, step));
     }
 
     public Mutex lockRotations() {
@@ -289,19 +241,6 @@ public class CuratorDb {
         }
     }
 
-    /** Try locking with a low timeout, meaning it is OK to fail lock acquisition.
-     *
-     * Useful for maintenance jobs, where there is no point in running the jobs back to back.
-     */
-    private Mutex tryLock(Path path, Path path2) throws TimeoutException {
-        try {
-            return new MultiplePathsLock(curator.lock(path, tryLockTimeout), curator.lock(path2, tryLockTimeout));
-        }
-        catch (UncheckedTimeoutException e) {
-            throw new TimeoutException(e.getMessage());
-        }
-    }
-
     private <T> Optional<T> read(Path path, Function<byte[], T> mapper) {
         return curator.getData(path).filter(data -> data.length > 0).map(mapper);
     }
@@ -730,32 +669,6 @@ public class CuratorDb {
                 .append(tenant.value());
     }
 
-    private Path legacyLockPath(TenantAndApplicationId application) {
-        return lockPath(application.tenant())
-                .append(application.application().value());
-    }
-
-    private Path legacyLockPath(ApplicationId instance) {
-        return legacyLockPath(TenantAndApplicationId.from(instance))
-                .append(instance.instance().value());
-    }
-
-    private Path legacyLockPath(ApplicationId instance, ZoneId zone) {
-        return legacyLockPath(instance)
-                .append(zone.environment().value())
-                .append(zone.region().value());
-    }
-
-    private Path legacyLockPath(ApplicationId instance, JobType type) {
-        return legacyLockPath(instance)
-                .append(type.jobName());
-    }
-
-    private Path legacyLockPath(ApplicationId instance, JobType type, Step step) {
-        return legacyLockPath(instance, type)
-                .append(step.name());
-    }
-
     private Path lockPath(TenantAndApplicationId application) {
         return lockRoot.append(application.tenant().value() + ":" + application.application().value());
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java
index d7391c69bf6..21414339a87 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java
@@ -5,8 +5,6 @@ import com.google.inject.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.curator.mock.MockCurator;
-import com.yahoo.vespa.flags.InMemoryFlagSource;
-
 import java.time.Duration;
 
 /**
@@ -34,7 +32,7 @@ public class MockCuratorDb extends CuratorDb {
     }
 
     public MockCuratorDb(MockCurator curator, SystemName system) {
-        super(curator, Duration.ofMillis(100), new InMemoryFlagSource(), system);
+        super(curator, Duration.ofMillis(100), system);
         this.curator = curator;
     }