diff options
author | jonmv <venstad@gmail.com> | 2022-10-12 10:50:54 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-10-12 10:50:54 +0200 |
commit | 1c6192ff71f635a25b069c7761948105f1ae6dae (patch) | |
tree | 6efca7431acab319788fd8b9fbcf4f1cabca8ea3 | |
parent | 09489728d0912761649106f00d7837281b272a76 (diff) |
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. - } } } |