Hadoop Common
  1. Hadoop Common
  2. HADOOP-6996

Allow CodecFactory to return a codec object given a codec' class name

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: io
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      CodecFactory specify the list of codec that are supported by Hadoop. However, it returns a codec only by a file's name. I would like to make getCodec method to alternatively take a codec's class name.

      This is required by HDFS-1435, where
      1. it allows an HDFS admin to configure which codec to use to save an image.
      2. It stores the codec class name in its on-disk image instead of a file's suffix.

      When saving and reading an image, I'd like to get an codec from CodecFactory by its class name.

        Issue Links

          Activity

          Hide
          Hairong Kuang added a comment -

          This patch
          1. adds a classname to codec map to the CodecFactory;
          2. adds a new method getCodecByClassName that returns a codec object given a codec class name;
          3. Adds a unit test to test this new method.

          Show
          Hairong Kuang added a comment - This patch 1. adds a classname to codec map to the CodecFactory; 2. adds a new method getCodecByClassName that returns a codec object given a codec class name; 3. Adds a unit test to test this new method.
          Hide
          Hairong Kuang added a comment -

          Ran ant patch and test on my dev machine:

          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to i
          [exec] nclude 3 new or modified tests.
          [exec]
          [exec] -1 javadoc. The javadoc tool appears to have generated 1 warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system tests framework. The patch passed system tests framework compile.
          [exec]

          BUILD SUCCESSFUL
          Total time: 8 minutes 8 seconds

          For -1 on java doc, all 6 javadoc warnings seem not related to my patch.

          Show
          Hairong Kuang added a comment - Ran ant patch and test on my dev machine: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to i [exec] nclude 3 new or modified tests. [exec] [exec] -1 javadoc. The javadoc tool appears to have generated 1 warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system tests framework. The patch passed system tests framework compile. [exec] BUILD SUCCESSFUL Total time: 8 minutes 8 seconds For -1 on java doc, all 6 javadoc warnings seem not related to my patch.
          Hide
          Hairong Kuang added a comment -

          This patch additionally adds a default constructor to CompressionCodecFactory.

          Show
          Hairong Kuang added a comment - This patch additionally adds a default constructor to CompressionCodecFactory.
          Hide
          dhruba borthakur added a comment -

          Code looks good. One minor comment:

          public CompressionCodec getCodecByClassName(String classname) {
              if (codecsByClassName == null) {
               return null;
             }
              return codecsByClassName.get(classname);
          }
          
          

          do we really need to check for codecsByClassName == null? I see that it always initialized in the constructor.

          Show
          dhruba borthakur added a comment - Code looks good. One minor comment: public CompressionCodec getCodecByClassName( String classname) { if (codecsByClassName == null ) { return null ; } return codecsByClassName.get(classname); } do we really need to check for codecsByClassName == null? I see that it always initialized in the constructor.
          Hide
          Hairong Kuang added a comment -

          You are right. But before I add the default class constructor, there was a possibility that codecClassName is null. Now I realize that adding the default constructor is an incompatible change. Let me revert this change by deleting the 2nd patch: getCodecByClassName1.patch.

          Show
          Hairong Kuang added a comment - You are right. But before I add the default class constructor, there was a possibility that codecClassName is null. Now I realize that adding the default constructor is an incompatible change. Let me revert this change by deleting the 2nd patch: getCodecByClassName1.patch.
          Hide
          dhruba borthakur added a comment -

          +1 looks good to me.

          Show
          dhruba borthakur added a comment - +1 looks good to me.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #395 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/395/)

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #395 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/395/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #489 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/489/)

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #489 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/489/ )
          Hide
          Hadoop QA added a comment -

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

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

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

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

          Console output: http://hudson.zones.apache.org/hudson/job/PreCommit-HADOOP-Build/2/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12456881/getCodecByClassName.patch against trunk revision 1022697. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/PreCommit-HADOOP-Build/2/console This message is automatically generated.
          Hide
          Hairong Kuang added a comment -

          I've committed this!

          Show
          Hairong Kuang added a comment - I've committed this!

            People

            • Assignee:
              Hairong Kuang
              Reporter:
              Hairong Kuang
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development