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

DelegationTokenRenewal attempts to renew token even after a job is removed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.1
    • Fix Version/s: 1.2.0
    • Component/s: security
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      mapreduce.security.token.DelegationTokenRenewal synchronizes on removeDelegationToken, but fails to synchronize on addToken, and renewing tokens in run().

      This inconsistency is exposed by frequent failures of TestDelegationTokenRenewal:

      Error Message
      
      renew wasn't called as many times as expected expected:<4> but was:<5>
      Stacktrace
      
      junit.framework.AssertionFailedError: renew wasn't called as many times as expected expected:<4> but was:<5>
      	at org.apache.hadoop.mapreduce.security.token.TestDelegationTokenRenewal.testDTRenewal(TestDelegationTokenRenewal.java:317)
      	at org.apache.hadoop.mapreduce.security.token.TestDelegationTokenRenewal.testDTRenewalAfterClose(TestDelegationTokenRenewal.java:338)
      
      
      1. mr-4860.patch
        4 kB
        Karthik Kambatla
      2. mr-4860.patch
        3 kB
        Karthik Kambatla
      3. mr-4860.patch
        2 kB
        Karthik Kambatla
      4. mr-4860.patch
        1 kB
        Karthik Kambatla
      5. mr-4860.patch
        1 kB
        Karthik Kambatla

        Issue Links

          Activity

          Karthik Kambatla made changes -
          Link This issue relates to MAPREDUCE-5384 [ MAPREDUCE-5384 ]
          Karthik Kambatla made changes -
          Link This issue relates to MAPREDUCE-5364 [ MAPREDUCE-5364 ]
          Matt Foley made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
          Alejandro Abdelnur made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Fix Version/s 1.2.0 [ 12321661 ]
          Resolution Fixed [ 1 ]
          Hide
          Alejandro Abdelnur added a comment -

          Thanks Karthik. Committed to branch-1.

          Show
          Alejandro Abdelnur added a comment - Thanks Karthik. Committed to branch-1.
          Hide
          Alejandro Abdelnur added a comment -

          +1

          Show
          Alejandro Abdelnur added a comment - +1
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12560821/mr-4860.patch
          against trunk revision .

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

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3122//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/12560821/mr-4860.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3122//console This message is automatically generated.
          Karthik Kambatla made changes -
          Attachment mr-4860.patch [ 12560821 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12560688/mr-4860.patch
          against trunk revision .

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

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3121//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/12560688/mr-4860.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3121//console This message is automatically generated.
          Karthik Kambatla made changes -
          Summary Inconsistent synchronization in mapreduce.security.token.DelegationTokenRenewal DelegationTokenRenewal attempts to renew token even after a job is removed
          Hide
          Karthik Kambatla added a comment -

          Reworked the patch to make it much simpler, and restricting the changes to RenewalTimerTask.

          Show
          Karthik Kambatla added a comment - Reworked the patch to make it much simpler, and restricting the changes to RenewalTimerTask.
          Karthik Kambatla made changes -
          Attachment mr-4860.patch [ 12560688 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12560439/mr-4860.patch
          against trunk revision .

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

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3119//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/12560439/mr-4860.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3119//console This message is automatically generated.
          Karthik Kambatla made changes -
          Attachment mr-4860.patch [ 12560439 ]
          Hide
          Karthik Kambatla added a comment -

          Thanks for pointing that out Alejandro. Verified that all operations on synchronized collections are synchronized on the returned object.

          Updated the patch to reflect that.

          Show
          Karthik Kambatla added a comment - Thanks for pointing that out Alejandro. Verified that all operations on synchronized collections are synchronized on the returned object. Updated the patch to reflect that.
          Hide
          Alejandro Abdelnur added a comment -

          delegationTokens is already a synchronized collection, except for the synchronization around the iterator() to avoid changes while iterating, I don't see why we need the others.

          Show
          Alejandro Abdelnur added a comment - delegationTokens is already a synchronized collection, except for the synchronization around the iterator() to avoid changes while iterating, I don't see why we need the others.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12559989/mr-4860.patch
          against trunk revision .

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

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3111//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/12559989/mr-4860.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3111//console This message is automatically generated.
          Karthik Kambatla made changes -
          Attachment mr-4860.patch [ 12559989 ]
          Karthik Kambatla made changes -
          Link This issue is cloned as MAPREDUCE-4862 [ MAPREDUCE-4862 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12559974/mr-4860.patch
          against trunk revision .

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

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3110//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/12559974/mr-4860.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3110//console This message is automatically generated.
          Karthik Kambatla made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Karthik Kambatla made changes -
          Attachment mr-4860.patch [ 12559974 ]
          Hide
          Karthik Kambatla added a comment -

          Taking a closer look, the problem seems to be with the use of TimerTask/Timer.

          If a task is already scheduled, calling task.cancel() from outside the task doesn't stop the scheduled task, but prevents the task from getting scheduled another time. However, the task can cancel itself.

          The patch:

          1. adds a check within the task if it has been stopped from outside
          2. synchronizes changes to delegationTokens
          Show
          Karthik Kambatla added a comment - Taking a closer look, the problem seems to be with the use of TimerTask/Timer. If a task is already scheduled, calling task.cancel() from outside the task doesn't stop the scheduled task, but prevents the task from getting scheduled another time. However, the task can cancel itself. The patch: adds a check within the task if it has been stopped from outside synchronizes changes to delegationTokens
          Karthik Kambatla made changes -
          Link This issue relates to MAPREDUCE-4861 [ MAPREDUCE-4861 ]
          Karthik Kambatla made changes -
          Affects Version/s 2.0.2-alpha [ 12322471 ]
          Karthik Kambatla made changes -
          Link This issue relates to MAPREDUCE-4795 [ MAPREDUCE-4795 ]
          Karthik Kambatla made changes -
          Field Original Value New Value
          Description mapreduce.security.token.DelegationTokenRenewal synchronizes on removeDelegationToken, but fails to synchronize on addToken, and renewing tokens in run().

          mapreduce.security.token.DelegationTokenRenewal synchronizes on removeDelegationToken, but fails to synchronize on addToken, and renewing tokens in run().

          This inconsistency is exposed by frequent failures of TestDelegationTokenRenewal:
          {noformat}
          Error Message

          renew wasn't called as many times as expected expected:<4> but was:<5>
          Stacktrace

          junit.framework.AssertionFailedError: renew wasn't called as many times as expected expected:<4> but was:<5>
          at org.apache.hadoop.mapreduce.security.token.TestDelegationTokenRenewal.testDTRenewal(TestDelegationTokenRenewal.java:317)
          at org.apache.hadoop.mapreduce.security.token.TestDelegationTokenRenewal.testDTRenewalAfterClose(TestDelegationTokenRenewal.java:338)

          {noformat}
          Hide
          Karthik Kambatla added a comment -

          In fact, I wonder why we can't use common.DelegationTokenRenewer for token renewal. That avoids duplicating code too.

          Show
          Karthik Kambatla added a comment - In fact, I wonder why we can't use common.DelegationTokenRenewer for token renewal. That avoids duplicating code too.
          Karthik Kambatla created issue -

            People

            • Assignee:
              Karthik Kambatla
              Reporter:
              Karthik Kambatla
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development