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/12/17 00:50:09 UTC
[helix] branch master updated: Patching zkClient leakage fix #656
(#657)
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 <58...@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();
+ }
}