Merge pull request #5873 from vespa-engine/mpolden/confidence-override-lock

Lock confidence overrides before writing

3 files changed, 21 insertions, 11 deletions

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 502df641313..8fb37ba2a15 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
@@ -9,6 +9,7 @@ import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.athenz.api.AthenzDomain;
+import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
@@ -193,9 +194,11 @@ public class Controller extends AbstractComponent {
 
     /** Remove confidence override for versions matching given filter */
     public void removeConfidenceOverride(Predicate<Version> filter) {
-        Map<Version, VespaVersion.Confidence> overrides = new LinkedHashMap<>(curator().readConfidenceOverrides());
-        overrides.keySet().removeIf(filter);
-        curator.writeConfidenceOverrides(overrides);
+        try (Lock lock = curator.lockConfidenceOverrides()) {
+            Map<Version, VespaVersion.Confidence> overrides = new LinkedHashMap<>(curator.readConfidenceOverrides());
+            overrides.keySet().removeIf(filter);
+            curator.writeConfidenceOverrides(overrides);
+        }
     }
     
     /** Returns the current system version: The controller should drive towards running all applications on this version */
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 843a3e8d97a..4692c2fb23a 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy;
+import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
@@ -131,9 +132,11 @@ public class Upgrader extends Maintainer {
 
     /** Override confidence for given version. This will cause the computed confidence to be ignored */
     public void overrideConfidence(Version version, Confidence confidence) {
-        Map<Version, Confidence> overrides = new LinkedHashMap<>(curator.readConfidenceOverrides());
-        overrides.put(version, confidence);
-        curator.writeConfidenceOverrides(overrides);
+        try (Lock lock = curator.lockConfidenceOverrides()) {
+            Map<Version, Confidence> overrides = new LinkedHashMap<>(curator.readConfidenceOverrides());
+            overrides.put(version, confidence);
+            curator.writeConfidenceOverrides(overrides);
+        }
     }
 
     /** Returns all confidence overrides */
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 4224609f1e2..184ac90691a 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
@@ -86,6 +86,13 @@ public class CuratorDb {
 
     // -------------- Locks ---------------------------------------------------
 
+    /** Create a reentrant lock */
+    private Lock lock(Path path, Duration timeout) {
+        Lock lock = locks.computeIfAbsent(path, (pathArg) -> new Lock(pathArg.getAbsolute(), curator));
+        lock.acquire(timeout);
+        return lock;
+    }
+
     public Lock lock(TenantName name, Duration timeout) {
         return lock(lockPath(name), timeout);
     }
@@ -98,11 +105,8 @@ public class CuratorDb {
         return lock(lockRoot.append("rotations"), defaultLockTimeout);
     }
 
-    /** Create a reentrant lock */
-    private Lock lock(Path path, Duration timeout) {
-        Lock lock = locks.computeIfAbsent(path, (pathArg) -> new Lock(pathArg.getAbsolute(), curator));
-        lock.acquire(timeout);
-        return lock;
+    public Lock lockConfidenceOverrides() {
+        return lock(lockRoot.append("confidenceOverrides"), defaultLockTimeout);
     }
 
     public Lock lockInactiveJobs() {