aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHÃ¥kon Hallingstad <hakon.hallingstad@gmail.com>2023-09-26 15:22:26 +0200
committerGitHub <noreply@github.com>2023-09-26 15:22:26 +0200
commitf3ddf24f6b540e10fc2a87914531391c8d64b272 (patch)
tree203d12c0f04a99411fb2e2571bfd95fcf0c4cf4b
parentb5ca6e534bb44e0afb43aa8e08acd213d60f46f9 (diff)
parent531f145441af440f8edb6ab003b0e26de201ef4e (diff)
Merge pull request #28671 from vespa-engine/hmusum/refactor-completion-waiter-2
Hmusum/refactor completion waiter 2
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java50
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java12
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java14
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorCompletionWaiter.java12
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java4
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();
}