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

Deadlock while shutting down the Leader server.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.5.2, 3.6.0
    • Fix Version/s: 3.5.3, 3.6.0
    • Component/s: server
    • Labels:
      None

      Description

      Leader server enters into deadlock while shutting down. This happens some time only.
      The reason and deadlock flow is same as ZOOKEEPER-2380.
      shutdown was removed from synchronized block in ZOOKEEPER-2380
      Now shutdown is called from synchronized block from another place.

      // check leader running status
      if (!this.isRunning()) {
          shutdown("Unexpected internal error");
          return;
      }
      
      1. ZOOKEEPER-2687-01.patch
        0.7 kB
        Mohammad Arshad

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12851881/ZOOKEEPER-2687-01.patch
          against trunk revision 434abbbb7eef271fab02306bcc9c8ad29ec2fe2e.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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-Build/3576//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3576//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3576//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12851881/ZOOKEEPER-2687-01.patch against trunk revision 434abbbb7eef271fab02306bcc9c8ad29ec2fe2e. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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-Build/3576//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3576//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3576//console This message is automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user arshadmohammad opened a pull request:

          https://github.com/apache/zookeeper/pull/176

          ZOOKEEPER-2687:Deadlock while shutting down the Leader server.

          Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2687

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/zookeeper/pull/176.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #176


          commit 1e3ed70b281e1afc9adc6a8c8ea72bef5b9c25e8
          Author: Mohammad Arshad <arshad@apache.org>
          Date: 2017-02-13T19:48:07Z

          ZOOKEEPER-2687:Deadlock while shutting down the Leader server.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user arshadmohammad opened a pull request: https://github.com/apache/zookeeper/pull/176 ZOOKEEPER-2687 :Deadlock while shutting down the Leader server. Leader server enters into deadlock while shutting down itself. Shutdown of the leader server is called from the synchronized block which must be called from outside the synchronized block. For detail pls refer ZOOKEEPER-2380 You can merge this pull request into a Git repository by running: $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-2687 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/176.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #176 commit 1e3ed70b281e1afc9adc6a8c8ea72bef5b9c25e8 Author: Mohammad Arshad <arshad@apache.org> Date: 2017-02-13T19:48:07Z ZOOKEEPER-2687 :Deadlock while shutting down the Leader server.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user arshadmohammad commented on the issue:

          https://github.com/apache/zookeeper/pull/176

          There is not test case, as the change is very simple and understandable and the test will complicate the overall fix.

          Show
          githubbot ASF GitHub Bot added a comment - Github user arshadmohammad commented on the issue: https://github.com/apache/zookeeper/pull/176 There is not test case, as the change is very simple and understandable and the test will complicate the overall fix.
          Hide
          arshad.mohammad Mohammad Arshad added a comment -

          Rakesh R can you please have a look on the proposed change as you are most familiar with the related defect ZOOKEEPER-2380.

          Show
          arshad.mohammad Mohammad Arshad added a comment - Rakesh R can you please have a look on the proposed change as you are most familiar with the related defect ZOOKEEPER-2380 .
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user afine commented on the issue:

          https://github.com/apache/zookeeper/pull/176

          +1 lgtm

          looked through he code and i did not see any other places where this particular deadlock occurs

          Show
          githubbot ASF GitHub Bot added a comment - Github user afine commented on the issue: https://github.com/apache/zookeeper/pull/176 +1 lgtm looked through he code and i did not see any other places where this particular deadlock occurs
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rakeshadr commented on a diff in the pull request:

          https://github.com/apache/zookeeper/pull/176#discussion_r101259308

          — Diff: src/java/main/org/apache/zookeeper/server/quorum/Leader.java —
          @@ -590,8 +590,8 @@ void lead() throws IOException, InterruptedException {

          // check leader running status
          if (!this.isRunning()) {

          • shutdown("Unexpected internal error");
          • return;
            + shutdownMessage = "Unexpected internal error";
              • End diff –

          Good catch @arshadmohammad. Yes, this is another code where it can leads to deadlock similar to ZK-2380.

          As we know, `shutdownMessage` is acting as flag. Could you please add comments to make this clear,
          `// set shutdown flag`
          `shutdownMessage` = "Unexpected internal error";`

          Show
          githubbot ASF GitHub Bot added a comment - Github user rakeshadr commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/176#discussion_r101259308 — Diff: src/java/main/org/apache/zookeeper/server/quorum/Leader.java — @@ -590,8 +590,8 @@ void lead() throws IOException, InterruptedException { // check leader running status if (!this.isRunning()) { shutdown("Unexpected internal error"); return; + shutdownMessage = "Unexpected internal error"; End diff – Good catch @arshadmohammad. Yes, this is another code where it can leads to deadlock similar to ZK-2380. As we know, `shutdownMessage` is acting as flag. Could you please add comments to make this clear, `// set shutdown flag` `shutdownMessage` = "Unexpected internal error";`
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rakeshadr commented on the issue:

          https://github.com/apache/zookeeper/pull/176

          +1 LGTM, I will merge this to branch-3.5 and master shortly.

          Show
          githubbot ASF GitHub Bot added a comment - Github user rakeshadr commented on the issue: https://github.com/apache/zookeeper/pull/176 +1 LGTM, I will merge this to branch-3.5 and master shortly.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/zookeeper/pull/176

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/zookeeper/pull/176
          Hide
          rakeshr Rakesh R added a comment -

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

          Show
          rakeshr Rakesh R added a comment - Issue resolved by pull request 176 https://github.com/apache/zookeeper/pull/176
          Show
          rakeshr Rakesh R added a comment - Thanks Mohammad Arshad for the contribution. Thanks Abraham Fine for the reviews. Merged to branch-3.5 : https://git-wip-us.apache.org/repos/asf?p=zookeeper.git;a=commit;h=f07a836e3e24e9b643fecf0c0cfca3d7dbc60859 Merged to master : https://git-wip-us.apache.org/repos/asf?p=zookeeper.git;a=commit;h=fd211a5275b6231e668268fb9df2820e07f5f33c
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build ZooKeeper-trunk #3281 (See https://builds.apache.org/job/ZooKeeper-trunk/3281/)
          ZOOKEEPER-2687: Deadlock while shutting down the Leader server (rakeshr: rev fd211a5275b6231e668268fb9df2820e07f5f33c)

          • (edit) src/java/main/org/apache/zookeeper/server/quorum/Leader.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build ZooKeeper-trunk #3281 (See https://builds.apache.org/job/ZooKeeper-trunk/3281/ ) ZOOKEEPER-2687 : Deadlock while shutting down the Leader server (rakeshr: rev fd211a5275b6231e668268fb9df2820e07f5f33c) (edit) src/java/main/org/apache/zookeeper/server/quorum/Leader.java

            People

            • Assignee:
              arshad.mohammad Mohammad Arshad
              Reporter:
              arshad.mohammad Mohammad Arshad
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development