From 51d97f62b16b5af66943633ef0880d9b2d314f95 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 18 Sep 2023 09:06:06 +0200 Subject: Minor refactoring of CuratorCompletionWaiter, no functional changes --- .../main/java/com/yahoo/vespa/curator/Curator.java | 4 ++-- .../vespa/curator/CuratorCompletionWaiter.java | 21 ++++----------------- .../com/yahoo/vespa/curator/mock/MockCurator.java | 2 +- .../vespa/curator/CuratorCompletionWaiterTest.java | 4 ++-- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 2781e81cd7c..80646dc5607 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -177,8 +177,8 @@ public class Curator extends AbstractComponent implements AutoCloseable { return CuratorCompletionWaiter.create(this, waiterPath, id, waitForAll); } - public CompletionWaiter createCompletionWaiter(Path parentPath, String waiterNode, String id, Duration waitForAll) { - return CuratorCompletionWaiter.createAndInitialize(this, parentPath, waiterNode, id, waitForAll); + public CompletionWaiter createCompletionWaiter(Path waiterPath, String id, Duration waitForAll) { + return CuratorCompletionWaiter.createAndInitialize(this, waiterPath, id, waitForAll); } /** Creates a listenable cache which keeps in sync with changes to all the immediate children of a path */ diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java index 5a3d6668231..420b049cc50 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java @@ -6,7 +6,6 @@ import com.yahoo.path.Path; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; import java.util.List; import java.util.logging.Level; @@ -60,10 +59,9 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { Instant endTime = startTime.plus(timeout); Instant gotQuorumTime = Instant.EPOCH; - List respondents = new ArrayList<>(); + List respondents; do { - respondents.clear(); - respondents.addAll(curator.framework().getChildren().forPath(barrierPath)); + respondents = (curator.framework().getChildren().forPath(barrierPath)); if (log.isLoggable(Level.FINER)) { log.log(Level.FINER, respondents.size() + "/" + curator.zooKeeperEnsembleCount() + " responded: " + respondents + ", all participants: " + curator.zooKeeperEnsembleConnectionSpec()); @@ -105,16 +103,12 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { @Override public void notifyCompletion() { try { - notifyInternal(); + curator.framework().create().forPath(myId); } catch (Exception e) { throw new RuntimeException(e); } } - private void notifyInternal() throws Exception { - curator.framework().create().forPath(myId); - } - @Override public String toString() { return "'" + barrierPath + "', " + barrierMemberCount() + " members"; @@ -124,17 +118,10 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { return new CuratorCompletionWaiter(curator, barrierPath.getAbsolute(), id, Clock.systemUTC(), waitForAll); } - public static Curator.CompletionWaiter createAndInitialize(Curator curator, Path parentPath, String waiterNode, String id, Duration waitForAll) { - Path waiterPath = parentPath.append(waiterNode); - - String debugMessage = log.isLoggable(Level.FINE) ? "Recreating ZK path " + waiterPath : null; - if (debugMessage != null) log.fine(debugMessage); - + public static Curator.CompletionWaiter createAndInitialize(Curator curator, Path waiterPath, String id, Duration waitForAll) { curator.delete(waiterPath); curator.createAtomically(waiterPath); - if (debugMessage != null) log.fine(debugMessage + ": Done"); - return new CuratorCompletionWaiter(curator, waiterPath.getAbsolute(), id, Clock.systemUTC(), waitForAll); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java index e578746d348..592b9fc2a05 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java @@ -87,7 +87,7 @@ public class MockCurator extends Curator { } @Override - public CompletionWaiter createCompletionWaiter(Path parentPath, String waiterNode, String id, Duration waitForAll) { + public CompletionWaiter createCompletionWaiter(Path waiterPath, String id, Duration waitForAll) { return mockFramework().createCompletionWaiter(); } diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java index dbdb14b9214..17ec7fb65e9 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java @@ -19,7 +19,7 @@ public class CuratorCompletionWaiterTest { @Test public void testCompletionWaiter() throws InterruptedException { Curator curator = new MockCurator(); - Curator.CompletionWaiter waiter = CuratorCompletionWaiter.createAndInitialize(curator, Path.createRoot(), "foo", "foo", waitForAll); + Curator.CompletionWaiter waiter = CuratorCompletionWaiter.createAndInitialize(curator, Path.createRoot().append("foo"), "foo", waitForAll); Curator.CompletionWaiter notifier = CuratorCompletionWaiter.create(curator, Path.fromString("/foo"), "bar", waitForAll); Thread t1 = new Thread(() -> { try { @@ -36,7 +36,7 @@ public class CuratorCompletionWaiterTest { @Test(expected = CompletionTimeoutException.class) public void testCompletionWaiterFailure() { Curator curator = new MockCurator(); - Curator.CompletionWaiter waiter = CuratorCompletionWaiter.createAndInitialize(curator, Path.createRoot(), "foo", "foo", waitForAll); + Curator.CompletionWaiter waiter = CuratorCompletionWaiter.createAndInitialize(curator, Path.createRoot().append("foo"), "foo", waitForAll); waiter.awaitCompletion(Duration.ofMillis(100)); } } -- cgit v1.2.3 From 24cc103df27df8c90f42240dccabe64371ca7ec0 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 18 Sep 2023 09:10:49 +0200 Subject: Follow API changes --- .../yahoo/vespa/config/server/session/SessionZooKeeperClient.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java index 85abd937ac0..2bc8cb5bc0a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java @@ -118,9 +118,9 @@ public class SessionZooKeeperClient { public long sessionId() { return sessionId; } - public CompletionWaiter createActiveWaiter() { return createCompletionWaiter(ACTIVE_BARRIER); } + public CompletionWaiter createActiveWaiter() { return createCompletionWaiter(getWaiterPath(ACTIVE_BARRIER)); } - CompletionWaiter createPrepareWaiter() { return createCompletionWaiter(PREPARE_BARRIER); } + CompletionWaiter createPrepareWaiter() { return createCompletionWaiter(getWaiterPath(PREPARE_BARRIER)); } CompletionWaiter getPrepareWaiter() { return getCompletionWaiter(getWaiterPath(PREPARE_BARRIER)); } @@ -136,8 +136,8 @@ public class SessionZooKeeperClient { return sessionPath.append(barrierName); } - private CompletionWaiter createCompletionWaiter(String waiterNode) { - return curator.createCompletionWaiter(sessionPath, waiterNode, serverId, barrierWaitForAllTimeout); + private CompletionWaiter createCompletionWaiter(Path path) { + return curator.createCompletionWaiter(path, serverId, barrierWaitForAllTimeout); } private CompletionWaiter getCompletionWaiter(Path path) { -- cgit v1.2.3 From 50b7f332b3dc12d3ff909971c84fb9f82b075d22 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 18 Sep 2023 09:42:19 +0200 Subject: Use Path, rename some arguments --- .../vespa/curator/CuratorCompletionWaiter.java | 24 ++++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java index 420b049cc50..7d918baaf54 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java @@ -9,6 +9,8 @@ import java.time.Instant; import java.util.List; import java.util.logging.Level; +import static com.yahoo.vespa.curator.Curator.CompletionWaiter; + /** * Implementation of a Barrier that handles the case where more than number of members can call synchronize. * Will wait for some time for all servers to do the operation, but will accept the majority of servers to have @@ -17,18 +19,18 @@ import java.util.logging.Level; * @author Vegard Havdal * @author Ulf Lilleengen */ -class CuratorCompletionWaiter implements Curator.CompletionWaiter { +class CuratorCompletionWaiter implements CompletionWaiter { private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(CuratorCompletionWaiter.class.getName()); private final Curator curator; - private final String barrierPath; - private final String myId; + private final Path barrierPath; + private final Path waiterNode; private final Clock clock; private final Duration waitForAll; - CuratorCompletionWaiter(Curator curator, String barrierPath, String myId, Clock clock, Duration waitForAll) { - this.myId = barrierPath + "/" + myId; + CuratorCompletionWaiter(Curator curator, Path barrierPath, String serverId, Clock clock, Duration waitForAll) { + this.waiterNode = barrierPath.append(serverId); this.curator = curator; this.barrierPath = barrierPath; this.clock = clock; @@ -61,7 +63,7 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { List respondents; do { - respondents = (curator.framework().getChildren().forPath(barrierPath)); + respondents = curator.framework().getChildren().forPath(barrierPath.getAbsolute()); if (log.isLoggable(Level.FINER)) { log.log(Level.FINER, respondents.size() + "/" + curator.zooKeeperEnsembleCount() + " responded: " + respondents + ", all participants: " + curator.zooKeeperEnsembleConnectionSpec()); @@ -103,7 +105,7 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { @Override public void notifyCompletion() { try { - curator.framework().create().forPath(myId); + curator.framework().create().forPath(waiterNode.getAbsolute()); } catch (Exception e) { throw new RuntimeException(e); } @@ -114,15 +116,15 @@ class CuratorCompletionWaiter implements Curator.CompletionWaiter { return "'" + barrierPath + "', " + barrierMemberCount() + " members"; } - public static Curator.CompletionWaiter create(Curator curator, Path barrierPath, String id, Duration waitForAll) { - return new CuratorCompletionWaiter(curator, barrierPath.getAbsolute(), id, Clock.systemUTC(), waitForAll); + public static CompletionWaiter create(Curator curator, Path barrierPath, String id, Duration waitForAll) { + return new CuratorCompletionWaiter(curator, barrierPath, id, Clock.systemUTC(), waitForAll); } - public static Curator.CompletionWaiter createAndInitialize(Curator curator, Path waiterPath, String id, Duration waitForAll) { + public static CompletionWaiter createAndInitialize(Curator curator, Path waiterPath, String id, Duration waitForAll) { curator.delete(waiterPath); curator.createAtomically(waiterPath); - return new CuratorCompletionWaiter(curator, waiterPath.getAbsolute(), id, Clock.systemUTC(), waitForAll); + return new CuratorCompletionWaiter(curator, waiterPath, id, Clock.systemUTC(), waitForAll); } private int barrierMemberCount() { -- cgit v1.2.3