Acquire a single lock again in VespaCurator

3 files changed, 1 insertions, 105 deletions

diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java
index 2cba6ee6efe..832755de6eb 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java
@@ -101,11 +101,7 @@ public class CuratorWrapper extends AbstractComponent implements VespaCurator {
 
     @Override
     public AutoCloseable lock(Path path, Duration timeout) {
-        // TODO jonmv: clear up
-        Lock current, old = curator.lock(path, timeout);
-        try { current = curator.lock(userRoot.append(path), timeout); }
-        catch (Throwable t) { old.close(); throw t; }
-        return () -> { try(old) { current.close(); } };
+        return curator.lock(userRoot.append(path), timeout);
     }
 
     @Override
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/api/AbstractSingletonWorker.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/api/AbstractSingletonWorker.java
deleted file mode 100644
index dc0540e73c5..00000000000
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/api/AbstractSingletonWorker.java
+++ /dev/null
@@ -1,95 +0,0 @@
-package com.yahoo.vespa.curator.api;
-
-import com.yahoo.component.AbstractComponent;
-import com.yahoo.concurrent.UncheckedTimeoutException;
-import com.yahoo.vespa.curator.api.VespaCurator.SingletonWorker;
-import com.yahoo.yolean.UncheckedInterruptedException;
-
-import java.time.Duration;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicReference;
-
-/**
- * Super-class for implementations of {@link SingletonWorker}.
- * Users should call {@link VespaCurator#registerSingleton} at construction, and
- * {@link VespaCurator#unregisterSingleton} at deconstruction.
- * If this fails activation on registration, it will immediately unregister as well, before propagating the error.
- * Consequently, registering this on construction will allow detecting a failed component generation, and instead
- * retain the previous generation, provided enough verification is done in {@link #activate()}.
- * The default ID to use with registration is the concrete class name, e.g., {@code my.example.Singleton}.
- *
- * @author jonmv
- */
-public abstract class AbstractSingletonWorker implements SingletonWorker {
-
-    private final AtomicReference<VespaCurator> owner = new AtomicReference<>();
-
-    /**
-     * The singleton ID to use when registering this with a {@link VespaCurator}.
-     * At most one singleton worker with the given ID will be active, in the cluster, at any time.
-     * {@link VespaCurator#isActive(String)} may be polled to see whether this container is currently
-     * allowed to have an active singleton with the given ID.
-     */
-    public String id() { return getClass().getName(); }
-
-    /**
-     * <strong>Call this at the end of construction of the owner of this.</strong>
-     * If this activates the singleton, this happens synchronously, and any errors are propagated here.
-     * If this replaces an already active singleton, its deactivation is also called, prior to activation of this.
-     * If (de)activation is not complete within the given timeout, a timeout exception is thrown.
-     * If an error occurs (due to failed activation), unregistration is automatically attempted, with the same timeout.
-     */
-    public final void register(VespaCurator curator, Duration timeout) {
-        if ( ! owner.compareAndSet(null, curator)) {
-            throw new IllegalArgumentException(this + " is already registered with " + owner.get());
-        }
-        try {
-            await(curator.registerSingleton(id(), this), timeout, "register");
-        }
-        catch (RuntimeException e) {
-            try {
-                unregister(timeout);
-            }
-            catch (Exception f) {
-                e.addSuppressed(f);
-            }
-            throw e;
-        }
-    }
-
-    /**
-     * <strong>Call this at the start of deconstruction of the owner of this!</strong>
-     * <p>
-     * If this singleton is active, deactivation will be called synchronously, and errors propagated here.
-     * If this also triggers activation of a new singleton, its activation is called after deactivation of this.
-     * If (de)activation is not complete within the given timeout, a timeout exception is thrown.
-     */
-    public final void unregister(Duration timeout) {
-        VespaCurator curator = owner.getAndSet(null);
-        if (curator == null) {
-            throw new IllegalArgumentException(this + " was not registered with any owners");
-        }
-        await(curator.unregisterSingleton(this), timeout, "unregister");
-    }
-
-    private void await(Future<?> future, Duration timeout, String verb) {
-        try {
-            future.get(timeout.toMillis(), TimeUnit.MILLISECONDS);
-        }
-        catch (InterruptedException e) {
-            future.cancel(true);
-            throw new UncheckedInterruptedException("interrupted while " + verb + "ing " + this, e, true);
-        }
-        catch (TimeoutException e) {
-            future.cancel(true);
-            throw new UncheckedTimeoutException("timed out while " + verb + "ing " + this, e);
-        }
-        catch (ExecutionException e) {
-            throw new RuntimeException("failed to " + verb + " " + this, e.getCause());
-        }
-    }
-
-}
diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java
index 1a5a9413774..030104b82f0 100644
--- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java
+++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java
@@ -61,13 +61,8 @@ public class CuratorWrapperTest {
             assertEquals(List.of(), curator.list(Path.createRoot()));
 
             try (AutoCloseable lock = curator.lock(path, Duration.ofSeconds(1))) {
-                assertEquals(List.of("user", "path"), wrapped.getChildren(Path.createRoot()));
                 assertEquals(List.of("path"), wrapped.getChildren(CuratorWrapper.userRoot));
             }
-
-            try (AutoCloseable lock = curator.lock(path, Duration.ofSeconds(1))) {
-                // Both previous locks were released.
-            }
         }
     }