I am a bit confused here. I see that you added in a new mapreduce StreamInputFormat, with the corresponding StreamXmlRecordReader and StreamBaseRecordReader, but how does this enable us to use the new MapReduce API? Can you update the documentation to provide some examples of how you can use these new classes you have added? Also the test you have added in is not actually testing the new code at all. It is still testing the old input format code. I can delete the new code entirely and the test still passes. It looks like a great start, but I think there is some more wiring that needs to be done to make this work.

Also, MAPREDUCE-1122 would be a useful pre-requisite for this.

I am a bit confused here. I see that you added in a new mapreduce StreamInputFormat, with the corresponding StreamXmlRecordReader and StreamBaseRecordReader, but how does this enable us to use the new MapReduce API?
Can you update the documentation to provide some examples of how you can use these new classes you have added?

>> New Map Reduce API requires InputFormat class to extend org.apache.hadoop.mapreduce.InputFormat but StreamInputFormat is extending org.apache.hadoop.mapred.InputFormat.So when I try to set it in Job as below
job.setInputFormatClass(StreamInputFormat.class);
it gives compilation error. More info here http://search-hadoop.com/m/evL3S1deWQ72 .
So when I refer new API, i mean that porting the StreamInputFormat to new InputFormat class , so that it can be used with new API code.

Also the test you have added in is not actually testing the new code at all. It is still testing the old input format code. I can delete the new code entirely and the test still passes. It looks like a great start, but I think there is some more wiring that needs to be done to make this work.
>> updated the test case

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12533028/HADOOP-8521-1.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 javac. The applied patch generated 2057 javac compiler warnings (more than the trunk's current 2054 warnings).

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

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

-1 findbugs. The patch appears to introduce 1 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 hadoop-tools/hadoop-streaming.

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

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1138//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1138//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-streaming.html
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1138//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1138//console

This message is automatically generated.

Canceling patch so that findbugs issues can be addressed

updated the patch to fix findbug issue

also, some code style issues

1. ASF comment should appear at top of files
2. some of the files have different indentation policy from the Hadoop code -an IDE can fix that.
3. variables marked as final static in StreamXmlRecordReader should be private or protected unless there is a very good reason for package scoping.

The thing that worries me most is the call to init() in the constructor of StreamXmlRecordReader. Unless that class or method is final, there's the risk that a subclass will have its init() method called before that subclass is fully initialized, which leads to bad things.

If moving the init phase out of the constructor isn't possible because of the way Hadoop instantiates & uses the reader, then that class or method ought to be final.

Hi,
Most of the code is reserved from earlier old API code, only making the changes in places where new API is required. Other than style issues, if its needed to be changed we have to make changes to both old and new code.

I understand. This may be the time to do those changes, as there's no reason not to improve the codebase except when it makes patching worse across branches.

updated patch with style fixes and code fixes

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12534149/HADOOP-8521-3.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 javac. The applied patch generated 2073 javac compiler warnings (more than the trunk's current 2070 warnings).

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

+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 hadoop-tools/hadoop-streaming.

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

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1161//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1161//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1161//console

This message is automatically generated.

Please take a look at the new javac warnings

https://builds.apache.org/job/PreCommit-HADOOP-Build/1161/artifact/trunk/patchprocess/diffJavacWarnings.txt

You don't necessarily need to fix them, just make sure that they are what you expect and are things that don't need to be fixed.

Hi Robert,
These javac warning are resulted from use of Reflection API and old API deprecation. I think these warning should be fine since they don't effect actual functionality.

Madhukara,

The patch looks good to me, most of the patch appears to be a copy and paste from the existing code with a few tweaks for the new APIs. I am not an expert on the reader APIs, nor am I an expert on how streaming loads and uses these APIs, so I am giving this patch a +1, but I would appreciate someone else to take a look, especially in relation to MAPREDUCE-1122.

If no one else takes a look in the next day or so I'll assume that this is OK and check it in. Thanks for your work on the Madhukara. Sorry it has taken so long.

Thanks Madhukara,

I put this into trunk.

Integrated in Hadoop-Hdfs-trunk-Commit #2516 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2516/)
HADOOP-8521. Port StreamInputFormat to new Map Reduce API (madhukara phatak via bobby) (Revision 1360238)

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

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamUtil.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamInputFormat.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce/TestStreamXmlRecordReader.java

Integrated in Hadoop-Common-trunk-Commit #2450 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2450/)
HADOOP-8521. Port StreamInputFormat to new Map Reduce API (madhukara phatak via bobby) (Revision 1360238)

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

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamUtil.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamInputFormat.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce/TestStreamXmlRecordReader.java

Integrated in Hadoop-Mapreduce-trunk-Commit #2468 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2468/)
HADOOP-8521. Port StreamInputFormat to new Map Reduce API (madhukara phatak via bobby) (Revision 1360238)

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

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamUtil.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamInputFormat.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce/TestStreamXmlRecordReader.java

Integrated in Hadoop-Hdfs-trunk #1101 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1101/)
HADOOP-8521. Port StreamInputFormat to new Map Reduce API (madhukara phatak via bobby) (Revision 1360238)

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

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamUtil.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamInputFormat.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce/TestStreamXmlRecordReader.java

Integrated in Hadoop-Mapreduce-trunk #1134 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1134/)
HADOOP-8521. Port StreamInputFormat to new Map Reduce API (madhukara phatak via bobby) (Revision 1360238)

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

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamUtil.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamBaseRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamInputFormat.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/streaming/mapreduce/StreamXmlRecordReader.java
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce
• /hadoop/common/trunk/hadoop-tools/hadoop-streaming/src/test/java/org/apache/hadoop/streaming/mapreduce/TestStreamXmlRecordReader.java