helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hu...@apache.org
Subject [helix] branch master updated: Patching zkClient leakage fix #656 (#657)
Date Tue, 17 Dec 2019 00:50:09 GMT
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 9adb6a2  Patching zkClient leakage fix #656 (#657)
9adb6a2 is described below

commit 9adb6a26c4563aca2cc6cf314ceaa7440936c538
Author: nealsun-linkedin <58795766+nealsun-linkedin@users.noreply.github.com>
AuthorDate: Mon Dec 16 16:49:59 2019 -0800

    Patching zkClient leakage fix #656 (#657)
    
    During some of the instantiations of zkClient, the closing of the clients are not guaranteed
due to the possible exceptions being thrown before the closing of the clients; in those cases,
zkClient leakage happens. I'm adding "try finally" clauses to guard against zkClient leakage
in these cases.
---
 .../java/org/apache/helix/manager/zk/ZKUtil.java   | 94 ++++++++++++++++------
 .../java/org/apache/helix/tools/MessagePoster.java | 20 +++--
 .../apache/helix/tools/commandtools/ZKDumper.java  | 32 +++++---
 3 files changed, 100 insertions(+), 46 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java
index daee5a8..bd98c15 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java
@@ -56,8 +56,12 @@ public final class ZKUtil {
    */
   public static boolean isClusterSetup(String clusterName, String zkAddress) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    boolean result = isClusterSetup(clusterName, zkClient);
-    zkClient.close();
+    boolean result;
+    try {
+      result = isClusterSetup(clusterName, zkClient);
+    } finally {
+      zkClient.close();
+    }
     return result;
   }
 
@@ -119,8 +123,12 @@ public final class ZKUtil {
   public static boolean isInstanceSetup(String zkAddress, String clusterName, String instanceName,
       InstanceType type) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    boolean result = isInstanceSetup(zkClient, clusterName, instanceName, type);
-    zkClient.close();
+    boolean result;
+    try {
+      result = isInstanceSetup(zkClient, clusterName, instanceName, type);
+    } finally {
+      zkClient.close();
+    }
     return result;
   }
 
