Hadoop Common
  1. Hadoop Common
  2. HADOOP-7350

Use ServiceLoader to discover compression codec classes

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-alpha
    • Component/s: conf, io
    • Labels:
      None

      Description

      By using a ServiceLoader users wouldn't have to add codec classes to io.compression.codecs for codecs that aren't shipped with Hadoop (e.g. LZO), since they would be automatically picked up from the classpath.

      1. HADOOP-7350.patch
        10 kB
        Tom White
      2. HADOOP-7350.patch
        9 kB
        Tom White
      3. HADOOP-7350.patch
        9 kB
        Tom White
      4. HADOOP-7350.patch
        8 kB
        Tom White
      5. HADOOP-7350.patch
        8 kB
        Tom White
      6. HADOOP-7350.patch
        8 kB
        Tom White
      7. HADOOP-7350.patch
        8 kB
        Tom White
      8. HADOOP-7350.patch
        7 kB
        Tom White

        Activity

        Hide
        Tom White added a comment -

        This patch modifies CompressionCodecFactory.getCodecClasses() to use a service loader in addition to reading class names from io.compression.codecs.

        Show
        Tom White added a comment - This patch modifies CompressionCodecFactory.getCodecClasses() to use a service loader in addition to reading class names from io.compression.codecs.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481172/HADOOP-7350.patch
        against trunk revision 1129989.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/557//testReport/
        Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/557//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/557//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/12481172/HADOOP-7350.patch against trunk revision 1129989. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/557//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/557//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/557//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -
        • We should probably remove the codecs from core-default.xml now that they're loaded via ServiceLoader
        • Is there a way to inject a new codec programatically through the ServiceLoader interface? If so, we could entirely deprecate io.compression.codecs. If not, maybe we should rename it to something like io.compression.extra.codecs and specify that it's only necessary if you have a codec that doesn't expose itself through ServiceLoader?
        • hdfs-default.xml has an item dfs.image.compression.codec that needs to be updated
        Show
        Todd Lipcon added a comment - We should probably remove the codecs from core-default.xml now that they're loaded via ServiceLoader Is there a way to inject a new codec programatically through the ServiceLoader interface? If so, we could entirely deprecate io.compression.codecs. If not, maybe we should rename it to something like io.compression.extra.codecs and specify that it's only necessary if you have a codec that doesn't expose itself through ServiceLoader? hdfs-default.xml has an item dfs.image.compression.codec that needs to be updated
        Hide
        Tom White added a comment -
        • We should probably remove the codecs from core-default.xml now that they're loaded via ServiceLoader

        Done - see new patch.

        • Is there a way to inject a new codec programatically through the ServiceLoader interface? If so, we could entirely deprecate io.compression.codecs. If not, maybe we should rename it to something like io.compression.extra.codecs and specify that it's only necessary if you have a codec that doesn't expose itself through ServiceLoader?

        I don't think we need to deprecate or rename io.compression.codecs - it's just used to specify additional codecs to the ones that are loaded through a ServiceLoader. Note that duplicates are ignored, so there's no problem with users older configs having codecs that could be loaded through ServiceLoader.

        • hdfs-default.xml has an item dfs.image.compression.codec that needs to be updated

        This doesn't need to be updated, although with HADOOP-7323 (and a corresponding HDFS change) it could be changed to "default".

        Show
        Tom White added a comment - We should probably remove the codecs from core-default.xml now that they're loaded via ServiceLoader Done - see new patch. Is there a way to inject a new codec programatically through the ServiceLoader interface? If so, we could entirely deprecate io.compression.codecs. If not, maybe we should rename it to something like io.compression.extra.codecs and specify that it's only necessary if you have a codec that doesn't expose itself through ServiceLoader? I don't think we need to deprecate or rename io.compression.codecs - it's just used to specify additional codecs to the ones that are loaded through a ServiceLoader. Note that duplicates are ignored, so there's no problem with users older configs having codecs that could be loaded through ServiceLoader. hdfs-default.xml has an item dfs.image.compression.codec that needs to be updated This doesn't need to be updated, although with HADOOP-7323 (and a corresponding HDFS change) it could be changed to "default".
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481287/HADOOP-7350.patch
        against trunk revision 1130758.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/563//testReport/
        Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/563//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/563//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/12481287/HADOOP-7350.patch against trunk revision 1130758. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/563//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/563//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/563//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        This doesn't need to be updated

        Sorry, the default doesn't need to change, but the docs should be updated: it no longer needs to be one of the codecs listed in io.compression.codecs. It just needs to be any codec that's "registered" (either via conf or via ServiceLoader). I don't know the best verbiage to succinctly document it, though.

        The rest of your points make sense.

        Show
        Todd Lipcon added a comment - This doesn't need to be updated Sorry, the default doesn't need to change, but the docs should be updated: it no longer needs to be one of the codecs listed in io.compression.codecs. It just needs to be any codec that's "registered" (either via conf or via ServiceLoader). I don't know the best verbiage to succinctly document it, though. The rest of your points make sense.
        Hide
        Todd Lipcon added a comment -

        +1 on most recent patch

        Show
        Todd Lipcon added a comment - +1 on most recent patch
        Hide
        Tom White added a comment -

        Slight adjustment to load codec classes only once using a ServiceLoader.

        I'll address the HDFS documentation change in another JIRA.

        Show
        Tom White added a comment - Slight adjustment to load codec classes only once using a ServiceLoader. I'll address the HDFS documentation change in another JIRA.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481304/HADOOP-7350.patch
        against trunk revision 1130833.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/565//testReport/
        Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/565//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/565//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/12481304/HADOOP-7350.patch against trunk revision 1130833. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/565//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/565//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/565//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        hmm, the two most recent patches are identical best I can see...

        Show
        Todd Lipcon added a comment - hmm, the two most recent patches are identical best I can see...
        Hide
        Tom White added a comment -

        Sorry - this should be the right patch now.

        Show
        Tom White added a comment - Sorry - this should be the right patch now.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481630/HADOOP-7350.patch
        against trunk revision 1132776.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/586//testReport/
        Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/586//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/586//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/12481630/HADOOP-7350.patch against trunk revision 1132776. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/586//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/586//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/586//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        Curious: why the change to only load once in the static initializer?

        Show
        Todd Lipcon added a comment - Curious: why the change to only load once in the static initializer?
        Hide
        Tom White added a comment -

        I moved it to take advantage of the ServiceLoader's caching of providers. This new patch does that but moves the iteration back to the getCodecClasses() method. (This is now like the example in the ServiceLoader documentation.)

        Show
        Tom White added a comment - I moved it to take advantage of the ServiceLoader's caching of providers. This new patch does that but moves the iteration back to the getCodecClasses() method. (This is now like the example in the ServiceLoader documentation.)
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481726/HADOOP-7350.patch
        against trunk revision 1132776.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/587//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/587//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/587//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/12481726/HADOOP-7350.patch against trunk revision 1132776. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/587//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/587//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/587//console This message is automatically generated.
        Hide
        Owen O'Malley added a comment -

        Tom,
        We should apply this. Can you rebase it to reflect the mavenization?

        Show
        Owen O'Malley added a comment - Tom, We should apply this. Can you rebase it to reflect the mavenization?
        Hide
        Owen O'Malley added a comment -

        It needs to be rebased to the mavenized source tree.

        Show
        Owen O'Malley added a comment - It needs to be rebased to the mavenized source tree.
        Hide
        Tom White added a comment -

        Yes, this got forgotten. I've updated the patch to the new source structure, and added a couple of tests to check that Snappy and LZ4 extensions are picked up.

        Show
        Tom White added a comment - Yes, this got forgotten. I've updated the patch to the new source structure, and added a couple of tests to check that Snappy and LZ4 extensions are picked up.
        Hide
        Hadoop QA added a comment -

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

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

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

        -1 javadoc. The javadoc tool appears to have generated 7 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/528//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/528//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/12511199/HADOOP-7350.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 7 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/528//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/528//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        Tom, thanks for reviving this one. Things look OK, the only thing is that I'd revert the logic to first load the codecs defined as services and after that the ones defined in Hadoop's configuration. By doing that, it is possible for a user to override a provided implementation.

        Show
        Alejandro Abdelnur added a comment - Tom, thanks for reviving this one. Things look OK, the only thing is that I'd revert the logic to first load the codecs defined as services and after that the ones defined in Hadoop's configuration. By doing that, it is possible for a user to override a provided implementation.
        Hide
        Tom White added a comment -

        Thanks for taking a look, Alejandro. Here's an updated patch in which codecs defined in the configuration take precedence over those picked up by the service loader.

        Show
        Tom White added a comment - Thanks for taking a look, Alejandro. Here's an updated patch in which codecs defined in the configuration take precedence over those picked up by the service loader.
        Hide
        Alejandro Abdelnur added a comment -

        I may have expressed incorrectly, my suggestion was that in the getCodecClasses(Configuration) method, the loop loading the codecs from CODEC_PROVIDERS should be done before loading the codecs from the Hadoop 'io.compression.codec' property. Also, in the CompressionCodecFactory() constructor, doing a reverse of the list seems wrong as that would effectively put the service loaded ones first. I think that the reverse should not be there, then the list of codecs is ALL_SERVICE_CODECS (in non-deterministic order) and then the configuration set ones, in order or appearance where the last one overrides.

        Show
        Alejandro Abdelnur added a comment - I may have expressed incorrectly, my suggestion was that in the getCodecClasses(Configuration) method, the loop loading the codecs from CODEC_PROVIDERS should be done before loading the codecs from the Hadoop 'io.compression.codec' property. Also, in the CompressionCodecFactory() constructor, doing a reverse of the list seems wrong as that would effectively put the service loaded ones first. I think that the reverse should not be there, then the list of codecs is ALL_SERVICE_CODECS (in non-deterministic order) and then the configuration set ones, in order or appearance where the last one overrides.
        Hide
        Hadoop QA added a comment -

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

        +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: https://builds.apache.org/job/PreCommit-HADOOP-Build/768//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/12514395/HADOOP-7350.patch against trunk revision . +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: https://builds.apache.org/job/PreCommit-HADOOP-Build/768//console This message is automatically generated.
        Hide
        Tom White added a comment -

        Updated patch to address Alejandro's feedback. Note that codecs defined in the configuration still take precedence over those picked up by the service loader.

        Show
        Tom White added a comment - Updated patch to address Alejandro's feedback. Note that codecs defined in the configuration still take precedence over those picked up by the service loader.
        Hide
        Hadoop QA added a comment -

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

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.fs.viewfs.TestViewFsTrash

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/850//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/850//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/12522601/HADOOP-7350.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.fs.viewfs.TestViewFsTrash +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/850//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/850//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        +1

        small NIT:

        Where you do:

        +    List<Class<? extends CompressionCodec>> result
        +      = new ArrayList<Class<? extends CompressionCodec>>();
        +    // Add codec classes discovered via service loading
        +    for (CompressionCodec codec : CODEC_PROVIDERS) {
        +      if (!result.contains(codec.getClass())) {
        +        result.add(codec.getClass());
        +      }
        +    }
        

        The IF NOT is not required as the ServiceLoader will ignore duplicate classes in a non-deterministic way per ServiceLoader javadocs.

        Show
        Alejandro Abdelnur added a comment - +1 small NIT: Where you do: + List< Class <? extends CompressionCodec>> result + = new ArrayList< Class <? extends CompressionCodec>>(); + // Add codec classes discovered via service loading + for (CompressionCodec codec : CODEC_PROVIDERS) { + if (!result.contains(codec.getClass())) { + result.add(codec.getClass()); + } + } The IF NOT is not required as the ServiceLoader will ignore duplicate classes in a non-deterministic way per ServiceLoader javadocs.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2182 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2182/)
        HADOOP-7350. Use ServiceLoader to discover compression codec classes. (Revision 1328083)

        Result = SUCCESS
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2182 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2182/ ) HADOOP-7350 . Use ServiceLoader to discover compression codec classes. (Revision 1328083) Result = SUCCESS tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2108 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2108/)
        HADOOP-7350. Use ServiceLoader to discover compression codec classes. (Revision 1328083)

        Result = SUCCESS
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2108 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2108/ ) HADOOP-7350 . Use ServiceLoader to discover compression codec classes. (Revision 1328083) Result = SUCCESS tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Hide
        Tom White added a comment -

        I fixed the nit and committed this. Thanks for the review Alejandro.

        Show
        Tom White added a comment - I fixed the nit and committed this. Thanks for the review Alejandro.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2125 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2125/)
        HADOOP-7350. Use ServiceLoader to discover compression codec classes. (Revision 1328083)

        Result = ABORTED
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2125 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2125/ ) HADOOP-7350 . Use ServiceLoader to discover compression codec classes. (Revision 1328083) Result = ABORTED tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1020 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1020/)
        HADOOP-7350. Use ServiceLoader to discover compression codec classes. (Revision 1328083)

        Result = FAILURE
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1020 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1020/ ) HADOOP-7350 . Use ServiceLoader to discover compression codec classes. (Revision 1328083) Result = FAILURE tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1055 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1055/)
        HADOOP-7350. Use ServiceLoader to discover compression codec classes. (Revision 1328083)

        Result = FAILURE
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1055 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1055/ ) HADOOP-7350 . Use ServiceLoader to discover compression codec classes. (Revision 1328083) Result = FAILURE tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328083 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/compress/CompressionCodecFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.io.compress.CompressionCodec /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodecFactory.java

          People

          • Assignee:
            Tom White
            Reporter:
            Tom White
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development