zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From h...@apache.org
Subject zookeeper git commit: ZOOKEEPER-2743: Netty connection leaks JMX connection bean.
Date Mon, 10 Apr 2017 17:19:39 GMT
Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.4 13ca3f3b8 -> c14cdd367


ZOOKEEPER-2743: Netty connection leaks JMX connection bean.

See https://issues.apache.org/jira/browse/ZOOKEEPER-2743 for details on the symptom and diagnostic.

There are many ways of fixing this. For example we can enforce certain ordering of the close
operations to eliminate the race condition however that would incur non trivial performance
penalty as a result of synchronization. I think the solution in this patch is simple and effective
as it observes that the bean unregister call is idempotent and the call itself is trivial
so we just always unregister connection upon close the connection, to ensure the bean is unregistered
regardless of the races.

I've also stress tested this solution on internal Jenkins which indicates no failures of https://issues.apache.org/jira/browse/ZOOKEEPER-2707
for the past two days.

A side note is NIO connection in theory would suffer the same problem however I am unable
to reproduce the same racing with existing unit test. So I just leave NIO as it is, for now.

Author: Michael Han <hanm@apache.org>

Reviewers: Rakesh Radhakrishnan <rakeshr@apache.org>

Closes #211 from hanm/ZOOKEEPER-2743

(cherry picked from commit b3ef40bffe49c1e42aee23a600a775194f36bca0)
Signed-off-by: Michael Han <hanm@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/c14cdd36
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/c14cdd36
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/c14cdd36

Branch: refs/heads/branch-3.4
Commit: c14cdd3673ebdd8014118ec961068ee51f2327b1
Parents: 13ca3f3
Author: Michael Han <hanm@apache.org>
Authored: Mon Apr 10 10:19:34 2017 -0700
Committer: Michael Han <hanm@apache.org>
Committed: Mon Apr 10 10:19:47 2017 -0700

----------------------------------------------------------------------
 .../main/org/apache/zookeeper/server/NettyServerCnxn.java     | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/c14cdd36/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
index 203f0e6..99058d1 100644
--- a/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
+++ b/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
@@ -89,6 +89,12 @@ public class NettyServerCnxn extends ServerCnxn {
             LOG.debug("close called for sessionid:0x"
                     + Long.toHexString(sessionId));
         }
+
+        // ZOOKEEPER-2743:
+        // Always unregister connection upon close to prevent
+        // connection bean leak under certain race conditions.
+        factory.unregisterConnection(this);
+
         synchronized(factory.cnxns){
             // if this is not in cnxns then it's already closed
             if (!factory.cnxns.remove(this)) {
@@ -113,7 +119,6 @@ public class NettyServerCnxn extends ServerCnxn {
         if (channel.isOpen()) {
             channel.close();
         }
-        factory.unregisterConnection(this);
     }
 
     @Override


Mime
View raw message