Simplify, no functional changes

6 files changed, 30 insertions, 57 deletions

diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java
index c002ffa72ce..ae4a70e4677 100644
--- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java
+++ b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java
@@ -18,21 +18,21 @@ import java.util.concurrent.atomic.AtomicReference;
  */
 public class ReconfigurableVespaZooKeeperServer extends AbstractComponent implements VespaZooKeeperServer {
 
-    private final AtomicReference<QuorumPeer> peer = new AtomicReference<>();
+    private final QuorumPeer peer;
 
     @Inject
     public ReconfigurableVespaZooKeeperServer(Reconfigurer reconfigurer, ZookeeperServerConfig zookeeperServerConfig) {
-        reconfigurer.startOrReconfigure(zookeeperServerConfig, this, VespaQuorumPeer::new, peer::set);
+        peer = reconfigurer.startOrReconfigure(zookeeperServerConfig, this, VespaQuorumPeer::new);
     }
 
     @Override
     public void shutdown() {
-        peer.get().shutdown(Duration.ofMinutes(1));
+        peer.shutdown(Duration.ofMinutes(1));
     }
 
     @Override
     public void start(Path configFilePath) {
-        peer.get().start(configFilePath);
+        peer.start(configFilePath);
     }
 
     @Override
diff --git a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java
index 27aa18c64c7..d489c7d62f1 100644
--- a/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java
+++ b/zookeeper-server/zookeeper-server-3.6.3/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java
@@ -20,26 +20,19 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin {
 
     @Override
     public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException {
-        ZooKeeperAdmin zooKeeperAdmin = null;
-        try {
-            zooKeeperAdmin = createAdmin(connectionSpec);
+        try (ZooKeeperAdmin zooKeeperAdmin = createAdmin(connectionSpec)) {
             long fromConfig = -1;
             // Using string parameters because the List variant of reconfigure fails to join empty lists (observed on 3.5.6, fixed in 3.7.0)
             byte[] appliedConfig = zooKeeperAdmin.reconfigure(joiningServers, leavingServers, null, fromConfig, null);
             log.log(Level.INFO, "Applied ZooKeeper config: " + new String(appliedConfig, StandardCharsets.UTF_8));
-        } catch (KeeperException e) {
-            if (retryOn(e))
-                throw new ReconfigException(e);
-            else
-                throw new RuntimeException(e);
-        } catch (IOException | InterruptedException e) {
+        }
+        catch ( KeeperException.ReconfigInProgress
+              | KeeperException.ConnectionLossException
+              | KeeperException.NewConfigNoQuorum e) {
+            throw new ReconfigException(e);
+        }
+        catch (KeeperException | IOException | InterruptedException e) {
             throw new RuntimeException(e);
-        } finally {
-            if (zooKeeperAdmin != null) {
-                try {
-                    zooKeeperAdmin.close();
-                } catch (InterruptedException e) { /* ignore */}
-            }
         }
     }
 
@@ -48,11 +41,5 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin {
                                   (event) -> log.log(Level.INFO, event.toString()), new ZkClientConfigBuilder().toConfig());
     }
 
-    private static boolean retryOn(KeeperException e) {
-        return e instanceof KeeperException.ReconfigInProgress ||
-               e instanceof KeeperException.ConnectionLossException ||
-               e instanceof KeeperException.NewConfigNoQuorum;
-    }
-
 }
 
diff --git a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java
index c002ffa72ce..ae4a70e4677 100644
--- a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java
+++ b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/ReconfigurableVespaZooKeeperServer.java
@@ -18,21 +18,21 @@ import java.util.concurrent.atomic.AtomicReference;
  */
 public class ReconfigurableVespaZooKeeperServer extends AbstractComponent implements VespaZooKeeperServer {
 
-    private final AtomicReference<QuorumPeer> peer = new AtomicReference<>();
+    private final QuorumPeer peer;
 
     @Inject
     public ReconfigurableVespaZooKeeperServer(Reconfigurer reconfigurer, ZookeeperServerConfig zookeeperServerConfig) {
-        reconfigurer.startOrReconfigure(zookeeperServerConfig, this, VespaQuorumPeer::new, peer::set);
+        peer = reconfigurer.startOrReconfigure(zookeeperServerConfig, this, VespaQuorumPeer::new);
     }
 
     @Override
     public void shutdown() {
-        peer.get().shutdown(Duration.ofMinutes(1));
+        peer.shutdown(Duration.ofMinutes(1));
     }
 
     @Override
     public void start(Path configFilePath) {
-        peer.get().start(configFilePath);
+        peer.start(configFilePath);
     }
 
     @Override
diff --git a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java
index 27aa18c64c7..acd5cd07a28 100644
--- a/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java
+++ b/zookeeper-server/zookeeper-server-3.7.0/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperAdminImpl.java
@@ -20,26 +20,19 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin {
 
     @Override
     public void reconfigure(String connectionSpec, String joiningServers, String leavingServers) throws ReconfigException {
-        ZooKeeperAdmin zooKeeperAdmin = null;
-        try {
-            zooKeeperAdmin = createAdmin(connectionSpec);
+        try (ZooKeeperAdmin zooKeeperAdmin = createAdmin(connectionSpec)) {
             long fromConfig = -1;
             // Using string parameters because the List variant of reconfigure fails to join empty lists (observed on 3.5.6, fixed in 3.7.0)
             byte[] appliedConfig = zooKeeperAdmin.reconfigure(joiningServers, leavingServers, null, fromConfig, null);
             log.log(Level.INFO, "Applied ZooKeeper config: " + new String(appliedConfig, StandardCharsets.UTF_8));
-        } catch (KeeperException e) {
-            if (retryOn(e))
-                throw new ReconfigException(e);
-            else
-                throw new RuntimeException(e);
-        } catch (IOException | InterruptedException e) {
+        }
+        catch (   KeeperException.ReconfigInProgress
+                | KeeperException.ConnectionLossException
+                | KeeperException.NewConfigNoQuorum  e) {
+            throw new ReconfigException(e);
+        }
+        catch (KeeperException | IOException | InterruptedException e) {
             throw new RuntimeException(e);
-        } finally {
-            if (zooKeeperAdmin != null) {
-                try {
-                    zooKeeperAdmin.close();
-                } catch (InterruptedException e) { /* ignore */}
-            }
         }
     }
 
@@ -48,11 +41,5 @@ public class VespaZooKeeperAdminImpl implements VespaZooKeeperAdmin {
                                   (event) -> log.log(Level.INFO, event.toString()), new ZkClientConfigBuilder().toConfig());
     }
 
-    private static boolean retryOn(KeeperException e) {
-        return e instanceof KeeperException.ReconfigInProgress ||
-               e instanceof KeeperException.ConnectionLossException ||
-               e instanceof KeeperException.NewConfigNoQuorum;
-    }
-
 }
 
diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
index d4223e4d815..4452e74bb10 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Reconfigurer.java
@@ -13,7 +13,6 @@ import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -50,17 +49,17 @@ public class Reconfigurer extends AbstractComponent {
         this.sleeper = Objects.requireNonNull(sleeper);
     }
 
-    void startOrReconfigure(ZookeeperServerConfig newConfig, VespaZooKeeperServer server,
-                            Supplier<QuorumPeer> quorumPeerGetter, Consumer<QuorumPeer> quorumPeerSetter) {
+    QuorumPeer startOrReconfigure(ZookeeperServerConfig newConfig, VespaZooKeeperServer server,
+                            Supplier<QuorumPeer> quorumPeerCreator) {
         if (zooKeeperRunner == null) {
-            peer = quorumPeerGetter.get(); // Obtain the peer from the server. This will be shared with later servers.
+            peer = quorumPeerCreator.get(); // Obtain the peer from the server. This will be shared with later servers.
             zooKeeperRunner = startServer(newConfig, server);
         }
-        quorumPeerSetter.accept(peer);
 
         if (shouldReconfigure(newConfig)) {
             reconfigure(newConfig);
         }
+        return peer;
     }
 
     ZookeeperServerConfig activeConfig() {
@@ -76,7 +75,7 @@ public class Reconfigurer extends AbstractComponent {
     private boolean shouldReconfigure(ZookeeperServerConfig newConfig) {
         if (!newConfig.dynamicReconfiguration()) return false;
         if (activeConfig == null) return false;
-        return !newConfig.equals(activeConfig());
+        return ! newConfig.equals(activeConfig());
     }
 
     private ZooKeeperRunner startServer(ZookeeperServerConfig zookeeperServerConfig, VespaZooKeeperServer server) {
diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
index 1211624e3d6..1f551dc1e27 100644
--- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
+++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ReconfigurerTest.java
@@ -156,7 +156,7 @@ public class ReconfigurerTest {
         }
 
         void startOrReconfigure(ZookeeperServerConfig newConfig) {
-            startOrReconfigure(newConfig, this, MockQuorumPeer::new, peer -> serverPeer = peer);
+            serverPeer = startOrReconfigure(newConfig, this, MockQuorumPeer::new);
         }
 
         String connectionSpec() {