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

OIV's ReverseXML processor should support ACLs

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: tools
    • Labels:
      None
    • Target Version/s:

      Description

      OIV's ReverseXML processor should support ACLs. Currently ACLs show up in the fsimage.xml file, but we don't reconstruct them with ReverseXML.

      1. HDFS-10505-001.patch
        8 kB
        Surendra Singh Lilhore
      2. HDFS-10505-002.patch
        8 kB
        Surendra Singh Lilhore

        Issue Links

          Activity

          Hide
          cmccabe Colin P. McCabe added a comment -

          From hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageReconstructor.java:

            private INodeSection.AclFeatureProto.Builder aclXmlToProto(Node acl)
                throws IOException {
              // TODO: support ACLs
              throw new IOException("ACLs are not supported yet.");
            }
          
          Show
          cmccabe Colin P. McCabe added a comment - From hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageReconstructor.java : private INodeSection.AclFeatureProto.Builder aclXmlToProto(Node acl) throws IOException { // TODO: support ACLs throw new IOException( "ACLs are not supported yet." ); }
          Hide
          surendrasingh Surendra Singh Lilhore added a comment -

          Hi Colin P. McCabe,

          I have created patch for this jira but I am not able to attach patch.
          Please can you assign this jira to me ?

          Thanks

          Show
          surendrasingh Surendra Singh Lilhore added a comment - Hi Colin P. McCabe , I have created patch for this jira but I am not able to attach patch. Please can you assign this jira to me ? Thanks
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Assigned. Thanks Surendra Singh Lilhore.

          Show
          ajisakaa Akira Ajisaka added a comment - Assigned. Thanks Surendra Singh Lilhore .
          Hide
          surendrasingh Surendra Singh Lilhore added a comment -

          Thanks Akira Ajisaka..

          Attached patch, please review...

          Show
          surendrasingh Surendra Singh Lilhore added a comment - Thanks Akira Ajisaka .. Attached patch, please review...
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s 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.
          +1 mvninstall 6m 39s trunk passed
          +1 compile 0m 44s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 39s trunk passed
          +1 javadoc 0m 55s trunk passed
          +1 mvninstall 0m 45s the patch passed
          +1 compile 0m 42s the patch passed
          +1 javac 0m 42s the patch passed
          +1 checkstyle 0m 22s the patch passed
          +1 mvnsite 0m 48s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          -1 whitespace 0m 0s The patch has 20 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 1m 44s the patch passed
          +1 javadoc 0m 52s the patch passed
          +1 unit 56m 11s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          74m 44s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12809877/HDFS-10505-001.patch
          JIRA Issue HDFS-10505
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9c4239472cbe 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 / 28b66ae
          Default Java 1.8.0_91
          findbugs v3.0.0
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/15753/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15753/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15753/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 11s 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. +1 mvninstall 6m 39s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 39s trunk passed +1 javadoc 0m 55s trunk passed +1 mvninstall 0m 45s the patch passed +1 compile 0m 42s the patch passed +1 javac 0m 42s the patch passed +1 checkstyle 0m 22s the patch passed +1 mvnsite 0m 48s the patch passed +1 mvneclipse 0m 9s the patch passed -1 whitespace 0m 0s The patch has 20 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 1m 44s the patch passed +1 javadoc 0m 52s the patch passed +1 unit 56m 11s hadoop-hdfs in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 74m 44s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12809877/HDFS-10505-001.patch JIRA Issue HDFS-10505 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9c4239472cbe 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 / 28b66ae Default Java 1.8.0_91 findbugs v3.0.0 whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/15753/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15753/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15753/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          surendrasingh Surendra Singh Lilhore added a comment -

          Whitespace warnings are unrelated to this patch.

          Show
          surendrasingh Surendra Singh Lilhore added a comment - Whitespace warnings are unrelated to this patch.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Thanks for this, Surendra Singh Lilhore. It's good to see progress on supporting ACLs here!

          I am confused by the changes for setting latestStringId to 1, or special-casing null in registerStringId. If we are going to do "magical" things with special indexes in the string table, we need to document it somewhere. Actually, though, I would prefer to simply handle it without the magic. We know that a null entry for an ACL name simply means that the name was an empty string. You can see that in AclEntry.java:

                String name = split[index];
                if (!name.isEmpty()) {
                  builder.setName(name);
                }
          

          In ReverseXML, we should simply translate these null ACL names back into empty strings, and then the existing logic for handling the string table would work, with no magic. We also need a test case which has null ACL names, so that this code is being exercised.

          Show
          cmccabe Colin P. McCabe added a comment - Thanks for this, Surendra Singh Lilhore . It's good to see progress on supporting ACLs here! I am confused by the changes for setting latestStringId to 1, or special-casing null in registerStringId . If we are going to do "magical" things with special indexes in the string table, we need to document it somewhere. Actually, though, I would prefer to simply handle it without the magic. We know that a null entry for an ACL name simply means that the name was an empty string. You can see that in AclEntry.java : String name = split[index]; if (!name.isEmpty()) { builder.setName(name); } In ReverseXML, we should simply translate these null ACL names back into empty strings, and then the existing logic for handling the string table would work, with no magic. We also need a test case which has null ACL names, so that this code is being exercised.
          Hide
          surendrasingh Surendra Singh Lilhore added a comment -

          Thanks Colin P. McCabe for review.

          I am confused by the changes for setting latestStringId to 1, or special-casing null in registerStringId.

          This is changed because I saw same logic in FSImageFormatProtobuf.java .

                int getId(E value) {
                  if (value == null) {
                    return 0;
                  }
                  Integer v = map.get(value);
                  if (v == null) {
                    int nv = map.size() + 1;
                    map.put(value, nv);
                    return nv;
                  }
                  return v;
                }
          

          We also need a test case which has null ACL names, so that this code is being exercised.

          Test code already have ACL entry with null acl name. aclEntry(ACCESS, USER, ALL)

          I changed the patch based on your suggestion.

          Please review..

          Show
          surendrasingh Surendra Singh Lilhore added a comment - Thanks Colin P. McCabe for review. I am confused by the changes for setting latestStringId to 1, or special-casing null in registerStringId. This is changed because I saw same logic in FSImageFormatProtobuf.java . int getId(E value) { if (value == null ) { return 0; } Integer v = map.get(value); if (v == null ) { int nv = map.size() + 1; map.put(value, nv); return nv; } return v; } We also need a test case which has null ACL names, so that this code is being exercised. Test code already have ACL entry with null acl name. aclEntry(ACCESS, USER, ALL) I changed the patch based on your suggestion. Please review..
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 26s 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.
          +1 mvninstall 7m 0s trunk passed
          +1 compile 0m 50s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 0m 57s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 1m 45s trunk passed
          +1 javadoc 0m 56s trunk passed
          +1 mvninstall 0m 47s the patch passed
          +1 compile 0m 44s the patch passed
          +1 javac 0m 44s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 52s the patch passed
          +1 javadoc 0m 52s the patch passed
          -1 unit 80m 20s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          100m 23s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestRollingUpgrade
            hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics
            hadoop.hdfs.server.datanode.TestDataNodeMXBean
            hadoop.hdfs.server.namenode.TestNamenodeCapacityReport



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12810912/HDFS-10505-002.patch
          JIRA Issue HDFS-10505
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 515a0932a344 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 / 6f0aa75
          Default Java 1.8.0_91
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15785/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15785/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15785/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15785/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 26s 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. +1 mvninstall 7m 0s trunk passed +1 compile 0m 50s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 57s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 45s trunk passed +1 javadoc 0m 56s trunk passed +1 mvninstall 0m 47s the patch passed +1 compile 0m 44s the patch passed +1 javac 0m 44s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 52s the patch passed +1 javadoc 0m 52s the patch passed -1 unit 80m 20s hadoop-hdfs in the patch failed. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 100m 23s Reason Tests Failed junit tests hadoop.hdfs.TestRollingUpgrade   hadoop.hdfs.server.datanode.TestDataNodeErasureCodingMetrics   hadoop.hdfs.server.datanode.TestDataNodeMXBean   hadoop.hdfs.server.namenode.TestNamenodeCapacityReport Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12810912/HDFS-10505-002.patch JIRA Issue HDFS-10505 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 515a0932a344 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 / 6f0aa75 Default Java 1.8.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/15785/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15785/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15785/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15785/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment -
          Show
          cmccabe Colin P. McCabe added a comment - +1. Thanks, Surendra Singh Lilhore
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #9966 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9966/)
          HDFS-10505. OIV's ReverseXML processor should support ACLs (Surendra (cmccabe: rev 2449db507d84b1c4fac70a800fb2ad8905cf3db7)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageReconstructor.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #9966 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9966/ ) HDFS-10505 . OIV's ReverseXML processor should support ACLs (Surendra (cmccabe: rev 2449db507d84b1c4fac70a800fb2ad8905cf3db7) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageReconstructor.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java

            People

            • Assignee:
              surendrasingh Surendra Singh Lilhore
              Reporter:
              cmccabe Colin P. McCabe
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development