diff options
author | HÃ¥kon Hallingstad <hakon.hallingstad@gmail.com> | 2023-09-26 15:22:26 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-26 15:22:26 +0200 |
commit | f3ddf24f6b540e10fc2a87914531391c8d64b272 (patch) | |
tree | 203d12c0f04a99411fb2e2571bfd95fcf0c4cf4b | |
parent | b5ca6e534bb44e0afb43aa8e08acd213d60f46f9 (diff) | |
parent | 531f145441af440f8edb6ab003b0e26de201ef4e (diff) |
Merge pull request #28671 from vespa-engine/hmusum/refactor-completion-waiter-2
Hmusum/refactor completion waiter 2
5 files changed, 49 insertions, 43 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 693252da43a..d86e0e3c340 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -26,6 +26,7 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.CompletionTimeoutException; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.curator.transaction.CuratorOperations; import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.ListFlag; @@ -430,11 +431,19 @@ public class TenantApplications implements RequestHandler, HostValidator { public TenantFileSystemDirs getTenantFileSystemDirs() { return tenantFileSystemDirs; } public CompletionWaiter createRemoveApplicationWaiter(ApplicationId applicationId) { - return RemoveApplicationWaiter.createAndInitialize(curator, applicationId, serverId); + var barrierPath = barrierPath(applicationId); + return RemoveApplicationWaiter.createAndInitialize(curator, barrierPath, serverId); } public CompletionWaiter getRemoveApplicationWaiter(ApplicationId applicationId) { - return RemoveApplicationWaiter.create(curator, applicationId, serverId); + var barrierPath = barrierPath(applicationId); + return RemoveApplicationWaiter.create(curator, barrierPath, serverId); + } + + private Path barrierPath(ApplicationId applicationId) { + return TenantRepository.getBarriersPath().append(applicationId.tenant().value()) + .append("delete-application") + .append(applicationId.serializedForm()); } /** @@ -453,14 +462,12 @@ public class TenantApplications implements RequestHandler, HostValidator { private final Duration waitForAll; private final Clock clock = Clock.systemUTC(); - RemoveApplicationWaiter(Curator curator, ApplicationId applicationId, String serverId) { - this(curator, applicationId, serverId, waitForAllDefault); + RemoveApplicationWaiter(Curator curator, Path barrierPath, String serverId) { + this(curator, barrierPath, serverId, waitForAllDefault); } - RemoveApplicationWaiter(Curator curator, ApplicationId applicationId, String serverId, Duration waitForAll) { - this.barrierPath = TenantRepository.getBarriersPath().append(applicationId.tenant().value()) - .append("delete-application") - .append(applicationId.serializedForm()); + RemoveApplicationWaiter(Curator curator, Path barrierPath, String serverId, Duration waitForAll) { + this.barrierPath = barrierPath; this.waiterNode = barrierPath.append(serverId); this.curator = curator; this.waitForAll = waitForAll; @@ -542,34 +549,29 @@ public class TenantApplications implements RequestHandler, HostValidator { @Override public String toString() { return "'" + barrierPath + "', " + barrierMemberCount() + " members"; } - public static CompletionWaiter create(Curator curator, ApplicationId applicationId, String serverId) { - return new RemoveApplicationWaiter(curator, applicationId, serverId); + public static CompletionWaiter create(Curator curator, Path barrierPath, String serverId) { + return new RemoveApplicationWaiter(curator, barrierPath, serverId); } - public static CompletionWaiter create(Curator curator, ApplicationId applicationId, String serverId, Duration waitForAll) { - return new RemoveApplicationWaiter(curator, applicationId, serverId, waitForAll); + public static CompletionWaiter create(Curator curator, Path barrierPath, String serverId, Duration waitForAll) { + return new RemoveApplicationWaiter(curator, barrierPath, serverId, waitForAll); } - public static CompletionWaiter createAndInitialize(Curator curator, ApplicationId applicationId, String serverId) { - return createAndInitialize(curator, applicationId, serverId, waitForAllDefault); + public static CompletionWaiter createAndInitialize(Curator curator, Path barrierPath, String serverId) { + return createAndInitialize(curator, barrierPath, serverId, waitForAllDefault); } - public static CompletionWaiter createAndInitialize(Curator curator, ApplicationId applicationId, String serverId, Duration waitForAll) { - RemoveApplicationWaiter waiter = new RemoveApplicationWaiter(curator, applicationId, serverId, waitForAll); - - // Cleanup and create a new barrier path - Path barrierPath = waiter.barrierPath(); + public static CompletionWaiter createAndInitialize(Curator curator, Path barrierPath, String serverId, Duration waitForAll) { + // Note: Should be done atomically, but unable to that when path may not exist before delete + // and create should be able to create any missing parent paths curator.delete(barrierPath); - curator.create(barrierPath.getParentPath()); - curator.createAtomically(barrierPath); + curator.create(barrierPath); - return waiter; + return new RemoveApplicationWaiter(curator, barrierPath, serverId, waitForAll); } private int barrierMemberCount() { return (curator.zooKeeperEnsembleCount() / 2) + 1; /* majority */ } - private Path barrierPath() { return barrierPath; } - } } 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 2bc8cb5bc0a..378cd9bdb8c 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,21 +118,21 @@ public class SessionZooKeeperClient { public long sessionId() { return sessionId; } - public CompletionWaiter createActiveWaiter() { return createCompletionWaiter(getWaiterPath(ACTIVE_BARRIER)); } + public CompletionWaiter createActiveWaiter() { return createCompletionWaiter(barrierPath(ACTIVE_BARRIER)); } - CompletionWaiter createPrepareWaiter() { return createCompletionWaiter(getWaiterPath(PREPARE_BARRIER)); } + CompletionWaiter createPrepareWaiter() { return createCompletionWaiter(barrierPath(PREPARE_BARRIER)); } - CompletionWaiter getPrepareWaiter() { return getCompletionWaiter(getWaiterPath(PREPARE_BARRIER)); } + CompletionWaiter getPrepareWaiter() { return getCompletionWaiter(barrierPath(PREPARE_BARRIER)); } - CompletionWaiter getActiveWaiter() { return getCompletionWaiter(getWaiterPath(ACTIVE_BARRIER)); } + CompletionWaiter getActiveWaiter() { return getCompletionWaiter(barrierPath(ACTIVE_BARRIER)); } - CompletionWaiter getUploadWaiter() { return getCompletionWaiter(getWaiterPath(UPLOAD_BARRIER)); } + CompletionWaiter getUploadWaiter() { return getCompletionWaiter(barrierPath(UPLOAD_BARRIER)); } private static final String PREPARE_BARRIER = "prepareBarrier"; private static final String ACTIVE_BARRIER = "activeBarrier"; private static final String UPLOAD_BARRIER = "uploadBarrier"; - private Path getWaiterPath(String barrierName) { + private Path barrierPath(String barrierName) { return sessionPath.append(barrierName); } 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 80646dc5607..1bed85b1c02 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -166,19 +166,19 @@ public class Curator extends AbstractComponent implements AutoCloseable { private void addLoggingListener() { curatorFramework.getConnectionStateListenable().addListener((curatorFramework, connectionState) -> { switch (connectionState) { - case SUSPENDED: LOG.info("ZK connection state change: SUSPENDED"); break; - case RECONNECTED: LOG.info("ZK connection state change: RECONNECTED"); break; - case LOST: LOG.warning("ZK connection state change: LOST"); break; + case SUSPENDED -> LOG.info("ZK connection state change: SUSPENDED"); + case RECONNECTED -> LOG.info("ZK connection state change: RECONNECTED"); + case LOST -> LOG.warning("ZK connection state change: LOST"); } }); } - public CompletionWaiter getCompletionWaiter(Path waiterPath, String id, Duration waitForAll) { - return CuratorCompletionWaiter.create(this, waiterPath, id, waitForAll); + public CompletionWaiter getCompletionWaiter(Path barrierPath, String id, Duration waitForAll) { + return CuratorCompletionWaiter.create(this, barrierPath, id, waitForAll); } - public CompletionWaiter createCompletionWaiter(Path waiterPath, String id, Duration waitForAll) { - return CuratorCompletionWaiter.createAndInitialize(this, waiterPath, id, waitForAll); + public CompletionWaiter createCompletionWaiter(Path barrierPath, String id, Duration waitForAll) { + return CuratorCompletionWaiter.createAndInitialize(this, barrierPath, 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 7d918baaf54..9a8b9b5bf60 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java @@ -2,6 +2,8 @@ package com.yahoo.vespa.curator; import com.yahoo.path.Path; +import com.yahoo.vespa.curator.transaction.CuratorOperations; +import com.yahoo.vespa.curator.transaction.CuratorTransaction; import java.time.Clock; import java.time.Duration; @@ -120,11 +122,13 @@ class CuratorCompletionWaiter implements CompletionWaiter { return new CuratorCompletionWaiter(curator, barrierPath, id, Clock.systemUTC(), waitForAll); } - public static CompletionWaiter createAndInitialize(Curator curator, Path waiterPath, String id, Duration waitForAll) { - curator.delete(waiterPath); - curator.createAtomically(waiterPath); + public static CompletionWaiter createAndInitialize(Curator curator, Path barrierPath, String id, Duration waitForAll) { + // Note: Should be done atomically, but unable to that when path may not exist before delete + // and create should be able to create any missing parent paths + curator.delete(barrierPath); + curator.create(barrierPath); - return new CuratorCompletionWaiter(curator, waiterPath, id, Clock.systemUTC(), waitForAll); + return new CuratorCompletionWaiter(curator, barrierPath, id, Clock.systemUTC(), waitForAll); } private int barrierMemberCount() { 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 592b9fc2a05..e1376fb154b 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 @@ -82,12 +82,12 @@ public class MockCurator extends Curator { } @Override - public CompletionWaiter getCompletionWaiter(Path parentPath, String id, Duration waitForAll) { + public CompletionWaiter getCompletionWaiter(Path barrierPath, String id, Duration waitForAll) { return mockFramework().createCompletionWaiter(); } @Override - public CompletionWaiter createCompletionWaiter(Path waiterPath, String id, Duration waitForAll) { + public CompletionWaiter createCompletionWaiter(Path barrierPath, String id, Duration waitForAll) { return mockFramework().createCompletionWaiter(); } |