Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-2589

unnecessary hftp token fetch and renewal thread

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0
    • Fix Version/s: 1.0.0
    • Component/s: security
    • Labels:
      None
    • Target Version/s:

      Description

      Instantiation of the hftp filesystem is causing a token to be implicitly created and added to a custom token renewal thread. With the new token renewal feature in the JT, this causes the mapreduce obtainTokensForNamenodes to fetch two tokens (an implicit and uncancelled token, and an explicit token) and leave a spurious renewal thread running. This thread should not be running in the JT.

      After speaking with Owen, the quick solution is to lazy fetch the token, and to lazy start the renewer thread.

        Issue Links

          Activity

          Hide
          Daryn Sharp added a comment -

          implements lazy fetch and thread

          Show
          Daryn Sharp added a comment - implements lazy fetch and thread
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 3 new or modified tests.

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

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1607//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/12504914/HDFS-2589.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1607//console This message is automatically generated.
          Hide
          Jitendra Nath Pandey added a comment -

          I am not too sure about remoteIsInsecure check, because it will be set to true for any kind of exception.
          The patch looks ok to me, otherwise.

          Show
          Jitendra Nath Pandey added a comment - I am not too sure about remoteIsInsecure check, because it will be set to true for any kind of exception. The patch looks ok to me, otherwise.
          Hide
          Daryn Sharp added a comment -

          I 100% agree the error handling is less than ideal, but I don't believe I fundamentally changed the behavior. The pre-existing code already assumed that any exception means the other side is insecure. I only made it defer the fetching of the token.

          Show
          Daryn Sharp added a comment - I 100% agree the error handling is less than ideal, but I don't believe I fundamentally changed the behavior. The pre-existing code already assumed that any exception means the other side is insecure. I only made it defer the fetching of the token.
          Hide
          Jitendra Nath Pandey added a comment -

          +1.

          Show
          Jitendra Nath Pandey added a comment - +1.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Hi Daryn,

          It seems that it is better to change HftpFileSystem but not DelegationTokenRenewer.

          In WebHdfsFileSystem, the lazy initialization is implemented in addRenewAction(..), which is the only method using DT_RENEWER. It is more lazy – we even don't have to create a DelegationTokenRenewer object if addRenewAction(..) is not invoked.

          //WebHdfsFileSystem
            private static DelegationTokenRenewer<WebHdfsFileSystem> DT_RENEWER = null;
          
            private static synchronized void addRenewAction(final WebHdfsFileSystem webhdfs) {
              if (DT_RENEWER == null) {
                DT_RENEWER = new DelegationTokenRenewer<WebHdfsFileSystem>(WebHdfsFileSystem.class);
                DT_RENEWER.start();
              }
          
              DT_RENEWER.addRenewAction(webhdfs);
            }
          
          Show
          Tsz Wo Nicholas Sze added a comment - Hi Daryn, It seems that it is better to change HftpFileSystem but not DelegationTokenRenewer. In WebHdfsFileSystem, the lazy initialization is implemented in addRenewAction(..), which is the only method using DT_RENEWER. It is more lazy – we even don't have to create a DelegationTokenRenewer object if addRenewAction(..) is not invoked. //WebHdfsFileSystem private static DelegationTokenRenewer<WebHdfsFileSystem> DT_RENEWER = null ; private static synchronized void addRenewAction( final WebHdfsFileSystem webhdfs) { if (DT_RENEWER == null ) { DT_RENEWER = new DelegationTokenRenewer<WebHdfsFileSystem>(WebHdfsFileSystem.class); DT_RENEWER.start(); } DT_RENEWER.addRenewAction(webhdfs); }
          Hide
          Matt Foley added a comment -

          Committed to 1.0.0 and branch-1 on the strength of Jitendra's +1. Feel free to improve in 1.1.0.

          Show
          Matt Foley added a comment - Committed to 1.0.0 and branch-1 on the strength of Jitendra's +1. Feel free to improve in 1.1.0.
          Hide
          Matt Foley added a comment -

          Closed upon release of version 1.0.0.

          Show
          Matt Foley added a comment - Closed upon release of version 1.0.0.

            People

            • Assignee:
              Daryn Sharp
              Reporter:
              Daryn Sharp
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development