@@ -166,8 +174,11 @@ public final class ZKUtil {
    */
   public static void createChildren(String zkAddress, String parentPath, List<ZNRecord>
list) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    createChildren(zkClient, parentPath, list);
-    zkClient.close();
+    try {
+      createChildren(zkClient, parentPath, list);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void createChildren(HelixZkClient client, String parentPath, List<ZNRecord>
list) {
@@ -188,8 +199,11 @@ public final class ZKUtil {
    */
   public static void createChildren(String zkAddress, String parentPath, ZNRecord nodeRecord)
{
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    createChildren(zkClient, parentPath, nodeRecord);
-    zkClient.close();
+    try {
+      createChildren(zkClient, parentPath, nodeRecord);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void createChildren(HelixZkClient client, String parentPath, ZNRecord nodeRecord)
{
@@ -209,8 +223,11 @@ public final class ZKUtil {
    */
   public static void dropChildren(String zkAddress, String parentPath, List<ZNRecord>
list) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    dropChildren(zkClient, parentPath, list);
-    zkClient.close();
+    try {
+      dropChildren(zkClient, parentPath, list);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void dropChildren(HelixZkClient client, String parentPath, List<ZNRecord>
list) {
@@ -231,8 +248,11 @@ public final class ZKUtil {
    */
   public static void dropChildren(String zkAddress, String parentPath, ZNRecord nodeRecord)
{
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    dropChildren(zkClient, parentPath, nodeRecord);
-    zkClient.close();
+    try {
+      dropChildren(zkClient, parentPath, nodeRecord);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void dropChildren(HelixZkClient client, String parentPath, ZNRecord nodeRecord)
{
@@ -251,8 +271,12 @@ public final class ZKUtil {
    */
   public static List<ZNRecord> getChildren(String zkAddress, String path) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    List<ZNRecord> result = getChildren(zkClient, path);
-    zkClient.close();
+    List<ZNRecord> result;
+    try {
+      result = getChildren(zkClient, path);
+    } finally {
+      zkClient.close();
+    }
     return result;
   }
 
@@ -290,8 +314,11 @@ public final class ZKUtil {
   public static void updateIfExists(String zkAddress, String path, final ZNRecord record,
       boolean mergeOnUpdate) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    updateIfExists(zkClient, path, record, mergeOnUpdate);
-    zkClient.close();
+    try {
+      updateIfExists(zkClient, path, record, mergeOnUpdate);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void updateIfExists(HelixZkClient client, String path, final ZNRecord record,
@@ -319,8 +346,11 @@ public final class ZKUtil {
   public static void createOrMerge(String zkAddress, String path, final ZNRecord record,
       final boolean persistent, final boolean mergeOnUpdate) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    createOrMerge(zkClient, path, record, persistent, mergeOnUpdate);
-    zkClient.close();
+    try {
+      createOrMerge(zkClient, path, record, persistent, mergeOnUpdate);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void createOrMerge(HelixZkClient client, String path, final ZNRecord record,
@@ -371,8 +401,11 @@ public final class ZKUtil {
   public static void createOrUpdate(String zkAddress, String path, final ZNRecord record,
       final boolean persistent, final boolean mergeOnUpdate) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    createOrUpdate(zkClient, path, record, persistent, mergeOnUpdate);
-    zkClient.close();
+    try {
+      createOrUpdate(zkClient, path, record, persistent, mergeOnUpdate);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void createOrUpdate(HelixZkClient client, String path, final ZNRecord record,
@@ -417,8 +450,11 @@ public final class ZKUtil {
   public static void asyncCreateOrMerge(String zkAddress, String path, final ZNRecord record,
       final boolean persistent, final boolean mergeOnUpdate) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    asyncCreateOrMerge(zkClient, path, record, persistent, mergeOnUpdate);
-    zkClient.close();
+    try {
+      asyncCreateOrMerge(zkClient, path, record, persistent, mergeOnUpdate);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void asyncCreateOrMerge(HelixZkClient client, String path, final ZNRecord
record,
@@ -467,8 +503,11 @@ public final class ZKUtil {
   public static void createOrReplace(String zkAddress, String path, final ZNRecord record,
       final boolean persistent) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    createOrReplace(zkClient, path, record, persistent);
-    zkClient.close();
+    try {
+      createOrReplace(zkClient, path, record, persistent);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void createOrReplace(HelixZkClient client, String path, final ZNRecord record,
@@ -507,8 +546,11 @@ public final class ZKUtil {
   public static void subtract(String zkAddress, final String path,
       final ZNRecord recordTosubtract) {
     HelixZkClient zkClient = getHelixZkClient(zkAddress);
-    subtract(zkClient, path, recordTosubtract);
-    zkClient.close();
+    try {
+      subtract(zkClient, path, recordTosubtract);
+    } finally {
+      zkClient.close();
+    }
   }
 
   public static void subtract(HelixZkClient client, final String path,
diff --git a/helix-core/src/main/java/org/apache/helix/tools/MessagePoster.java b/helix-core/src/main/java/org/apache/helix/tools/MessagePoster.java
index b1d6582..4d096e9 100644
--- a/helix-core/src/main/java/org/apache/helix/tools/MessagePoster.java
+++ b/helix-core/src/main/java/org/apache/helix/tools/MessagePoster.java
@@ -35,14 +35,18 @@ public class MessagePoster {
   public void post(String zkServer, Message message, String clusterName, String instanceName)
{
     HelixZkClient client = SharedZkClientFactory.getInstance().buildZkClient(new HelixZkClient.ZkConnectionConfig(
         zkServer));
-    client.setZkSerializer(new ZNRecordSerializer());
-    String path = PropertyPathBuilder.instanceMessage(clusterName, instanceName, message.getId());
-    client.delete(path);
-    ZNRecord record = client.readData(PropertyPathBuilder.liveInstance(clusterName, instanceName));
-    message.setTgtSessionId(record.getSimpleField(LiveInstanceProperty.SESSION_ID.toString()));
-    message.setTgtName(record.getId());
-    // System.out.println(message);
-    client.createPersistent(path, message.getRecord());
+    try {
+      client.setZkSerializer(new ZNRecordSerializer());
+      String path = PropertyPathBuilder.instanceMessage(clusterName, instanceName, message.getId());
+      client.delete(path);
+      ZNRecord record = client.readData(PropertyPathBuilder.liveInstance(clusterName, instanceName));
+      message.setTgtSessionId(record.getSimpleField(LiveInstanceProperty.SESSION_ID.toString()));
+      message.setTgtName(record.getId());
+      // System.out.println(message);
+      client.createPersistent(path, message.getRecord());
+    } finally {
+      client.close();
+    }
   }
 
   public void postFaultInjectionMessage(String zkServer, String clusterName, String instanceName,
diff --git a/helix-core/src/main/java/org/apache/helix/tools/commandtools/ZKDumper.java b/helix-core/src/main/java/org/apache/helix/tools/commandtools/ZKDumper.java
index cf7f22e..7d9eeec 100644
--- a/helix-core/src/main/java/org/apache/helix/tools/commandtools/ZKDumper.java
+++ b/helix-core/src/main/java/org/apache/helix/tools/commandtools/ZKDumper.java
@@ -142,20 +142,24 @@ public class ZKDumper {
     String fsPath = cmd.getOptionValue("fspath");
 
     ZKDumper zkDump = new ZKDumper(zkAddress);
-    if (download) {
-      if (cmd.hasOption("addSuffix")) {
-        zkDump.suffix = cmd.getOptionValue("addSuffix");
+    try {
+      if (download) {
+        if (cmd.hasOption("addSuffix")) {
+          zkDump.suffix = cmd.getOptionValue("addSuffix");
+        }
+        zkDump.download(zkPath, fsPath + zkPath);
       }
-      zkDump.download(zkPath, fsPath + zkPath);
-    }
-    if (upload) {
-      if (cmd.hasOption("removeSuffix")) {
-        zkDump.removeSuffix = true;
+      if (upload) {
+        if (cmd.hasOption("removeSuffix")) {
+          zkDump.removeSuffix = true;
+        }
+        zkDump.upload(zkPath, fsPath);
       }
-      zkDump.upload(zkPath, fsPath);
-    }
-    if (del) {
-      zkDump.delete(zkPath);
+      if (del) {
+        zkDump.delete(zkPath);
+      }
+    } finally {
+      zkDump.close();
     }
   }
 
@@ -231,4 +235,8 @@ public class ZKDumper {
       out.close();
     }
   }
+
+  public void close() {
+    client.close();
+  }
 }


Mime
View raw message