I think it does break other InputFormat implementations. For example, FileInputFormat returns number of splits as 1 for an empty input file and the method getLocations() in FileSplit returns a 0 length array. This can cause array index out of bound exception if one is not careful enough to check the array length.

There is also another potential problem. When an MR job is run with an empty input, if the number of splits is 0, the number of mappers will be 0 and number of reducers can be non-zero. I am not sure whether the MR job will run successfully. My preference is to have all InputFormat implementations return 1 split of size 0 when the input is empty.

On a side note, when this test failed, the assertion message

testForEmptyFile(org.apache.hadoop.mapreduce.lib.input.TestCombineFileInputFormat): expected:<0> but was:<1>

was confusing. It should be:

testForEmptyFile(org.apache.hadoop.mapreduce.lib.input.TestCombineFileInputFormat): expected:<1> but was:<0>

Upon examining the test, there are several places where the parameter order is wrong to assertEquals(). I raised MAPREDUCE-4479 and posted a patch there.

I think we should fix this on the InputFormat side, if we expect an empty job to have 1 empty split. I think the underlying fix in Common is indeed an improvement.

Todd,
In the meantime, do you have any suggestion on how I can get a clean trunk build? Also, after this is fixed, can you please commit MAPREDUCE-4479 to the trunk?
Thanks.

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

+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. 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-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

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

This message is automatically generated.

I think a proper fix should address all InputFormat implementations. Tests for empty input should be added for all input formats. For example, I added a test in TestFileInputFormat.java to test for empty input. It is also failing.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12540068/TestFileInputFormat.java
against trunk revision .

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

Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2721//console

This message is automatically generated.

Sorry about the file upload. I did not mean it to be a patch

Mariappan,
Thank you for a test. Could you please clarify what is the correct way to obtain split locations in InputFormat?

Hi Ilya Katsov,
Sorry, I was not able to get back ASAP since Jira was down. I looked at the code for FileInputFormat. I am sorry, I misled you. The method getLocations() in InputSplit CAN return a 0 length array for an empty file since there is no physical block of storage associated with the file. I have attached the new version of TestFileInputFormat.java. I feel that it is good to have a test for an empty file since it tests a boundary condition.

Cheers,
– Asokan

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12540941/TestFileInputFormat.java
against trunk revision .

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

Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2731//console

This message is automatically generated.

Mariappan,
I've added this test to the patch. As far as I see, getSplits depends directly on FileSystem.getFileBlockLocations only in FileInputFormat and CombinedFileInputFormat, so other InputFormat implementations should be OK. Please let me know if I'm wrong on this.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12541014/MAPREDUCE-4470-2.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 does not increase the total number of javac compiler 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-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient.

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

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

This message is automatically generated.

Hi Ilya Katsov,
You are right. Your patch should fix the affected test. However, I think NLineInputFormat, DBInputFormat, and CompositeInputFormat implementations may fail on empty input for different reasons. There is no test for empty input for them I think this can be addressed in a different Jira.

By the way, is there a committer who is going to commit your patch?

Thanks.
– Asokan

>> By the way, is there a committer who is going to commit your patch?
No.

+1, the latest patch looks good to me. I have also locally applied the patch and I am no longer seeing the failure. Thanks!

With and without the patch I see the following failure:

$ mvn clean test -Dtest=TestCombineFileInputFormat
...
Running org.apache.hadoop.mapred.TestCombineFileInputFormat
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.673 sec
Running org.apache.hadoop.mapreduce.lib.input.TestCombineFileInputFormat
Tests run: 6, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 7.481 sec <<< FAILURE!

Results :

Tests in error: 
  testMissingBlocks(org.apache.hadoop.mapreduce.lib.input.TestCombineFileInputFormat): org.apache.hadoop.fs.BlockLocation
...

Seems the new changes pushed by HDFS-3672 exposed more errors. I see that the test was succeeding before this commit and failing afterwards.

I've just committed the fix for the ArrayStoreException in TestCombineFileInputFormat as described in MAPREDUCE-4577. We should be good to go on this JIRA.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12541014/MAPREDUCE-4470-2.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 does not increase the total number of javac compiler 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-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient.

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

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

This message is automatically generated.

+1

Thanks Iilya. Committed to trunk and branch-2. Would you please open a follow up JIRA for the other inputformats?

Integrated in Hadoop-Mapreduce-trunk-Commit #2653 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2653/)
MAPREDUCE-4470. Fix TestCombineFileInputFormat.testForEmptyFile (ikatsov via tucu) (Revision 1376325)

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

• /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/input/CombineFileInputFormat.java
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestFileInputFormat.java

Integrated in Hadoop-Hdfs-trunk-Commit #2689 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2689/)
MAPREDUCE-4470. Fix TestCombineFileInputFormat.testForEmptyFile (ikatsov via tucu) (Revision 1376325)

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

• /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/input/CombineFileInputFormat.java
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestFileInputFormat.java

Integrated in Hadoop-Common-trunk-Commit #2625 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2625/)
MAPREDUCE-4470. Fix TestCombineFileInputFormat.testForEmptyFile (ikatsov via tucu) (Revision 1376325)

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

• /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/input/CombineFileInputFormat.java
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestFileInputFormat.java

Integrated in Hadoop-Hdfs-trunk #1143 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1143/)
MAPREDUCE-4470. Fix TestCombineFileInputFormat.testForEmptyFile (ikatsov via tucu) (Revision 1376325)

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

• /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/input/CombineFileInputFormat.java
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestFileInputFormat.java

Integrated in Hadoop-Mapreduce-trunk #1175 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1175/)
MAPREDUCE-4470. Fix TestCombineFileInputFormat.testForEmptyFile (ikatsov via tucu) (Revision 1376325)

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

• /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/input/CombineFileInputFormat.java
• /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestFileInputFormat.java

Integrated in Hadoop-Hdfs-0.23-Build #513 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/513/)
MAPREDUCE-4470. Fix TestCombineFileInputFormat.testForEmptyFile (ikatsov via tgraves) (Revision 1441494)

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

• /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
• /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/input/CombineFileInputFormat.java
• /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestFileInputFormat.java