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

ReconfigurationProtocol not covered by HDFSPolicyProvider.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.9.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha1
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The HDFSPolicyProvider class contains an entry for defining the security policy of each HDFS RPC protocol interface. ReconfigurationProtocol is not listed currently. This may indicate that reconfiguration functionality is not working correctly in secured clusters.

      1. HDFS-10437.01.patch
        6 kB
        Arpit Agarwal
      2. HDFS-10437.02.patch
        6 kB
        Arpit Agarwal
      3. HDFS-10437.03.patch
        6 kB
        Arpit Agarwal

        Issue Links

          Activity

          Hide
          cnauroth Chris Nauroth added a comment -

          This came out of investigation on HDFS-10424, which reported a similar problem for DatanodeLifelineProtocol. The test I wrote for the patch attached there also indicated a possible problem for ReconfigurationProtocol.

          Xiaobing Zhou, would you please check? Cc Arpit Agarwal.

          Show
          cnauroth Chris Nauroth added a comment - This came out of investigation on HDFS-10424 , which reported a similar problem for DatanodeLifelineProtocol . The test I wrote for the patch attached there also indicated a possible problem for ReconfigurationProtocol . Xiaobing Zhou , would you please check? Cc Arpit Agarwal .
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Nice catch Chris Nauroth, thank you for the heads up!

          Show
          arpitagarwal Arpit Agarwal added a comment - Nice catch Chris Nauroth , thank you for the heads up!
          Hide
          arpitagarwal Arpit Agarwal added a comment -
          Show
          arpitagarwal Arpit Agarwal added a comment - cc Xiaobing Zhou
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Thanks Chris Nauroth for the catch. We should add ReconfigurationProtocol to HDFSPolicyProvider.

          Show
          xiaobingo Xiaobing Zhou added a comment - Thanks Chris Nauroth for the catch. We should add ReconfigurationProtocol to HDFSPolicyProvider.
          Hide
          cnauroth Chris Nauroth added a comment -

          Xiaobing Zhou, thanks for confirmation. When we do that, let's also update the new TestHDFSPolicyProvider suite that I'm adding in HDFS-10424 to remove the special-case check for ReconfigurationProtocol.

          Show
          cnauroth Chris Nauroth added a comment - Xiaobing Zhou , thanks for confirmation. When we do that, let's also update the new TestHDFSPolicyProvider suite that I'm adding in HDFS-10424 to remove the special-case check for ReconfigurationProtocol .
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Ok, I will jump to this after HDFS-10424 is committed. Thank you.

          Show
          xiaobingo Xiaobing Zhou added a comment - Ok, I will jump to this after HDFS-10424 is committed. Thank you.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Attaching a simple fix for this issue. Updated the test to remove the exception for ReconfigurationProtocol.

          Show
          arpitagarwal Arpit Agarwal added a comment - Attaching a simple fix for this issue. Updated the test to remove the exception for ReconfigurationProtocol.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 36s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 35s Maven dependency ordering for branch
          +1 mvninstall 8m 2s trunk passed
          +1 compile 7m 56s trunk passed
          +1 checkstyle 1m 31s trunk passed
          +1 mvnsite 2m 8s trunk passed
          +1 mvneclipse 0m 29s trunk passed
          +1 findbugs 3m 34s trunk passed
          +1 javadoc 2m 6s trunk passed
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 46s the patch passed
          +1 compile 8m 4s the patch passed
          +1 javac 8m 4s the patch passed
          -1 checkstyle 1m 37s root: The patch generated 4 new + 100 unchanged - 0 fixed = 104 total (was 100)
          +1 mvnsite 2m 4s the patch passed
          +1 mvneclipse 0m 30s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 46s the patch passed
          +1 javadoc 1m 53s the patch passed
          -1 unit 22m 10s hadoop-common in the patch failed.
          -1 unit 72m 29s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          142m 51s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12811499/HDFS-10437.01.patch
          JIRA Issue HDFS-10437
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 73a0c34f47d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0761379
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15821/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15821/console
          Powered by Apache Yetus 0.3.0 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 36s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 35s Maven dependency ordering for branch +1 mvninstall 8m 2s trunk passed +1 compile 7m 56s trunk passed +1 checkstyle 1m 31s trunk passed +1 mvnsite 2m 8s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 3m 34s trunk passed +1 javadoc 2m 6s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 46s the patch passed +1 compile 8m 4s the patch passed +1 javac 8m 4s the patch passed -1 checkstyle 1m 37s root: The patch generated 4 new + 100 unchanged - 0 fixed = 104 total (was 100) +1 mvnsite 2m 4s the patch passed +1 mvneclipse 0m 30s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 46s the patch passed +1 javadoc 1m 53s the patch passed -1 unit 22m 10s hadoop-common in the patch failed. -1 unit 72m 29s hadoop-hdfs in the patch failed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 142m 51s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12811499/HDFS-10437.01.patch JIRA Issue HDFS-10437 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 73a0c34f47d2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0761379 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15821/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15821/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15821/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Fix checkstyle issues.

          Show
          arpitagarwal Arpit Agarwal added a comment - Fix checkstyle issues.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Thank you Arpit Agarwal for helping to post the patch. It didn't come up from my todo list.
          The patch looks good to me.

          Show
          xiaobingo Xiaobing Zhou added a comment - Thank you Arpit Agarwal for helping to post the patch. It didn't come up from my todo list. The patch looks good to me.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for patch 02, pending pre-commit.

          -    policyProviderProtocols = new ArrayList<>(services.length);
          +    policyProviderProtocols = new HashSet<>(services.length);
          

          Just a very minor nit: Passing a length to a collection constructor is usually meant to avoid a memory realloc when the expected length is known. With the switch from ArrayList to HashSet, that doesn't really work anymore though, because the argument is now interpreted as the internal hash table's capacity, with a realloc occurring after exceeding (capacity * load factor), which defaults to 0.75. Things like Guava's Maps#newHashMapWithExpectedSize internally do some math on the argument to scale it up and try to stay ahead of the load factor to prevent a realloc.

          It doesn't really matter much here, where it's just test code and the data set is tiny, so I'm still +1 for the patch. It's just a common pitfall of the HashMap/HashSet API.

          Thanks for the patch!

          Show
          cnauroth Chris Nauroth added a comment - +1 for patch 02, pending pre-commit. - policyProviderProtocols = new ArrayList<>(services.length); + policyProviderProtocols = new HashSet<>(services.length); Just a very minor nit: Passing a length to a collection constructor is usually meant to avoid a memory realloc when the expected length is known. With the switch from ArrayList to HashSet , that doesn't really work anymore though, because the argument is now interpreted as the internal hash table's capacity, with a realloc occurring after exceeding (capacity * load factor), which defaults to 0.75. Things like Guava's Maps#newHashMapWithExpectedSize internally do some math on the argument to scale it up and try to stay ahead of the load factor to prevent a realloc. It doesn't really matter much here, where it's just test code and the data set is tiny, so I'm still +1 for the patch. It's just a common pitfall of the HashMap / HashSet API. Thanks for the patch!
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 24s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 42s Maven dependency ordering for branch
          +1 mvninstall 7m 43s trunk passed
          +1 compile 8m 23s trunk passed
          +1 checkstyle 1m 36s trunk passed
          +1 mvnsite 2m 19s trunk passed
          +1 mvneclipse 0m 35s trunk passed
          +1 findbugs 3m 30s trunk passed
          +1 javadoc 2m 8s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 52s the patch passed
          +1 compile 7m 1s the patch passed
          +1 javac 7m 1s the patch passed
          -1 checkstyle 1m 20s root: The patch generated 2 new + 100 unchanged - 0 fixed = 102 total (was 100)
          +1 mvnsite 1m 42s the patch passed
          +1 mvneclipse 0m 26s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 15s the patch passed
          +1 javadoc 1m 42s the patch passed
          +1 unit 7m 45s hadoop-common in the patch passed.
          -1 unit 75m 45s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 28s The patch does not generate ASF License warnings.
          129m 52s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.TestEditLog



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12811512/HDFS-10437.02.patch
          JIRA Issue HDFS-10437
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b85b6eb7fcec 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0761379
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15824/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15824/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15824/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15824/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15824/console
          Powered by Apache Yetus 0.3.0 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 24s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 42s Maven dependency ordering for branch +1 mvninstall 7m 43s trunk passed +1 compile 8m 23s trunk passed +1 checkstyle 1m 36s trunk passed +1 mvnsite 2m 19s trunk passed +1 mvneclipse 0m 35s trunk passed +1 findbugs 3m 30s trunk passed +1 javadoc 2m 8s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 52s the patch passed +1 compile 7m 1s the patch passed +1 javac 7m 1s the patch passed -1 checkstyle 1m 20s root: The patch generated 2 new + 100 unchanged - 0 fixed = 102 total (was 100) +1 mvnsite 1m 42s the patch passed +1 mvneclipse 0m 26s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 15s the patch passed +1 javadoc 1m 42s the patch passed +1 unit 7m 45s hadoop-common in the patch passed. -1 unit 75m 45s hadoop-hdfs in the patch failed. +1 asflicense 0m 28s The patch does not generate ASF License warnings. 129m 52s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.TestEditLog Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12811512/HDFS-10437.02.patch JIRA Issue HDFS-10437 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b85b6eb7fcec 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0761379 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15824/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15824/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15824/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15824/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15824/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thanks for the reviews Xiaobing Zhou, Chris Nauroth.

          Yes the initial length makes little sense for a set construction, it was inadvertently left in the test case. Thanks for catching it!

          Attaching a v3 patch to fix that. The remaining checkstyle issues are due to following the style of the original sources. I will commit this shortly.

          Show
          arpitagarwal Arpit Agarwal added a comment - Thanks for the reviews Xiaobing Zhou , Chris Nauroth . Yes the initial length makes little sense for a set construction, it was inadvertently left in the test case. Thanks for catching it! Attaching a v3 patch to fix that. The remaining checkstyle issues are due to following the style of the original sources. I will commit this shortly.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Pushed this to trunk and branch-2. Thank you for the reviews Chris and Xiaobing and thanks Chris for reporting it.

          Show
          arpitagarwal Arpit Agarwal added a comment - Pushed this to trunk and branch-2. Thank you for the reviews Chris and Xiaobing and thanks Chris for reporting it.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #9983 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9983/)
          HDFS-10437. ReconfigurationProtocol not covered by HDFSPolicyProvider. (arp: rev 0319d73c3b14c09da0f28371dc857b2cc5f6ec7a)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HDFSPolicyProvider.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHDFSPolicyProvider.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #9983 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9983/ ) HDFS-10437 . ReconfigurationProtocol not covered by HDFSPolicyProvider. (arp: rev 0319d73c3b14c09da0f28371dc857b2cc5f6ec7a) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/HDFSPolicyProvider.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestHDFSPolicyProvider.java

            People

            • Assignee:
              arpitagarwal Arpit Agarwal
              Reporter:
              cnauroth Chris Nauroth
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development