ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1321

Add number of client connections metric in JMX and srvr

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3.4, 3.4.2
    • Fix Version/s: 3.4.4, 3.5.0
    • Component/s: None
    • Labels:

      Description

      The related conversation on the zookeeper user mailing list is here - http://apache.markmail.org/message/4jjcmooniowwugu2?q=+list:org.apache.hadoop.zookeeper-user

      It is useful to be able to monitor the number of disconnect operations on a client. This is generally indicative of a client going through large number of GC and hence disconnecting way too often from a zookeeper cluster.

      Today, this information is only indirectly exposed as part of the stat command which requires counting the results. That's alot of work for the server to do just to get connection count.

      For monitoring purposes, it will be useful to have this exposed through JMX and 4lw srvr.

      1. ZOOKEEPER-1321_trunk.patch
        11 kB
        Neha Narkhede
      2. ZOOKEEPER-1321_3.4.patch
        10 kB
        Neha Narkhede
      3. ZOOKEEPER-1321_trunk.patch
        11 kB
        Neha Narkhede
      4. zookeeper-1321-trunk-v2.patch
        11 kB
        Neha Narkhede
      5. zk-1321-cleanup
        11 kB
        Camille Fournier
      6. zk-1321.patch
        32 kB
        Henry Robinson
      7. zk-1321-trunk.patch
        32 kB
        Henry Robinson
      8. ZK-1321-nowhitespace.patch
        11 kB
        Henry Robinson

        Activity

        Hide
        Neha Narkhede added a comment -

        This patch exposes the number of active client connections to a zookeeper server through JMX as well as srvr, stat and mntr.

        Show
        Neha Narkhede added a comment - This patch exposes the number of active client connections to a zookeeper server through JMX as well as srvr, stat and mntr.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12508429/ZOOKEEPER-1321_trunk.patch
        against trunk revision 1221868.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/851//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/851//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/851//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12508429/ZOOKEEPER-1321_trunk.patch against trunk revision 1221868. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/851//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/851//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/851//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12508429/ZOOKEEPER-1321_trunk.patch
        against trunk revision 1225059.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/864//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/864//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/864//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12508429/ZOOKEEPER-1321_trunk.patch against trunk revision 1225059. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/864//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/864//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/864//console This message is automatically generated.
        Hide
        Camille Fournier added a comment -

        Looks good modulo an unneeded import in ServerCnxnFactory. I will check this in.

        Show
        Camille Fournier added a comment - Looks good modulo an unneeded import in ServerCnxnFactory. I will check this in.
        Hide
        Camille Fournier added a comment -

        Neha, if you want this in 3.4 will you make me a patch that applies to that branch? It's failing to apply for ZooKeeperServer. Thanks.

        Show
        Camille Fournier added a comment - Neha, if you want this in 3.4 will you make me a patch that applies to that branch? It's failing to apply for ZooKeeperServer. Thanks.
        Hide
        Patrick Hunt added a comment -

        I reported an issue to the list - the call to cnxns.size() is not synchronized and I think it needs to be.

        Show
        Patrick Hunt added a comment - I reported an issue to the list - the call to cnxns.size() is not synchronized and I think it needs to be.
        Hide
        Neha Narkhede added a comment -

        >> Neha, if you want this in 3.4 will you make me a patch that applies to that branch?

        Certainly. I will submit another patch.

        >> the call to cnxns.size() is not synchronized

        I will look into this. How about I submit another patch with this fixed and the unrequired import removed ?

        Show
        Neha Narkhede added a comment - >> Neha, if you want this in 3.4 will you make me a patch that applies to that branch? Certainly. I will submit another patch. >> the call to cnxns.size() is not synchronized I will look into this. How about I submit another patch with this fixed and the unrequired import removed ?
        Hide
        Camille Fournier added a comment -

        Sounds good. Remove the TODO added in the Zab1_0Test too please!

        Show
        Camille Fournier added a comment - Sounds good. Remove the TODO added in the Zab1_0Test too please!
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1411 (See https://builds.apache.org/job/ZooKeeper-trunk/1411/)
        revert ZOOKEEPER-1321
        ZOOKEEPER-1321: Add number of client connections metric in JMX and srvr (Neha Narkhede via camille)

        camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1225352
        Files :

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java

        camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1225200
        Files :

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1411 (See https://builds.apache.org/job/ZooKeeper-trunk/1411/ ) revert ZOOKEEPER-1321 ZOOKEEPER-1321 : Add number of client connections metric in JMX and srvr (Neha Narkhede via camille) camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1225352 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1225200 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java
        Hide
        Patrick Hunt added a comment -

        Cancelling patch until feedback is addressed.

        Show
        Patrick Hunt added a comment - Cancelling patch until feedback is addressed.
        Hide
        Neha Narkhede added a comment -

        Patches for both trunk and the 3.4 branch. Changes include -

        1. Synchronized access to cnxns.size()
        2. Removed the TODO in Zab1_0Test

        Show
        Neha Narkhede added a comment - Patches for both trunk and the 3.4 branch. Changes include - 1. Synchronized access to cnxns.size() 2. Removed the TODO in Zab1_0Test
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12509942/ZOOKEEPER-1321_trunk.patch
        against trunk revision 1227927.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/890//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/890//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/890//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12509942/ZOOKEEPER-1321_trunk.patch against trunk revision 1227927. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/890//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/890//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/890//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12509942/ZOOKEEPER-1321_trunk.patch
        against trunk revision 1227927.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/891//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/891//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/891//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12509942/ZOOKEEPER-1321_trunk.patch against trunk revision 1227927. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/891//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/891//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/891//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12510515/zookeeper-1321-trunk-v2.patch
        against trunk revision 1227927.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/902//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/902//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/902//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12510515/zookeeper-1321-trunk-v2.patch against trunk revision 1227927. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/902//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/902//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/902//console This message is automatically generated.
        Hide
        Camille Fournier added a comment -

        clean up some tabs into spaces and refactor the cnxns and one of the methods into the superclass. It just seems a shame to copy-paste when we can clean up.

        Show
        Camille Fournier added a comment - clean up some tabs into spaces and refactor the cnxns and one of the methods into the superclass. It just seems a shame to copy-paste when we can clean up.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12510655/zk-1321-cleanup
        against trunk revision 1231389.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/907//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/907//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/907//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12510655/zk-1321-cleanup against trunk revision 1231389. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/907//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/907//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/907//console This message is automatically generated.
        Hide
        Camille Fournier added a comment -

        If one of the other committers wants take a quick glance at the cleanup patch that would be great, I can then check it in with your ok.

        Show
        Camille Fournier added a comment - If one of the other committers wants take a quick glance at the cleanup patch that would be great, I can then check it in with your ok.
        Hide
        Henry Robinson added a comment -

        +1 looks good! Only weirdness to my eyes is the following:

        +    public int getNumAliveConnections() {
        +        int numConnections;
        +        synchronized(cnxns) {
        +            numConnections = cnxns.size();
        +        }
        +        return numConnections;
        +    }
        

        It's perfectly legal to return inside a synchronized block, so it might be more concise to have:

        
        +    public int getNumAliveConnections() {
        +        synchronized(cnxns) {
        +            return cnxns.size();
        +        }
        +    }
        

        If you fix this nit I'm happy for you to commit this without another review pass.

        Show
        Henry Robinson added a comment - +1 looks good! Only weirdness to my eyes is the following: + public int getNumAliveConnections() { + int numConnections; + synchronized (cnxns) { + numConnections = cnxns.size(); + } + return numConnections; + } It's perfectly legal to return inside a synchronized block, so it might be more concise to have: + public int getNumAliveConnections() { + synchronized (cnxns) { + return cnxns.size(); + } + } If you fix this nit I'm happy for you to commit this without another review pass.
        Hide
        Henry Robinson added a comment -

        New patch with nit addressed and all added trailing whitespace removed (in fact, all whitespace removed from affected files period).

        Show
        Henry Robinson added a comment - New patch with nit addressed and all added trailing whitespace removed (in fact, all whitespace removed from affected files period).
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12512336/zk-1321.patch
        against trunk revision 1234974.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/932//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12512336/zk-1321.patch against trunk revision 1234974. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/932//console This message is automatically generated.
        Hide
        Henry Robinson added a comment -

        Correctly generated patch.

        Show
        Henry Robinson added a comment - Correctly generated patch.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12512337/zk-1321-trunk.patch
        against trunk revision 1234974.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/933//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/933//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/933//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12512337/zk-1321-trunk.patch against trunk revision 1234974. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/933//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/933//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/933//console This message is automatically generated.
        Hide
        Camille Fournier added a comment -

        This whitespace cleanup is creating a huge diff for a very little change. Are we sure we want to do this for the benefit of removing extra whitespace at the end of lines? I'd like to get the patch in but I'm not crazy about making it look like a lot more has changed than actually did.

        Show
        Camille Fournier added a comment - This whitespace cleanup is creating a huge diff for a very little change. Are we sure we want to do this for the benefit of removing extra whitespace at the end of lines? I'd like to get the patch in but I'm not crazy about making it look like a lot more has changed than actually did.
        Hide
        Henry Robinson added a comment -

        I take your point. I'll regenerate the patch without the whitespace difference. I'd like to figure out how to handle all our sloppy trailing whitespace at some point, but there's already too much in each file just to sneak it in patch-by-patch.

        Show
        Henry Robinson added a comment - I take your point. I'll regenerate the patch without the whitespace difference. I'd like to figure out how to handle all our sloppy trailing whitespace at some point, but there's already too much in each file just to sneak it in patch-by-patch.
        Hide
        Henry Robinson added a comment -

        Same diff with trailing whitespace left in.

        Show
        Henry Robinson added a comment - Same diff with trailing whitespace left in.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12513465/ZK-1321-nowhitespace.patch
        against trunk revision 1240959.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/951//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/951//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/951//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12513465/ZK-1321-nowhitespace.patch against trunk revision 1240959. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/951//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/951//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/951//console This message is automatically generated.
        Hide
        Camille Fournier added a comment -

        Great. I'm going to check this in now.

        Show
        Camille Fournier added a comment - Great. I'm going to check this in now.
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1456 (See https://builds.apache.org/job/ZooKeeper-trunk/1456/)
        ZOOKEEPER-1321. Add number of client connections metric in JMX and srvr. (Neha N. via camille)

        camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1242982
        Files :

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1456 (See https://builds.apache.org/job/ZooKeeper-trunk/1456/ ) ZOOKEEPER-1321 . Add number of client connections metric in JMX and srvr. (Neha N. via camille) camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1242982 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java

          People

          • Assignee:
            Neha Narkhede
            Reporter:
            Neha Narkhede
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development