Uploaded image for project: 'Hadoop Map/Reduce'
  1. Hadoop Map/Reduce
  2. MAPREDUCE-4860

DelegationTokenRenewal attempts to renew token even after a job is removed

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
        1 kB
        Karthik Kambatla
      2. mr-4860.patch
        1 kB
        Karthik Kambatla
      3. mr-4860.patch
        2 kB
        Karthik Kambatla
      4. mr-4860.patch
        3 kB
        Karthik Kambatla
      5. mr-4860.patch
        4 kB
        Karthik Kambatla

        Issue Links

          Activity

          Hide
          kkambatl Karthik Kambatla (Inactive) added a comment -

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

          Show
          kkambatl Karthik Kambatla (Inactive) added a comment - In fact, I wonder why we can't use common.DelegationTokenRenewer for token renewal. That avoids duplicating code too.
          Hide
          kkambatl Karthik Kambatla (Inactive) 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
          kkambatl Karthik Kambatla (Inactive) 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
          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/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
          hadoopqa 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.
          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/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
          hadoopqa 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.
          Hide
          tucu00 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
          tucu00 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
          kkambatl Karthik Kambatla (Inactive) 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
          kkambatl Karthik Kambatla (Inactive) 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
          hadoopqa 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
          hadoopqa 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.
          Hide
          kkambatl Karthik Kambatla (Inactive) added a comment -

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

          Show
          kkambatl Karthik Kambatla (Inactive) added a comment - Reworked the patch to make it much simpler, and restricting the changes to RenewalTimerTask.
          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/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
          hadoopqa 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.
          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/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
          hadoopqa 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.
          Hide
          tucu00 Alejandro Abdelnur added a comment -

          +1

          Show
          tucu00 Alejandro Abdelnur added a comment - +1
          Hide
          tucu00 Alejandro Abdelnur added a comment -

          Thanks Karthik. Committed to branch-1.

          Show
          tucu00 Alejandro Abdelnur added a comment - Thanks Karthik. Committed to branch-1.
          Hide
          mattf Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          mattf Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development