You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hu...@apache.org on 2019/11/22 17:57:41 UTC

[helix] branch master updated: Add close() to ClusterSetup to avoid ZkClient leak (#629)

This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new 4d4cca9  Add close() to ClusterSetup to avoid ZkClient leak (#629)
4d4cca9 is described below

commit 4d4cca9b289cede16b67abb607c9d54ba1921809
Author: Hunter Lee <hu...@linkedin.com>
AuthorDate: Fri Nov 22 09:57:35 2019 -0800

    Add close() to ClusterSetup to avoid ZkClient leak (#629)
    
    ClusterSetup is a helper class that's used for cluster-related operations. This class is missing a close() method users could call to prevent ZkClient/ZkConnection leak. This commit adds close() method.
---
 .../java/org/apache/helix/tools/ClusterSetup.java  | 25 ++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java b/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
index f0c909d..5d5f864 100644
--- a/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
+++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
@@ -134,10 +134,14 @@ public class ClusterSetup {
   public static final String setConstraint = "setConstraint";
   public static final String removeConstraint = "removeConstraint";
 
-  static Logger _logger = LoggerFactory.getLogger(ClusterSetup.class);
-  String _zkServerAddress;
-  HelixZkClient _zkClient;
-  HelixAdmin _admin;
+  private static final Logger _logger = LoggerFactory.getLogger(ClusterSetup.class);
+  private final String _zkServerAddress;
+  private final HelixZkClient _zkClient;
+  // true if ZkBaseDataAccessor was instantiated with a HelixZkClient, false otherwise
+  // This is used for close() to determine how ZkBaseDataAccessor should close the underlying
+  // ZkClient
+  private final boolean _usesExternalZkClient;
+  private final HelixAdmin _admin;
 
   public ClusterSetup(String zkServerAddress) {
     _zkServerAddress = zkServerAddress;
@@ -145,18 +149,31 @@ public class ClusterSetup {
         .buildZkClient(new HelixZkClient.ZkConnectionConfig(_zkServerAddress));
     _zkClient.setZkSerializer(new ZNRecordSerializer());
     _admin = new ZKHelixAdmin(_zkClient);
+    _usesExternalZkClient = false;
   }
 
   public ClusterSetup(HelixZkClient zkClient) {
     _zkServerAddress = zkClient.getServers();
     _zkClient = zkClient;
     _admin = new ZKHelixAdmin(_zkClient);
+    _usesExternalZkClient = true;
   }
 
   public ClusterSetup(HelixZkClient zkClient, HelixAdmin zkHelixAdmin) {
     _zkServerAddress = zkClient.getServers();
     _zkClient = zkClient;
     _admin = zkHelixAdmin;
+    _usesExternalZkClient = true;
+  }
+
+  /**
+   * Closes any stateful resources in ClusterSetup.
+   */
+  public void close() {
+    if (_zkClient != null && !_usesExternalZkClient) {
+      _admin.close();
+      _zkClient.close();
+    }
   }
 
   public void addCluster(String clusterName, boolean overwritePrevious) {