Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-3059

EventThread leak in case of Sasl AuthFailed

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • 3.4.12
    • 3.6.0, 3.5.5
    • None

    Description

      In case of an authFailed sasl event we shutdown the send thread however we never close the event thread. Even if the client tries to close the connection it results in a no-op since we check for cnxn.getState().isAlive() which results in negative for auth failed state and we return without cleaning up. For applications that retry in case of auth failed by closing the existing connection and then trying to reconnect(eg. hbase replication) this eventually ends up exhausting the system resources.

      Attachments

        1. stack_dump
          1.66 MB
          Abhishek Singh Chouhan

        Issue Links

          Activity

            hudson Hudson added a comment -

            FAILURE: Integrated in Jenkins build ZooKeeper-trunk #74 (See https://builds.apache.org/job/ZooKeeper-trunk/74/)
            ZOOKEEPER-3059: EventThread leak in case of Sasl AuthFailed (andor: rev 1fb644662b8e0530dec2c5668a3e49b3f614e9de)

            • (edit) src/java/main/org/apache/zookeeper/ClientCnxn.java
            • (edit) src/java/test/org/apache/zookeeper/SaslAuthTest.java
            hudson Hudson added a comment - FAILURE: Integrated in Jenkins build ZooKeeper-trunk #74 (See https://builds.apache.org/job/ZooKeeper-trunk/74/ ) ZOOKEEPER-3059 : EventThread leak in case of Sasl AuthFailed (andor: rev 1fb644662b8e0530dec2c5668a3e49b3f614e9de) (edit) src/java/main/org/apache/zookeeper/ClientCnxn.java (edit) src/java/test/org/apache/zookeeper/SaslAuthTest.java
            andor Andor Molnar added a comment -

            Issue resolved by pull request 541
            https://github.com/apache/zookeeper/pull/541

            andor Andor Molnar added a comment - Issue resolved by pull request 541 https://github.com/apache/zookeeper/pull/541
            hadoopqa Hadoop QA added a comment -

            +1 overall. GitHub Pull Request Build

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

            +1 tests included. The patch appears to include 3 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 3.0.1) 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-github-pr-build/1860//testReport/
            Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1860//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
            Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1860//console

            This message is automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 3.0.1) 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-github-pr-build/1860//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1860//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1860//console This message is automatically generated.
            hadoopqa Hadoop QA added a comment -

            -1 overall. GitHub Pull Request Build

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

            +1 tests included. The patch appears to include 3 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 3.0.1) 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-github-pr-build/1850//testReport/
            Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1850//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
            Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1850//console

            This message is automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 3.0.1) 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-github-pr-build/1850//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1850//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1850//console This message is automatically generated.
            andor Andor Molnar added a comment -

            Of course, anytime abhishek.chouhan

            Sorry for the delayed feedback, I'm currently reviewing and trying your patch.

            andor Andor Molnar added a comment - Of course, anytime abhishek.chouhan Sorry for the delayed feedback, I'm currently reviewing and trying your patch.
            abhishek.chouhan Abhishek Singh Chouhan added a comment - - edited

            hanm phunt andor Can i bother you guys for a review?

            abhishek.chouhan Abhishek Singh Chouhan added a comment - - edited hanm phunt andor Can i bother you guys for a review?
            abhishek.chouhan Abhishek Singh Chouhan added a comment - - edited

            andor Thanks for pointing out the jira. I was earlier thinking about taking a similar approach and making the close call less restrictive so as to work in the case of auth failed too, however going through the documentation a bit i see that auth_failed is considered similar to session expired (both are fatal events) and for session expire we do kill the event thread automatically, hence i went with a similar approach to kill the event thread in case of auth failed too, rather than leaving it to the user (by changing the close method and expecting the event thread to be shutdown when that is called post auth failed). What do you think? 

            abhishek.chouhan Abhishek Singh Chouhan added a comment - - edited andor Thanks for pointing out the jira. I was earlier thinking about taking a similar approach and making the close call less restrictive so as to work in the case of auth failed too, however going through the documentation a bit i see that auth_failed is considered similar to session expired (both are fatal events) and for session expire we do kill the event thread automatically, hence i went with a similar approach to kill the event thread in case of auth failed too, rather than leaving it to the user (by changing the close method and expecting the event thread to be shutdown when that is called post auth failed). What do you think? 
            andor Andor Molnar added a comment -

            abhishek.chouhan Thanks for the contribution.

            This one reminds me the other issue I've just linked. You might want to take a look at the patch attached to it.

            andor Andor Molnar added a comment - abhishek.chouhan Thanks for the contribution. This one reminds me the other issue I've just linked. You might want to take a look at the patch attached to it.
            hadoopqa Hadoop QA added a comment -

            -1 overall. GitHub Pull Request Build

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

            +0 tests included. The patch appears to be a documentation patch that doesn't require 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 3.0.1) 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-github-pr-build/1826//testReport/
            Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1826//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
            Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1826//console

            This message is automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall. GitHub Pull Request Build +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require 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 3.0.1) 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-github-pr-build/1826//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1826//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1826//console This message is automatically generated.

            People

              abhishek.chouhan Abhishek Singh Chouhan
              abhishek.chouhan Abhishek Singh Chouhan
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 2h 10m
                  2h 10m