Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-4651

Offline Image Viewer backport to branch-1

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.1
    • Fix Version/s: 1.2.0
    • Component/s: tools
    • Labels:
      None
    • Target Version/s:

      Description

      This issue tracks backporting the Offline Image Viewer tool to branch-1.

      1. HDFS-4651-branch-1.3.patch
        148 kB
        Chris Nauroth
      2. HDFS-4651-branch-1.2.patch
        148 kB
        Chris Nauroth
      3. HDFS-4651-branch-1.1.patch
        145 kB
        Chris Nauroth
      4. fsimageV19
        3 kB
        Chris Nauroth
      5. fsimageV18
        2 kB
        Chris Nauroth

        Issue Links

          Activity

          Matt Foley made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
          Hide
          Roman Shaposhnik added a comment -

          Suresh Srinivasan in general bumping up the version of Guava could be risky. That said, I believe that most of the upstream projects should be fine by now. Here's a complete list of different versions of guava in projects curated by Bigtop:

          $ cd /mnt/jenkins/jobs/Bigtop-trunk-Repository/configurations/axis-label
          $ for i in `find . -name \*.rpm` ; do rpm -ql -p $i | grep  guava.*jar ; done | sed -e 's#^.*/##g' | sort | uniq -c
               12 guava-10.0.1.jar
               96 guava-11.0.2.jar
               12 guava-13.0.jar
               12 guava-r05.jar
               36 guava-r09.jar
          
          Show
          Roman Shaposhnik added a comment - Suresh Srinivasan in general bumping up the version of Guava could be risky. That said, I believe that most of the upstream projects should be fine by now. Here's a complete list of different versions of guava in projects curated by Bigtop: $ cd /mnt/jenkins/jobs/Bigtop-trunk-Repository/configurations/axis-label $ for i in `find . -name \*.rpm` ; do rpm -ql -p $i | grep guava.*jar ; done | sed -e 's#^.*/##g' | sort | uniq -c 12 guava-10.0.1.jar 96 guava-11.0.2.jar 12 guava-13.0.jar 12 guava-r05.jar 36 guava-r09.jar
          Suresh Srinivas made changes -
          Target Version/s 1.2.1 [ 12324148 ] 1.2.0 [ 12321657 ]
          Suresh Srinivas made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 1.2.0 [ 12321657 ]
          Resolution Fixed [ 1 ]
          Hide
          Suresh Srinivas added a comment -

          I ran unit tests to ensure all the newly added unit tests pass. Also built the document using forrest and checked the documentation changes covers newly added image viewer functionality. I have committed this to branch-1 and branch-1.2.

          Thank you Chris for backporting this useful feature to branch-1.

          Show
          Suresh Srinivas added a comment - I ran unit tests to ensure all the newly added unit tests pass. Also built the document using forrest and checked the documentation changes covers newly added image viewer functionality. I have committed this to branch-1 and branch-1.2. Thank you Chris for backporting this useful feature to branch-1.
          Hide
          Suresh Srinivas added a comment -

          Thanks Chris for making this change. +1 for the patch.

          Show
          Suresh Srinivas added a comment - Thanks Chris for making this change. +1 for the patch.
          Chris Nauroth made changes -
          Attachment HDFS-4651-branch-1.3.patch [ 12576258 ]
          Hide
          Chris Nauroth added a comment -

          Thanks, Suresh. I'm uploading version 3 of the patch to inline the charset constant and remove the Guava dependency.

          Just as a reminder, there are also the 2 testing fsimage files attached to this issue that need to be committed into src/test/org/apache/hadoop/hdfs/tools/offlineImageViewer in addition to this patch file.

          Show
          Chris Nauroth added a comment - Thanks, Suresh. I'm uploading version 3 of the patch to inline the charset constant and remove the Guava dependency. Just as a reminder, there are also the 2 testing fsimage files attached to this issue that need to be committed into src/test/org/apache/hadoop/hdfs/tools/offlineImageViewer in addition to this patch file.
          Hide
          Suresh Srinivas added a comment -

          I think that is good idea. With guava dependency removed, I can commit this patch quickly.

          Show
          Suresh Srinivas added a comment - I think that is good idea. With guava dependency removed, I can commit this patch quickly.
          Hide
          Chris Nauroth added a comment -

          It looks like oiv only used Guava for one thing: a convenient constant definition of a charset in TextWriterImageVisitor:

          public TextWriterImageVisitor(String filename, boolean printToScreen)
                 throws IOException {
            super();
            this.printToScreen = printToScreen;
            fw = new OutputStreamWriter(new FileOutputStream(filename), Charsets.UTF_8);
            okToWrite = true;
          }
          

          If the Guava dependency is problematic, then it would be easy to inline this constant definition, which is just java.nio.charset.Charset.forName("UTF-8").

          https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/base/Charsets.java#56

          Show
          Chris Nauroth added a comment - It looks like oiv only used Guava for one thing: a convenient constant definition of a charset in TextWriterImageVisitor : public TextWriterImageVisitor( String filename, boolean printToScreen) throws IOException { super (); this .printToScreen = printToScreen; fw = new OutputStreamWriter( new FileOutputStream(filename), Charsets.UTF_8); okToWrite = true ; } If the Guava dependency is problematic, then it would be easy to inline this constant definition, which is just java.nio.charset.Charset.forName("UTF-8") . https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/base/Charsets.java#56
          Hide
          Suresh Srinivas added a comment -

          The patch looks good. This can go into 1.2.0, if Matt agrees to it. One thing I want to highlight is, this adds dependency on guava version 11.0.2. This is the same version of guava used by trunk. Roman Shaposhnik Do you see any issue for other projects due to this?

          Show
          Suresh Srinivas added a comment - The patch looks good. This can go into 1.2.0, if Matt agrees to it. One thing I want to highlight is, this adds dependency on guava version 11.0.2. This is the same version of guava used by trunk. Roman Shaposhnik Do you see any issue for other projects due to this?
          Chris Nauroth made changes -
          Attachment HDFS-4651-branch-1.2.patch [ 12576195 ]
          Hide
          Chris Nauroth added a comment -

          I just realized that my first patch didn't include TestLayoutVersion, which is a test suite to go with LayoutVersion. Here is a second version of the patch that adds this test suite. I confirmed that it passes.

          Show
          Chris Nauroth added a comment - I just realized that my first patch didn't include TestLayoutVersion , which is a test suite to go with LayoutVersion . Here is a second version of the patch that adds this test suite. I confirmed that it passes.
          Chris Nauroth made changes -
          Link This issue relates to HADOOP-5467 [ HADOOP-5467 ]
          Chris Nauroth made changes -
          Attachment HDFS-4651-branch-1.1.patch [ 12576193 ]
          Hide
          Chris Nauroth added a comment -

          I'm uploading the patch. Please also note that the tests depend on 2 external fsimage files. These are not present in the patch file, because the patch can't carry binary files. Instead, I am attaching the files to this issue. When we commit this patch, we also need to commit these files at src/test/org/apache/hadoop/hdfs/tools/offlineImageViewer.

          For the backport, I started with the current trunk version of the code. I took the docs from the current branch-0.22, because they were in the old XML format, but they were more complete than the original docs from the HADOOP-5467 patch.

          Then, there were a few minor adjustments required to make the code work with branch-1:

          1. I've included LayoutVersion, because ImageLoaderCurrent needs it to determine what to parse.
          2. FSImageSerialization class is not available in branch-1. The original implementation of this code used the read methods in FSImage instead.
          3. ImageLoaderCurrent needed some new methods in CompressionCodecFactory, so I included those.
          4. TestOfflineImageViewer depended on a special testing flag not present in branch-1 to force generation of delegation tokens. I've included a minimal port of this flag so that we can maintain the same level of testing coverage.
          5. The trunk version of the code uses FSEditLogLoader#PositionTrackingInputStream so that oiv can report the byte offset where a failure occurred in the event of a corrupted fsimage. This class is not present in branch-1, and porting it would have swept in a lot of unrelated code changes. Instead, I have provided a minimal version as a nested class of OfflineImageViewer.

          All new tests pass, and I have manually tested on fsimage files from a deployed branch-1-based cluster.

          Show
          Chris Nauroth added a comment - I'm uploading the patch. Please also note that the tests depend on 2 external fsimage files. These are not present in the patch file, because the patch can't carry binary files. Instead, I am attaching the files to this issue. When we commit this patch, we also need to commit these files at src/test/org/apache/hadoop/hdfs/tools/offlineImageViewer. For the backport, I started with the current trunk version of the code. I took the docs from the current branch-0.22, because they were in the old XML format, but they were more complete than the original docs from the HADOOP-5467 patch. Then, there were a few minor adjustments required to make the code work with branch-1: I've included LayoutVersion , because ImageLoaderCurrent needs it to determine what to parse. FSImageSerialization class is not available in branch-1. The original implementation of this code used the read methods in FSImage instead. ImageLoaderCurrent needed some new methods in CompressionCodecFactory , so I included those. TestOfflineImageViewer depended on a special testing flag not present in branch-1 to force generation of delegation tokens. I've included a minimal port of this flag so that we can maintain the same level of testing coverage. The trunk version of the code uses FSEditLogLoader#PositionTrackingInputStream so that oiv can report the byte offset where a failure occurred in the event of a corrupted fsimage. This class is not present in branch-1, and porting it would have swept in a lot of unrelated code changes. Instead, I have provided a minimal version as a nested class of OfflineImageViewer . All new tests pass, and I have manually tested on fsimage files from a deployed branch-1-based cluster.
          Chris Nauroth made changes -
          Field Original Value New Value
          Attachment fsimageV18 [ 12576191 ]
          Attachment fsimageV19 [ 12576192 ]
          Hide
          Chris Nauroth added a comment -

          Attaching several testing fsimage files that will need to be committed in addition to the patch.

          Show
          Chris Nauroth added a comment - Attaching several testing fsimage files that will need to be committed in addition to the patch.
          Hide
          Chris Nauroth added a comment -

          From some initial work, I think we can start with the current trunk version of Offline Image Viewer, copy it to branch-1, and then make some minor adjustments for compatibility with the branch-1 codebase structure. This would include porting the LayoutVersion class, which may be a useful thing to have on branch-1 even outside the context of OIV.

          Show
          Chris Nauroth added a comment - From some initial work, I think we can start with the current trunk version of Offline Image Viewer, copy it to branch-1, and then make some minor adjustments for compatibility with the branch-1 codebase structure. This would include porting the LayoutVersion class, which may be a useful thing to have on branch-1 even outside the context of OIV.
          Chris Nauroth created issue -

            People

            • Assignee:
              Chris Nauroth
              Reporter:
              Chris Nauroth
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development