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

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

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

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

+1 javadoc. There were no new javadoc warning messages.

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

-1 findbugs. The patch appears to introduce 3 new Findbugs (version 2.0.3) 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 in hadoop-common-project/hadoop-common:

org.apache.hadoop.crypto.random.TestOsSecureRandom

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5288//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/5288//artifact/patchprocess/newPatchFindbugsWarningshadoop-common.html
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5288//console

This message is automatically generated.

Could someone give a glance to this issue and comment?

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

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

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

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

+1 javadoc. There were no new javadoc warning messages.

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

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 hadoop-common-project/hadoop-common.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5826//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5826//console

This message is automatically generated.

Thanks fang fang chen for the report and the patch. Nice catch. The fix looks good to me. Two comments:
1. The code lookups conf twice. I'm thinking it's better to do in a single lookup as follows:

          String extClazz = conf.get(CONF_LZO_CLASS);
          if (extClazz == null) {
            extClazz = System.getProperty(CONF_LZO_CLASS);
          }

2. Would you add a regression test for this? I'm thinking the test will

• Set "io.compression.codec.lzo.class" to a codec other than the default lzo codec (ex. o.a.h.io.compress.DefaultCodec)
• Call Algorithm.LZO.isSupported()
• Call Algorithm.LZO.getCodec() and verify the specified codec is obtained.

Sure, thanks for looking at this JIRA, Akira. I generated new patch based on current trunk. And the regression test has been made with pig before, here is the process and log:
1. Add following settings in hadoop core-site.xml:
<property>
<name>io.compression.codecs</name>
<value>com.ibm.biginsights.compress.CmxCodec</value>
</property>
<property>
<name>io.compression.codec.lzo.class</name>
<value>com.ibm.biginsights.compress.CmxCodec</value>
</property>
2. Launch pig grunt with properties:
-Dpig.tmpfilecompression=true -Dpig.tmpfilecompression.codec=cmx -Dpig.additional.jars=ibm-compression.jar:libcmxbiginsights.so

Together with patch in PIG-4341, test case passed with following massage:
2014-12-22 17:37:24,453 [main] INFO org.apache.pig.backend.hadoop.executionengine.util.MapRedUtil - Total input paths to process : 2
2014-12-22 17:37:24,550 [main] INFO org.apache.hadoop.io.file.tfile.Compression - Trying to load Lzo codec class: com.ibm.biginsights.compress.CmxCodec
2014-12-22 17:37:24,554 [main] INFO com.ibm.biginsights.compress.NativeCodeLoader - Loaded the native library
2014-12-22 17:37:24,554 [main] INFO org.apache.hadoop.io.compress.CodecPool - Got brand-new decompressor [.cmx]

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

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

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

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

+1 javadoc. There were no new javadoc warning messages.

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

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 hadoop-common-project/hadoop-common.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5829//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5829//console

This message is automatically generated.

And the regression test has been made with pig before, here is the process and log:

The log looks good to me, however, I'd like to add a regression test in hadoop side also. I'm not sure PIG-4341 will be merged soon.

+              (extClazz_conf == null ? System.getProperty(CONF_LZO_CLASS) : extClazz_conf);

• The line contains unnecessary parentheses. Would you remove them?
• The line is longer than 80 characters. Would you render it within 80 characters?

About "unnecessary parentheses", do you mean it should me like this:
+ String extClazz = extClazz_conf == null ? System
+ .getProperty(CONF_LZO_CLASS) : extClazz_conf;

It looks like a little strange.

Let me provide 2 patch files, HADOOP-11418-3.patch remove parentheses and HADOOP-11418-2.patch keep the parentheses.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12702381/HADOOP-11418-3.patch
against trunk revision 29bb689.

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

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

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

+1 javadoc. There were no new javadoc warning messages.

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

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 hadoop-common-project/hadoop-common.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5839//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5839//console

This message is automatically generated.

It looks like a little strange.

I agree with you. Rethinking this, the below looks clear to me. What do you think?

  String extClazz = (extClazz_conf == null) ?
      System.getProperty(CONF_LZO_CLASS) : extClazz_conf;

Would you please add a regression test? If you don't want to add it, I'd like to succeed your work.

Please go ahead to add a regression test. Thanks

Thanks fang fang chen. Added a regression test.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12707171/HADOOP-11418-004.patch
against trunk revision b351086.

+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 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

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

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 hadoop-common-project/hadoop-common.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5995//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5995//console

This message is automatically generated.

v5 patch:

• Fix inconsistency between

            String extClazz = (extClazz_conf == null) ?
                System.getProperty(CONF_LZO_CLASS) : extClazz_conf;
  

  and

            String clazz = (extClazz != null) ? extClazz : defaultClazz;
  

  . One is using ==, the other is using !=.

-1 overall

This message was automatically generated.

Fixed checkstyle issue.

+1 overall

This message was automatically generated.

-1 overall

This message was automatically generated.

Akira Ajisaka Thanks for your work. This issue has been assigned to me. I've reviewed the patch and it looks good. Could you please tell me what the next step is so that we can fix and close this issue?

Thank you for your comment. The latest patch needs another committer's +1 because I wrote it. Hi Masatake Iwasaki, would you please review the latest patch?

+1 from me. Committing this.

FAILURE: Integrated in Hadoop-trunk-Commit #9663 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9663/)
HADOOP-11418. Property "io.compression.codec.lzo.class" does not work (iwasakims: rev bec5b4cd8c1fb27a5b707d601b508224a9c107ed)

• hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Compression.java
• hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestCompression.java

Committed. Thanks, Yuanbo Liu, fang fang chen and Akira Ajisaka.

Thanks Masatake!

Thanks for your great work, Akira and Masatake