diff options
author | Harald Musum <musum@yahooinc.com> | 2023-02-20 12:13:36 +0100 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2023-02-20 12:13:36 +0100 |
commit | 232a0fd9f10e6a89de7e51a2f3cf7f406a6d00fe (patch) | |
tree | 4a9d806f2be0fc55483fee2ab8e57ea51d23814f /zkfacade | |
parent | acbdecc1f1b65e069dc9a5fc73575f0e8f653a98 (diff) | |
parent | 8895f9401559fe00dbeacfc9eee9f89d4dd8374b (diff) |
Merge branch 'master' into hmusum/upgrade-curator-2
Diffstat (limited to 'zkfacade')
-rw-r--r-- | zkfacade/pom.xml | 5 | ||||
-rw-r--r-- | zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java | 2 | ||||
-rw-r--r-- | zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java | 39 |
3 files changed, 21 insertions, 25 deletions
diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index 9c5ed23636f..d9a9cb51c89 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -58,11 +58,6 @@ </exclusions> </dependency> <dependency> - <groupId>com.google.guava</groupId> - <artifactId>guava</artifactId> - <scope>provided</scope> - </dependency> - <dependency> <groupId>org.apache.zookeeper</groupId> <artifactId>zookeeper</artifactId> <exclusions> 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 bb81135dae3..80159624eed 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -68,7 +68,7 @@ public class Curator extends AbstractComponent implements AutoCloseable { private static final Duration ZK_CONNECTION_TIMEOUT = Duration.ofSeconds(30); private static final Duration BASE_SLEEP_TIME = Duration.ofSeconds(1); - private static final int MAX_RETRIES = 10; + private static final int MAX_RETRIES = 4; private static final RetryPolicy DEFAULT_RETRY_POLICY = new ExponentialBackoffRetry((int) BASE_SLEEP_TIME.toMillis(), MAX_RETRIES); // Default value taken from ZookeeperServerConfig static final long defaultJuteMaxBuffer = Long.parseLong(System.getProperty("jute.maxbuffer", "52428800")); diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index 8eda57b0476..e37d561ca1b 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -128,11 +128,12 @@ class SingletonManager { } - private static final Instant INVALID = Instant.ofEpochMilli(-1); + private static final Instant EMPTY = Instant.ofEpochMilli(-1); + private static final Instant INVALID = Instant.ofEpochMilli(-2); final BlockingDeque<Task> tasks = new LinkedBlockingDeque<>(); final Deque<SingletonWorker> singletons = new ArrayDeque<>(2); - final AtomicReference<Instant> doom = new AtomicReference<>(); + final AtomicReference<Instant> doom = new AtomicReference<>(EMPTY); final AtomicBoolean shutdown = new AtomicBoolean(); final Thread worker; final String id; @@ -152,15 +153,15 @@ class SingletonManager { } public void unlock() { - doom.set(null); if (lock != null) try { logger.log(INFO, "Relinquishing lease for " + id); lock.close(); - lock = null; } catch (Exception e) { logger.log(WARNING, "Failed closing " + lock, e); } + doom.set(EMPTY); + lock = null; } private void run() { @@ -272,13 +273,12 @@ class SingletonManager { * If lock is held, or acquired, ping the ZK cluster to extend our deadline. */ private void renewLease() { - if (doom.get() == INVALID) { + // Witness value to detect if invalidation occurs between here and successful ping. + Instant ourDoom = doom.get(); + if (ourDoom == INVALID) { logger.log(INFO, "Lease invalidated"); - doom.set(null); return; // Skip to updateStatus, deactivation, and release the lock. } - // Witness value to detect if invalidation occurs between here and successful ping. - Instant ourDoom = doom.get(); Instant start = clock.instant(); if (lock == null) try { lock = curator.lock(path.append("lock"), tickTimeout); @@ -302,7 +302,7 @@ class SingletonManager { return; } if ( ! doom.compareAndSet(ourDoom, start.plus(curator.sessionTimeout().multipliedBy(9).dividedBy(10)))) { - logger.log(FINE, "Deadline changed, current lease renewal is void"); + logger.log(INFO, "Deadline changed from " + ourDoom + " to " + doom.get() + "; current lease renewal is void"); } } @@ -324,8 +324,7 @@ class SingletonManager { * If activation fails, we release the lock, so a different container may acquire it. */ private void updateStatus() { - Instant ourDoom = doom.get(); - boolean shouldBeActive = ourDoom != null && ourDoom != INVALID && ! clock.instant().isAfter(ourDoom); + boolean shouldBeActive = doom.get().isAfter(clock.instant()); if ( ! active && shouldBeActive) { try { active = true; @@ -336,14 +335,16 @@ class SingletonManager { shouldBeActive = false; } } - if (active && ! shouldBeActive) { - logger.log(FINE, () -> "Doom value is " + doom); - try { - if ( ! singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); - active = false; - } - catch (RuntimeException e) { - logger.log(WARNING, "Failed to deactivate " + singletons.peek(), e); + if ( ! shouldBeActive) { + logger.log(FINE, () -> "Doom value is " + doom.get()); + if (active) { + try { + if ( ! singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); + active = false; + } + catch (RuntimeException e) { + logger.log(WARNING, "Failed to deactivate " + singletons.peek(), e); + } } unlock(); } |