Implement suggestions from review (sleeper, more logging, sleep shorter)

2 files changed, 26 insertions, 9 deletions

diff --git a/zookeeper-server/zookeeper-server-3.5.6/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java b/zookeeper-server/zookeeper-server-3.5.6/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
index cd5161cad12..92211403fac 100644
--- a/zookeeper-server/zookeeper-server-3.5.6/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
+++ b/zookeeper-server/zookeeper-server-3.5.6/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
@@ -13,6 +13,7 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Consumer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -27,7 +28,8 @@ public class Reconfigurer extends AbstractComponent {
 
     private static final Logger log = java.util.logging.Logger.getLogger(Reconfigurer.class.getName());
     private static final Duration sessionTimeout = Duration.ofSeconds(30);
-    private static final Duration retryReconfigurationPeriod = Duration.ofMinutes(5);
+    private static final Duration retryReconfigurationPeriod = Duration.ofSeconds(30);
+    private static final Duration timeBetweenRetries = Duration.ofSeconds(1);
 
     private ZooKeeperRunner zooKeeperRunner;
     private ZookeeperServerConfig activeConfig;
@@ -38,11 +40,15 @@ public class Reconfigurer extends AbstractComponent {
     }
 
     void startOrReconfigure(ZookeeperServerConfig newConfig) {
+        startOrReconfigure(newConfig, Reconfigurer::defaultSleeper);
+    }
+
+    void startOrReconfigure(ZookeeperServerConfig newConfig, Consumer<Duration> sleeper) {
         if (zooKeeperRunner == null)
             zooKeeperRunner = startServer(newConfig);
 
         if (shouldReconfigure(newConfig))
-            reconfigure(newConfig);
+            reconfigure(newConfig, sleeper);
     }
 
     ZookeeperServerConfig activeConfig() {
@@ -81,7 +87,7 @@ public class Reconfigurer extends AbstractComponent {
         return runner;
     }
 
-    private void reconfigure(ZookeeperServerConfig newConfig) {
+    private void reconfigure(ZookeeperServerConfig newConfig, Consumer<Duration> sleeper) {
         String leavingServers = String.join(",", difference(serverIds(activeConfig), serverIds(newConfig)));
         String joiningServers = String.join(",", difference(servers(newConfig), servers(activeConfig)));
         leavingServers = leavingServers.isEmpty() ? null : leavingServers;
@@ -95,17 +101,15 @@ public class Reconfigurer extends AbstractComponent {
         // Loop reconfiguring since we might need to wait until another reconfiguration is finished before we can succeed
         while ( ! reconfigured && Instant.now().isBefore(end)) {
             try {
+                Instant start = Instant.now();
                 zooKeeperReconfigure(connectionSpec, joiningServers, leavingServers);
+                log.log(Level.INFO, "Reconfiguration finished after " + Duration.between(start, Instant.now()));
                 reconfigured = true;
             } catch (KeeperException e) {
                 if ( ! (e instanceof KeeperException.ReconfigInProgress))
                     throw new RuntimeException(e);
-                log.log(Level.INFO, "Will retry in 1 second");
-                try {
-                    Thread.sleep(1000);
-                } catch (InterruptedException interruptedException) {
-                    log.log(Level.INFO, "Interrupted waiting for reconfiguration to be done", interruptedException);
-                }
+                log.log(Level.INFO, "Will retry in " + timeBetweenRetries);
+                sleeper.accept(timeBetweenRetries);
             }
         }
         activeConfig = newConfig;
@@ -138,4 +142,12 @@ public class Reconfigurer extends AbstractComponent {
         return copy;
     }
 
+    private static void defaultSleeper(Duration duration) {
+        try {
+            Thread.sleep(duration.toMillis());
+        } catch (InterruptedException interruptedException) {
+            interruptedException.printStackTrace();
+        }
+    }
+
 }
diff --git a/zookeeper-server/zookeeper-server-3.5.6/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java b/zookeeper-server/zookeeper-server-3.5.6/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
index 91e4ffd6462..afbe2b38665 100644
--- a/zookeeper-server/zookeeper-server-3.5.6/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
+++ b/zookeeper-server/zookeeper-server-3.5.6/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
@@ -127,6 +127,11 @@ public class ReconfigurerTest {
         private String leavingServers;
 
         @Override
+        void startOrReconfigure(ZookeeperServerConfig newConfig) {
+            super.startOrReconfigure(newConfig, l->{});
+        }
+
+        @Override
         void zooKeeperReconfigure(String connectionSpec, String joiningServers, String leavingServers) throws KeeperException {
             this.connectionSpec = connectionSpec;
             this.joiningServers = joiningServers;