Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-11689

New exception thrown by DFSClient#isHDFSEncryptionEnabled broke hacky hive code

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha4, 2.8.1
    • Fix Version/s: 2.9.0, 3.0.0-alpha4, 2.8.2
    • Component/s: None
    • Labels:
      None

      Description

      Though Hive should be fixed not to access DFSClient which is private to HADOOP, removing the throws added by HADOOP-14104 is a quicker solution to unblock hive.

      Hive code

      private boolean isEncryptionEnabled(DFSClient client, Configuration conf) {
        try {
          DFSClient.class.getMethod("isHDFSEncryptionEnabled");
        } catch (NoSuchMethodException e) {
          // the method is available since Hadoop-2.7.1
          // if we run with an older Hadoop, check this ourselves
          return !conf.getTrimmed(DFSConfigKeys.DFS_ENCRYPTION_KEY_PROVIDER_URI, "").isEmpty();
        }
        return client.isHDFSEncryptionEnabled();
      }
      
      1. HADOOP-14333.001.patch
        3 kB
        Yongjun Zhang
      2. HADOOP-14333.002.patch
        3 kB
        Yongjun Zhang
      3. HADOOP-14333.003.patch
        2 kB
        Yongjun Zhang
      4. HDFS-11689.001.patch
        2 kB
        Yongjun Zhang

        Issue Links

          Activity

          Hide
          ngangam Naveen Gangam added a comment -

          Yongjun Zhang Should this be a public API? Is there an alternate means to accomplish the same from hive ? Thanks

          Show
          ngangam Naveen Gangam added a comment - Yongjun Zhang Should this be a public API? Is there an alternate means to accomplish the same from hive ? Thanks
          Hide
          shahrs87 Rushabh S Shah added a comment - - edited

          If I understood the hive code correctly, isEncryptionEnabled method is getting called indirectly by CryptoProcessor.
          The javadoc for the CryptoProcessor says:

          /**
           * This class processes HADOOP commands used for HDFS encryption. It is meant to be run 
           * only by Hive unit & queries tests.
           */
          public class CryptoProcessor implements CommandProcessor {
          

          So I assume its only test code that is breaking. In that case I wouldn't change anything from HADOOP-14104 just to fix hive tests failure.

          removing the throws added by HADOOP-14104 is a quicker solution to unblock hive

          Secondly removing throws and returning false in case of Exception is wrong since there can be any exception while calling getServerDefaults (like socket timeout) and we will return false even if encryption is enabled.
          It will be good to hear from members who are working on hive.
          PS: I was referring to code from hive git repository master branch.

          Show
          shahrs87 Rushabh S Shah added a comment - - edited If I understood the hive code correctly, isEncryptionEnabled method is getting called indirectly by CryptoProcessor. The javadoc for the CryptoProcessor says: /** * This class processes HADOOP commands used for HDFS encryption. It is meant to be run * only by Hive unit & queries tests. */ public class CryptoProcessor implements CommandProcessor { So I assume its only test code that is breaking. In that case I wouldn't change anything from HADOOP-14104 just to fix hive tests failure. removing the throws added by HADOOP-14104 is a quicker solution to unblock hive Secondly removing throws and returning false in case of Exception is wrong since there can be any exception while calling getServerDefaults (like socket timeout) and we will return false even if encryption is enabled. It will be good to hear from members who are working on hive. PS: I was referring to code from hive git repository master branch.
          Hide
          shahrs87 Rushabh S Shah added a comment -

          s there an alternate means to accomplish the same from hive ? Thanks

          The right way to ask whether the path is encrypted or not is to call DistributedFileSystem#getEZForPath(final Path path)
          Refer to https://github.com/apache/hadoop/blob/branch-2.8/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L2245

          Show
          shahrs87 Rushabh S Shah added a comment - s there an alternate means to accomplish the same from hive ? Thanks The right way to ask whether the path is encrypted or not is to call DistributedFileSystem#getEZForPath(final Path path) Refer to https://github.com/apache/hadoop/blob/branch-2.8/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L2245
          Hide
          yzhangal Yongjun Zhang added a comment -

          HI Rushabh S Shah,

          Agree that returning false when exception is thrown is not quite right.

          However, currently DFSClient#isHDFSEncryptionEnabled is only called by DistributedFileSystem#getTrashRoot (which simply issue a warning message and ignore the exception, and treat it as if false is returned) in Hadoop code, which is used only by FSShell.

          Then it's the hive code that tries to use it.

          How do we expect any client code to handle if there is exception thrown from this code? retry? That'd means a big change on hive side I guess.

          Maybe we can temporarily change the behavior here to issue a warning message and return false when exception is throw. Then coordinate a change between hadoop and hive (if hive is currently the only hacky user). Right now hive issue is a compile time issue.

          What do you think?

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - HI Rushabh S Shah , Agree that returning false when exception is thrown is not quite right. However, currently DFSClient#isHDFSEncryptionEnabled is only called by DistributedFileSystem#getTrashRoot (which simply issue a warning message and ignore the exception, and treat it as if false is returned) in Hadoop code, which is used only by FSShell. Then it's the hive code that tries to use it. How do we expect any client code to handle if there is exception thrown from this code? retry? That'd means a big change on hive side I guess. Maybe we can temporarily change the behavior here to issue a warning message and return false when exception is throw. Then coordinate a change between hadoop and hive (if hive is currently the only hacky user). Right now hive issue is a compile time issue. What do you think? Thanks.
          Hide
          aw Allen Wittenauer added a comment -

          Though Hive should be fixed not to access DFSClient which is private to HADOOP

          Yes. Break the rules, live with the consequences. The whole point of having interface stability connotations is to prevent stuff like this. If this was a problem, the Hive project should have raised the issue before using it.

          unblock hive

          unblock from what, exactly?

          Show
          aw Allen Wittenauer added a comment - Though Hive should be fixed not to access DFSClient which is private to HADOOP Yes. Break the rules, live with the consequences. The whole point of having interface stability connotations is to prevent stuff like this. If this was a problem, the Hive project should have raised the issue before using it. unblock hive unblock from what, exactly?
          Hide
          shahrs87 Rushabh S Shah added a comment -

          If you read back Daryn Sharp's comment in HADOOP-14104, we discussed the exact same thing and came to decision that since DFSClient is a private interface, we are allowed to change the signature of isHDFSEncryptionEnabled and are allowed to throw IOException.
          Since getTrashRoot was doing the wrong thing before HADOOP-14104 (Refer to https://github.com/apache/hadoop/blob/branch-2.8/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L2498), we decided to do the same thing. At least that is limited to shell commands not jobs.

          Maybe we can temporarily change the behavior here to issue a warning message and return false when exception is throw.

          On this, I still don't think to return false in case of any exception.
          If we want to temporarily fix hive use case, we can call DFSClient#getKeyProviderUri from DistributedFileSystem#getTrashRoot.
          This means DFSClient#isHDFSEncryptionEnabled is just called from hive code.
          Do something like this in DFSClient#isHDFSEncryptionEnabled

          DFSClient.java
          /**
             * Probe for encryption enabled on this filesystem.
             * @return true if encryption is enabled
             */
            public boolean isHDFSEncryptionEnabled() {
            try {
              URI keyProviderUri = getKeyProviderUri();
            } catch(IOException ioe) {
              return false;
            }
            return keyProviderUri !=null
          

          Probably add a comment on isHDFSEncryptionEnabled saying that this is here only to unblock hive and no-one should use this.

          Show
          shahrs87 Rushabh S Shah added a comment - If you read back Daryn Sharp 's comment in HADOOP-14104 , we discussed the exact same thing and came to decision that since DFSClient is a private interface, we are allowed to change the signature of isHDFSEncryptionEnabled and are allowed to throw IOException. Since getTrashRoot was doing the wrong thing before HADOOP-14104 (Refer to https://github.com/apache/hadoop/blob/branch-2.8/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L2498 ), we decided to do the same thing. At least that is limited to shell commands not jobs. Maybe we can temporarily change the behavior here to issue a warning message and return false when exception is throw. On this, I still don't think to return false in case of any exception. If we want to temporarily fix hive use case, we can call DFSClient#getKeyProviderUri from DistributedFileSystem#getTrashRoot . This means DFSClient#isHDFSEncryptionEnabled is just called from hive code. Do something like this in DFSClient#isHDFSEncryptionEnabled DFSClient.java /** * Probe for encryption enabled on this filesystem. * @ return true if encryption is enabled */ public boolean isHDFSEncryptionEnabled() { try { URI keyProviderUri = getKeyProviderUri(); } catch (IOException ioe) { return false ; } return keyProviderUri != null Probably add a comment on isHDFSEncryptionEnabled saying that this is here only to unblock hive and no-one should use this.
          Hide
          shahrs87 Rushabh S Shah added a comment -

          I didn't read the update from Allen Wittenauer before my last update.

          Yes. Break the rules, live with the consequences. The whole point of having interface stability connotations is to prevent stuff like this. If this was a problem, the Hive project should have raised the issue before using it.

          I agree with him.

          Show
          shahrs87 Rushabh S Shah added a comment - I didn't read the update from Allen Wittenauer before my last update. Yes. Break the rules, live with the consequences. The whole point of having interface stability connotations is to prevent stuff like this. If this was a problem, the Hive project should have raised the issue before using it. I agree with him.
          Hide
          aw Allen Wittenauer added a comment -

          The right way to ask whether the path is encrypted or not is to call DistributedFileSystem#getEZForPath(final Path path)

          (hopefully) quick question: why is this not floated up to FileSystem and FileContext? It seems something that should be part of the standard file system API rather than (the useless) LimitedPrivate.

          Show
          aw Allen Wittenauer added a comment - The right way to ask whether the path is encrypted or not is to call DistributedFileSystem#getEZForPath(final Path path) (hopefully) quick question: why is this not floated up to FileSystem and FileContext? It seems something that should be part of the standard file system API rather than (the useless) LimitedPrivate.
          Hide
          shahrs87 Rushabh S Shah added a comment - - edited

          why is this not floated up to FileSystem and FileContext?

          I was not a part of api design when it was implemented.
          But I think Transparent encryption is only supported by HDFS Filesystem (DistributedFileSystem). Thats why it's not a part of FileSystem api.

          Show
          shahrs87 Rushabh S Shah added a comment - - edited why is this not floated up to FileSystem and FileContext? I was not a part of api design when it was implemented. But I think Transparent encryption is only supported by HDFS Filesystem (DistributedFileSystem). Thats why it's not a part of FileSystem api.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Allen Wittenauer and Rushabh S Shah.

          Agree that Hive project should have raised the issue before using it.

          unblock from what, exactly?

          The hive code that is broken is listed in the description. I was trying to propose that, as a temporary solution, we can let isHDFSEncryptionEnabled not to throw. Then we can coordinate a change between hadoop and hive to fix together.

          Thanks Rushabh for the sample code. I think in the case of SocketTimeOut, we should do some retry. If it's other exception, we return false. Let me post a patch to see if it makes sense.

          Thanks,

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Allen Wittenauer and Rushabh S Shah . Agree that Hive project should have raised the issue before using it. unblock from what, exactly? The hive code that is broken is listed in the description. I was trying to propose that, as a temporary solution, we can let isHDFSEncryptionEnabled not to throw. Then we can coordinate a change between hadoop and hive to fix together. Thanks Rushabh for the sample code. I think in the case of SocketTimeOut, we should do some retry. If it's other exception, we return false. Let me post a patch to see if it makes sense. Thanks,
          Hide
          aw Allen Wittenauer added a comment -

          But I think Transparent encryption is only supported by HDFS Filesystem (DistributedFileSystem). Thats why it's not a part of FileSystem api.

          This argument never ever works out in our favor. Just because Hadoop code doesn't implement feature X for file system Y, doesn't mean file system Y doesn't actually have feature X. Case in point: quotas. The only public APIs are in DistributedFileSystem because people (wrongly) thought that directory-based quotas are unique to Hadoop. As a result, we now have to publish DFS JavaDocs publicly and it still leaves the gap for other file systems that also implement this feature.

          This really needs to get fixed before it's too late.

          The hive code that is broken is listed in the description.

          That doesn't answer my question. This exception was added in unreleased versions of Hadoop. So I'm not sure how this blocks Hive given they can push out a new release before or immediately after these versions of Hadoop get out.

          Show
          aw Allen Wittenauer added a comment - But I think Transparent encryption is only supported by HDFS Filesystem (DistributedFileSystem). Thats why it's not a part of FileSystem api. This argument never ever works out in our favor. Just because Hadoop code doesn't implement feature X for file system Y, doesn't mean file system Y doesn't actually have feature X. Case in point: quotas. The only public APIs are in DistributedFileSystem because people (wrongly) thought that directory-based quotas are unique to Hadoop. As a result, we now have to publish DFS JavaDocs publicly and it still leaves the gap for other file systems that also implement this feature. This really needs to get fixed before it's too late. The hive code that is broken is listed in the description. That doesn't answer my question. This exception was added in unreleased versions of Hadoop. So I'm not sure how this blocks Hive given they can push out a new release before or immediately after these versions of Hadoop get out.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks for the discussion here guys.

          I just posted a patch for reference. I included some retries for socket time out. Wonder if we can consider this as a temporary solution.

          Show
          yzhangal Yongjun Zhang added a comment - Thanks for the discussion here guys. I just posted a patch for reference. I included some retries for socket time out. Wonder if we can consider this as a temporary solution.
          Hide
          andrew.wang Andrew Wang added a comment -

          I took a look at Hive's usage of DFSClient. They're digging into our private bits to get access to the KeyProvider, which they use to get key metadata and also to create keys, delete keys, etc.

          This seems pretty easy to handle in a supportable way by adding a new HdfsAdmin API for getKeyProvider. The issue is that the Hive team never told us they needed access to a KP, so we were unaware of this need.

          We're already swallowing this exception in e.g. getTrashRoot for compatibility reasons, so the question is really whether we swallow it in DFSClient vs. in DFS. This doesn't seem worth making a stink over, when the impact is that existing versions of Hive won't work with 2.8.1 or 3.0.0-alpha3 HDFS clients.

          My proposal:

          • We move swallowing the exception back to DFSClient for now
          • Add whatever APIs Hive requires to HdfsAdmin, get them to use these new APIs
          • Clean things up in HDFS as we want
          Show
          andrew.wang Andrew Wang added a comment - I took a look at Hive's usage of DFSClient. They're digging into our private bits to get access to the KeyProvider, which they use to get key metadata and also to create keys, delete keys, etc. This seems pretty easy to handle in a supportable way by adding a new HdfsAdmin API for getKeyProvider . The issue is that the Hive team never told us they needed access to a KP, so we were unaware of this need. We're already swallowing this exception in e.g. getTrashRoot for compatibility reasons, so the question is really whether we swallow it in DFSClient vs. in DFS. This doesn't seem worth making a stink over, when the impact is that existing versions of Hive won't work with 2.8.1 or 3.0.0-alpha3 HDFS clients. My proposal: We move swallowing the exception back to DFSClient for now Add whatever APIs Hive requires to HdfsAdmin, get them to use these new APIs Clean things up in HDFS as we want
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Andrew Wang.

          Discussed with Andrew and he suggested not to do retries since RPC layer is already doing that. Cleaned up the code and uploaded rev002.

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Andrew Wang . Discussed with Andrew and he suggested not to do retries since RPC layer is already doing that. Cleaned up the code and uploaded rev002.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 13m 50s trunk passed
          +1 compile 0m 33s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          -1 findbugs 1m 24s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings.
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 31s the patch passed
          +1 compile 0m 29s the patch passed
          +1 javac 0m 29s the patch passed
          -0 checkstyle 0m 14s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 1 new + 69 unchanged - 0 fixed = 70 total (was 69)
          +1 mvnsite 0m 31s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 27s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 1m 11s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          24m 35s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-14333
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864379/HADOOP-14333.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 27e0b2c1993d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f356f0f
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 13m 50s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 34s trunk passed +1 mvneclipse 0m 13s trunk passed -1 findbugs 1m 24s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings. +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed -0 checkstyle 0m 14s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 1 new + 69 unchanged - 0 fixed = 70 total (was 69) +1 mvnsite 0m 31s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 27s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 1m 11s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 24m 35s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-14333 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864379/HADOOP-14333.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 27e0b2c1993d 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f356f0f Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12138/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 13m 31s trunk passed
          +1 compile 0m 32s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          -1 findbugs 1m 23s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings.
          +1 javadoc 0m 20s trunk passed
          +1 mvninstall 0m 32s the patch passed
          +1 compile 0m 31s the patch passed
          +1 javac 0m 31s the patch passed
          -0 checkstyle 0m 13s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 1 new + 68 unchanged - 0 fixed = 69 total (was 68)
          +1 mvnsite 0m 33s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 30s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 1m 11s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          24m 24s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-14333
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864385/HADOOP-14333.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f99e5f0f4ff2 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f356f0f
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 13m 31s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 34s trunk passed +1 mvneclipse 0m 14s trunk passed -1 findbugs 1m 23s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings. +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 32s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed -0 checkstyle 0m 13s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 1 new + 68 unchanged - 0 fixed = 69 total (was 68) +1 mvnsite 0m 33s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 30s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 1m 11s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 24m 24s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-14333 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864385/HADOOP-14333.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f99e5f0f4ff2 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f356f0f Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12139/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          andrew.wang Andrew Wang added a comment -

          +1 LGTM, can remove the unused SocketTimeoutException on commit. I'll file Hive and Hadoop JIRAs for tracking adding and using new public APIs.

          Show
          andrew.wang Andrew Wang added a comment - +1 LGTM, can remove the unused SocketTimeoutException on commit. I'll file Hive and Hadoop JIRAs for tracking adding and using new public APIs.
          Hide
          shahrs87 Rushabh S Shah added a comment -

          Can we please hold off committing patch v2 until tomorrow ?
          I work in CST and don't have enough time to review.
          Will post review comments first thing tomorrow morning.
          Thanks !

          Show
          shahrs87 Rushabh S Shah added a comment - Can we please hold off committing patch v2 until tomorrow ? I work in CST and don't have enough time to review. Will post review comments first thing tomorrow morning. Thanks !
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks much Andrew Wang and Rushabh S Shah.

          Uploaded v3 to drop the unused import.

          Hi Rushabh, really appreciate that you will review first thing tomorrow morning, thanks a lot!

          Show
          yzhangal Yongjun Zhang added a comment - Thanks much Andrew Wang and Rushabh S Shah . Uploaded v3 to drop the unused import. Hi Rushabh, really appreciate that you will review first thing tomorrow morning, thanks a lot!
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 15m 46s trunk passed
          +1 compile 0m 37s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 39s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          -1 findbugs 1m 39s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings.
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 43s the patch passed
          +1 compile 0m 36s the patch passed
          +1 javac 0m 36s the patch passed
          +1 checkstyle 0m 16s the patch passed
          +1 mvnsite 0m 35s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 42s the patch passed
          +1 javadoc 0m 21s the patch passed
          +1 unit 1m 26s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          28m 21s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HADOOP-14333
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864415/HADOOP-14333.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b90b8e2835ac 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 667966c
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12142/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12142/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12142/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 15m 46s trunk passed +1 compile 0m 37s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 15s trunk passed -1 findbugs 1m 39s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings. +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 43s the patch passed +1 compile 0m 36s the patch passed +1 javac 0m 36s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 35s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 42s the patch passed +1 javadoc 0m 21s the patch passed +1 unit 1m 26s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 28m 21s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HADOOP-14333 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864415/HADOOP-14333.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b90b8e2835ac 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 667966c Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12142/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12142/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12142/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Filesystems are a special case, not just because HDFS adds stuff, but because there are fundamental differences between different filesystems (case sensitivity, full posix seek+write, atomic dir rename, o(1) File rename, consistent world view....). You can't declare that something supports this just through an interface, as (a) it varies at runtime and (b) FSDataOutputStream shows how base classes declare functionality which subclasses end up rejecting by dynamically throwing exceptions.

          without getting into the versioning row, note HADOOP-9565 has narrowed down to some method on FileSystem to probe for features, something like

          boolean hasFeature(Path, String)
          

          Implementations can switch on the feature string, return true iff the feature is present and enabled. There's been discussion of a similar problem related to output stream features, we could do some similar interface here.

          Show
          stevel@apache.org Steve Loughran added a comment - Filesystems are a special case, not just because HDFS adds stuff, but because there are fundamental differences between different filesystems (case sensitivity, full posix seek+write, atomic dir rename, o(1) File rename, consistent world view....). You can't declare that something supports this just through an interface, as (a) it varies at runtime and (b) FSDataOutputStream shows how base classes declare functionality which subclasses end up rejecting by dynamically throwing exceptions. without getting into the versioning row, note HADOOP-9565 has narrowed down to some method on FileSystem to probe for features, something like boolean hasFeature(Path, String ) Implementations can switch on the feature string, return true iff the feature is present and enabled. There's been discussion of a similar problem related to output stream features, we could do some similar interface here.
          Hide
          daryn Daryn Sharp added a comment -

          Quiet bothered by hive using private hdfs apis with the added touch of reflection. It's one thing to hack your own project because when you break it you fix it, but completely different to hack another project... There should be a followup jira to remove the method to ensure hive makes the change.

          +1 After adding @Deprecated to the method and moving this jira to the hdfs project.

          Show
          daryn Daryn Sharp added a comment - Quiet bothered by hive using private hdfs apis with the added touch of reflection. It's one thing to hack your own project because when you break it you fix it, but completely different to hack another project... There should be a followup jira to remove the method to ensure hive makes the change. +1 After adding @Deprecated to the method and moving this jira to the hdfs project.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Steve Loughran and Daryn Sharp.

          I moved the jira to hdfs project. About "adding @Deprecated to the method", my understanding is that this method is not deprecated. It's hive's mistake to access DFSClient which is private to hadoop. Do we want to make the method deprecate?

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Steve Loughran and Daryn Sharp . I moved the jira to hdfs project. About "adding @Deprecated to the method", my understanding is that this method is not deprecated. It's hive's mistake to access DFSClient which is private to hadoop. Do we want to make the method deprecate?
          Hide
          andrew.wang Andrew Wang added a comment -

          Adding Deprecated will make Hive's build print a deprecation warning, so it's an additional signal that they're doing something incorrect, and also a reminder for us to change this later. Explanatory comment would be good.

          Show
          andrew.wang Andrew Wang added a comment - Adding Deprecated will make Hive's build print a deprecation warning, so it's an additional signal that they're doing something incorrect, and also a reminder for us to change this later. Explanatory comment would be good.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Thanks Andrew Wang. I wish the private annotation

          @InterfaceAudience.Private
          public class DFSClient implements java.io.Closeable, RemotePeerFactory,
              DataEncryptionKeyFactory {
          

          could trigger a warning on hive build, but it seems not right?

          Will add deprecate, as a workaround for lack of build warning on the private annotation.

          Show
          yzhangal Yongjun Zhang added a comment - Thanks Andrew Wang . I wish the private annotation @InterfaceAudience.Private public class DFSClient implements java.io.Closeable, RemotePeerFactory, DataEncryptionKeyFactory { could trigger a warning on hive build, but it seems not right? Will add deprecate, as a workaround for lack of build warning on the private annotation.
          Hide
          yzhangal Yongjun Zhang added a comment -

          Andrew explained that @InterfaceAudience.Private is not java build in thus we don't have compiler warning.

          Uploaded new patch HDFS-11689.001.patch with Deprecated added.

          Will commit once the jenkins test is finished.

          Thanks.

          Show
          yzhangal Yongjun Zhang added a comment - Andrew explained that @InterfaceAudience.Private is not java build in thus we don't have compiler warning. Uploaded new patch HDFS-11689 .001.patch with Deprecated added. Will commit once the jenkins test is finished. Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 28s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 61m 1s trunk passed
          +1 compile 0m 36s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 39s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          -1 findbugs 1m 33s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings.
          +1 javadoc 0m 28s trunk passed
          +1 mvninstall 0m 48s the patch passed
          +1 compile 0m 44s the patch passed
          -1 javac 0m 44s hadoop-hdfs-project_hadoop-hdfs-client generated 1 new + 14 unchanged - 0 fixed = 15 total (was 14)
          +1 checkstyle 0m 20s the patch passed
          +1 mvnsite 0m 48s the patch passed
          +1 mvneclipse 0m 16s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 4s the patch passed
          +1 javadoc 0m 28s the patch passed
          +1 unit 1m 45s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 24s The patch does not generate ASF License warnings.
          74m 46s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Issue HDFS-11689
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864536/HDFS-11689.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2764aff33d21 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b080338
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19171/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
          javac https://builds.apache.org/job/PreCommit-HDFS-Build/19171/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-client.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19171/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19171/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 28s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 61m 1s trunk passed +1 compile 0m 36s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 16s trunk passed -1 findbugs 1m 33s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings. +1 javadoc 0m 28s trunk passed +1 mvninstall 0m 48s the patch passed +1 compile 0m 44s the patch passed -1 javac 0m 44s hadoop-hdfs-project_hadoop-hdfs-client generated 1 new + 14 unchanged - 0 fixed = 15 total (was 14) +1 checkstyle 0m 20s the patch passed +1 mvnsite 0m 48s the patch passed +1 mvneclipse 0m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 4s the patch passed +1 javadoc 0m 28s the patch passed +1 unit 1m 45s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 24s The patch does not generate ASF License warnings. 74m 46s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Issue HDFS-11689 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864536/HDFS-11689.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2764aff33d21 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b080338 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/19171/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html javac https://builds.apache.org/job/PreCommit-HDFS-Build/19171/artifact/patchprocess/diff-compile-javac-hadoop-hdfs-project_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/19171/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/19171/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          shahrs87 Rushabh S Shah added a comment - - edited

          +1 (non-binding) for the latest patch.
          Yongjun Zhang: thanks !

          Show
          shahrs87 Rushabh S Shah added a comment - - edited +1 (non-binding) for the latest patch. Yongjun Zhang : thanks !
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11621 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11621/)
          HDFS-11689. New exception thrown by DFSClient%isHDFSEncryptionEnabled (yzhang: rev 5078df7be317e635615c05c5da3285798993ff1f)

          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11621 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11621/ ) HDFS-11689 . New exception thrown by DFSClient%isHDFSEncryptionEnabled (yzhang: rev 5078df7be317e635615c05c5da3285798993ff1f) (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          Hide
          yzhangal Yongjun Zhang added a comment - - edited

          Thanks a lot Rushabh S Shah, Daryn Sharp, Allen Wittenauer, Steve Loughran and Andrew Wang!

          I committed to trunk, branch-2, branch-2.8, branch-2.8.1

          Show
          yzhangal Yongjun Zhang added a comment - - edited Thanks a lot Rushabh S Shah , Daryn Sharp , Allen Wittenauer , Steve Loughran and Andrew Wang ! I committed to trunk, branch-2, branch-2.8, branch-2.8.1
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - 2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.

            People

            • Assignee:
              yzhangal Yongjun Zhang
              Reporter:
              yzhangal Yongjun Zhang
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development