Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-2452

Delegation token cancellation shouldn't hold global JobTracker lock

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.1
    • Component/s: jobtracker
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently, when the JobTracker cancels a job's delegation token (at the end of the job), it holds the global lock. This is not desired.

      1. cancel-delegation-token-fix-trunk.patch
        5 kB
        Devaraj Das
      2. cancel-delegation-token-fix-trunk.patch
        5 kB
        Devaraj Das
      3. cancel-delegation-token-fix.patch
        4 kB
        Devaraj Das
      4. cancel-delegation-token-fix.patch
        4 kB
        Devaraj Das
      5. cancel-delegation-token-fix.patch
        4 kB
        Devaraj Das

        Issue Links

          Activity

          Hide
          Owen O'Malley added a comment -

          Devaraj,
          Can you please have the Thread catch IOExceptions and log them. Throwable should be caught, logged, and call System.exit.

          I think you need to retry if the offer returns false rather than just dropping the cancel request.

          Show
          Owen O'Malley added a comment - Devaraj, Can you please have the Thread catch IOExceptions and log them. Throwable should be caught, logged, and call System.exit. I think you need to retry if the offer returns false rather than just dropping the cancel request.
          Hide
          Devaraj Das added a comment -

          I should ideally put a small sleep between the retries whenever 'offer' returns false. That would need InterruptedException to be handled. I ideally don't want to swallow the InterruptedException but if I don't it leads to a big cascading change in the method declarations in the call hierarchy of the token cancellation method... I am in two minds on whether to swallow/ignore the interrupted exception or propagate it all the way up and effect a major change in the method signatures...

          Show
          Devaraj Das added a comment - I should ideally put a small sleep between the retries whenever 'offer' returns false. That would need InterruptedException to be handled. I ideally don't want to swallow the InterruptedException but if I don't it leads to a big cascading change in the method declarations in the call hierarchy of the token cancellation method... I am in two minds on whether to swallow/ignore the interrupted exception or propagate it all the way up and effect a major change in the method signatures...
          Hide
          Devaraj Das added a comment -

          Patch addressing Owen's comments.

          Show
          Devaraj Das added a comment - Patch addressing Owen's comments.
          Hide
          Owen O'Malley added a comment -

          Looking good. A few more points:

          • Please have the cancelToken catch the InterruptedException and create a new RuntimeException that wraps it.
          • Please have the DelegationTokenCancelThread catch InterruptedException and exit gracefully.
          Show
          Owen O'Malley added a comment - Looking good. A few more points: Please have the cancelToken catch the InterruptedException and create a new RuntimeException that wraps it. Please have the DelegationTokenCancelThread catch InterruptedException and exit gracefully.
          Hide
          Devaraj Das added a comment -

          Addresses Owen's comments..

          Show
          Devaraj Das added a comment - Addresses Owen's comments..
          Hide
          Owen O'Malley added a comment -

          +1

          Show
          Owen O'Malley added a comment - +1
          Hide
          Devaraj Das added a comment -

          Patch for trunk

          Show
          Devaraj Das added a comment - Patch for trunk
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481158/cancel-delegation-token-fix-trunk.patch
          against trunk revision 1130147.

          +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 appears to introduce 2 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 these core unit tests:
          org.apache.hadoop.cli.TestMRCLI

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/330//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/330//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/330//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/12481158/cancel-delegation-token-fix-trunk.patch against trunk revision 1130147. +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 appears to introduce 2 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 these core unit tests: org.apache.hadoop.cli.TestMRCLI +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/330//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/330//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/330//console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          The test failed without the patch too. The findbugs warning (to do with System.exit) is expected. Not sure why it is flagging two warnings..

          Show
          Devaraj Das added a comment - The test failed without the patch too. The findbugs warning (to do with System.exit) is expected. Not sure why it is flagging two warnings..
          Hide
          Owen O'Malley added a comment -

          +1

          Please add a suppression of the exit findbugs warning to src/test/findbugsExcludeFile.xml.

          Show
          Owen O'Malley added a comment - +1 Please add a suppression of the exit findbugs warning to src/test/findbugsExcludeFile.xml.
          Hide
          Devaraj Das added a comment -

          Suppresses the findbugs warning.

          Show
          Devaraj Das added a comment - Suppresses the findbugs warning.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481390/cancel-delegation-token-fix-trunk.patch
          against trunk revision 1130994.

          +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 appears to introduce 1 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 these core unit tests:
          org.apache.hadoop.cli.TestMRCLI

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/344//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/344//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/344//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/12481390/cancel-delegation-token-fix-trunk.patch against trunk revision 1130994. +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 appears to introduce 1 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 these core unit tests: org.apache.hadoop.cli.TestMRCLI +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/344//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/344//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/344//console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          Compared the number of findbugs warnings with the number of findbugs warnings for another recent hudson run. They are the same. Not sure why this patch is triggering false findbugs alarms. I intend to commit the patch soon..

          Show
          Devaraj Das added a comment - Compared the number of findbugs warnings with the number of findbugs warnings for another recent hudson run. They are the same. Not sure why this patch is triggering false findbugs alarms. I intend to commit the patch soon..
          Hide
          Devaraj Das added a comment -

          I just committed this.

          Show
          Devaraj Das added a comment - I just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #712 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/712/)
          MAPREDUCE-2452. Makes the cancellation of delegation tokens happen in a separate thread. Contributed by Devaraj Das.

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

          • /hadoop/mapreduce/trunk/CHANGES.txt
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/security/token/DelegationTokenRenewal.java
          • /hadoop/mapreduce/trunk/src/test/findbugsExcludeFile.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #712 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/712/ ) MAPREDUCE-2452 . Makes the cancellation of delegation tokens happen in a separate thread. Contributed by Devaraj Das. ddas : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1131265 Files : /hadoop/mapreduce/trunk/CHANGES.txt /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/security/token/DelegationTokenRenewal.java /hadoop/mapreduce/trunk/src/test/findbugsExcludeFile.xml
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #722 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/722/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #722 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/722/ )
          Hide
          Benoy Antony added a comment -

          Patch for 22

          Show
          Benoy Antony added a comment - Patch for 22
          Hide
          Benoy Antony added a comment -

          I removed my patch for 22. The latest patch for trunk is good for 22

          Show
          Benoy Antony added a comment - I removed my patch for 22. The latest patch for trunk is good for 22
          Hide
          Konstantin Shvachko added a comment -

          I just committed this to branch 0.22.1. Thank you Benoy.

          Show
          Konstantin Shvachko added a comment - I just committed this to branch 0.22.1. Thank you Benoy.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-22-branch #104 (See https://builds.apache.org/job/Hadoop-Mapreduce-22-branch/104/)
          MAPREDUCE-2452. Moves the cancellation of delegation tokens to a separate thread. Contributed by Devaraj Das and Benoy Antony. (Revision 1346235)

          Result = SUCCESS
          shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1346235
          Files :

          • /hadoop/common/branches/branch-0.22/mapreduce/CHANGES.txt
          • /hadoop/common/branches/branch-0.22/mapreduce/src/java/org/apache/hadoop/mapreduce/security/token/DelegationTokenRenewal.java
          • /hadoop/common/branches/branch-0.22/mapreduce/src/test/findbugsExcludeFile.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-22-branch #104 (See https://builds.apache.org/job/Hadoop-Mapreduce-22-branch/104/ ) MAPREDUCE-2452 . Moves the cancellation of delegation tokens to a separate thread. Contributed by Devaraj Das and Benoy Antony. (Revision 1346235) Result = SUCCESS shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1346235 Files : /hadoop/common/branches/branch-0.22/mapreduce/CHANGES.txt /hadoop/common/branches/branch-0.22/mapreduce/src/java/org/apache/hadoop/mapreduce/security/token/DelegationTokenRenewal.java /hadoop/common/branches/branch-0.22/mapreduce/src/test/findbugsExcludeFile.xml

            People

            • Assignee:
              Devaraj Das
              Reporter:
              Devaraj Das
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development