Merge branch 'master' into hmusum/upgrade-curator-2

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();
             }