You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-issues@hadoop.apache.org by "Niels Basjes (JIRA)" <ji...@apache.org> on 2010/09/28 11:41:35 UTC
[jira] Created: (MAPREDUCE-2094)
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable
implements unsafe default behaviour that is different from the documented
behaviour.
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable implements unsafe default behaviour that is different from the documented behaviour.
-------------------------------------------------------------------------------------------------------------------------------------------------------
Key: MAPREDUCE-2094
URL: https://issues.apache.org/jira/browse/MAPREDUCE-2094
Project: Hadoop Map/Reduce
Issue Type: Improvement
Components: task
Affects Versions: 0.21.0, 0.20.2, 0.20.1
Reporter: Niels Basjes
When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times. A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;". This is a very unsafe default and is in contradcition with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. "
For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable inour class that does "return false; "
Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
# Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
# "Force" developers to think about it and make this method (and therfor the entire FileInputFormat class) abstract.
# Use a "safe" default (i.e. return false)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAPREDUCE-2094)
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable
implements unsafe default behaviour that is different from the documented
behaviour.
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAPREDUCE-2094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915929#action_12915929 ]
Doug Cutting commented on MAPREDUCE-2094:
-----------------------------------------
Changing the default implementation to return false would be an incompatible change, potentially breaking existing subclasses. Making the method abstract would also be incompatible and break subclasses, but in a way that they'd easily detect.
Perhaps the javadoc should just be clarified to better document this default?
> org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable implements unsafe default behaviour that is different from the documented behaviour.
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-2094
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2094
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: task
> Affects Versions: 0.20.1, 0.20.2, 0.21.0
> Reporter: Niels Basjes
>
> When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times.
> A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
> It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;".
> This is a very unsafe default and is in contradiction with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. " . The actual implementation effectively does "Is the given filename splitable? Always true, even if the file is stream compressed using an unsplittable compression codec. "
> For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable in our class that does "return false; "
> Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
> # Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
> # "Force" developers to think about it and make this method abstract.
> # Use a "safe" default (i.e. return false)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAPREDUCE-2094)
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable
implements unsafe default behaviour that is different from the documented
behaviour.
Posted by "Niels Basjes (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAPREDUCE-2094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916076#action_12916076 ]
Niels Basjes commented on MAPREDUCE-2094:
-----------------------------------------
bq. Changing the default implementation to return false would be an incompatible change, potentially breaking existing subclasses.
If you mean with "breaking" : "Some subclasses will see an unexpected performance degradation" . then Yes, that will most likely occur ( first one I think of is SequenceFileInputFormat ).
I however do not expect any functional "breaking" of the output of these subclasses.
bq. Making the method abstract would also be incompatible and break subclasses, but in a way that they'd easily detect.
Yes.
The downside of this option is that if subclasses want to have detection depending on the compression I expect a lot of code duplication to occur.
This code duplication is already occurring within the main code base in KeyValueTextInputFormat , TextInputFormat, CombineFileInputFormat and their old API counterparts (I found a total of 5 duplicates of the same isSplittable implementation).
bq. Perhaps the javadoc should just be clarified to better document this default?
Definitely an option. However this would not fix the effect in the existing subclasses.
I just did a quick manual code check of the current trunk and I found that the following classes are derived from FileInputFormat yet do not implement the isSplittable method (and thus use "return true").
* ./src/java/org/apache/hadoop/mapreduce/lib/input/NLineInputFormat.java
* ./src/contrib/streaming/src/java/org/apache/hadoop/streaming/AutoInputFormat.java
* ./src/java/org/apache/hadoop/mapred/SequenceFileInputFormat.java
I expect that the NLineInputFormat and the AutoInputFormat will affected by this "large gzip" bug.
So expect that simply fixing the isSplittable documentation would lead to the need to fix *at least* these two classes.
As far as I understand the SequenceFileInputFormat can only be compressed using a "splittable" compression, so the "return true;" from FileInputFormat will work fine there.
Overall I still prefer the clean option of returning the correct value depending on the compression. That would effectively leave the behavior in most use cases unchanged. Yet in those cases where splitting is known to cause problems it would avoid those problems. Thus avoiding major issues like the ones we had and described in HADOOP-6901.
For the SequenceFileInputFormat it may be needed to implement the isSplittable as "return true;"
Effectively the set of changes I propose (in both the old and new API versions of these classes):
1) FileInputFormat . isSplittable gets the implementation as seen in TextInputFormat
2) The isSplittable implementation is removed from KeyValueTextInputFormat , TextInputFormat, CombineFileInputFormat (useless code duplication)
3) The isSplittable implementation "return true" is added to SequenceFileInputFormat. Given the fact that you cannot gzip a sequencefile I expect this to be an optional fix.
> org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable implements unsafe default behaviour that is different from the documented behaviour.
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-2094
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2094
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: task
> Affects Versions: 0.20.1, 0.20.2, 0.21.0
> Reporter: Niels Basjes
>
> When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times.
> A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
> It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;".
> This is a very unsafe default and is in contradiction with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. " . The actual implementation effectively does "Is the given filename splitable? Always true, even if the file is stream compressed using an unsplittable compression codec. "
> For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable in our class that does "return false; "
> Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
> # Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
> # "Force" developers to think about it and make this method abstract.
> # Use a "safe" default (i.e. return false)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAPREDUCE-2094)
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable
implements unsafe default behaviour that is different from the documented
behaviour.
Posted by "Niels Basjes (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAPREDUCE-2094?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12918851#action_12918851 ]
Niels Basjes commented on MAPREDUCE-2094:
-----------------------------------------
I just noticed that the Yahoo Hadoop tutorial "[Module 5: Advanced MapReduce Features |http://developer.yahoo.com/hadoop/tutorial/module5.html]" shows a code example for defining your own [FileInputFormat|http://developer.yahoo.com/hadoop/tutorial/module5.html#fileformat].
The shown example code implements a derivative using FileInputFormat and LineRecordReader without overruling isSplittable ... I expect this tutorial code to lead people into this bug.
Since this bug will only become apparent when using large "non splittable" (gzipped) input files it is also important to notice that almost no one will have a (unit) test that will trip on this bug.
> org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable implements unsafe default behaviour that is different from the documented behaviour.
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-2094
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2094
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: task
> Affects Versions: 0.20.1, 0.20.2, 0.21.0
> Reporter: Niels Basjes
>
> When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times.
> A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
> It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;".
> This is a very unsafe default and is in contradiction with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. " . The actual implementation effectively does "Is the given filename splitable? Always true, even if the file is stream compressed using an unsplittable compression codec. "
> For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable in our class that does "return false; "
> Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
> # Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
> # "Force" developers to think about it and make this method abstract.
> # Use a "safe" default (i.e. return false)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAPREDUCE-2094)
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable
implements unsafe default behaviour that is different from the documented
behaviour.
Posted by "Niels Basjes (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAPREDUCE-2094?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niels Basjes updated MAPREDUCE-2094:
------------------------------------
Issue Type: Bug (was: Improvement)
> org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable implements unsafe default behaviour that is different from the documented behaviour.
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-2094
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2094
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: task
> Affects Versions: 0.20.1, 0.20.2, 0.21.0
> Reporter: Niels Basjes
>
> When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times. A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
> It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;". This is a very unsafe default and is in contradcition with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. "
> For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable inour class that does "return false; "
> Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
> # Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
> # "Force" developers to think about it and make this method (and therfor the entire FileInputFormat class) abstract.
> # Use a "safe" default (i.e. return false)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAPREDUCE-2094)
org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable
implements unsafe default behaviour that is different from the documented
behaviour.
Posted by "Niels Basjes (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAPREDUCE-2094?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niels Basjes updated MAPREDUCE-2094:
------------------------------------
Description:
When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times.
A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;".
This is a very unsafe default and is in contradiction with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. " . The actual implementation effectively does "Is the given filename splitable? Always true, even if the file is stream compressed using an unsplittable compression codec. "
For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable in our class that does "return false; "
Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
# Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
# "Force" developers to think about it and make this method abstract.
# Use a "safe" default (i.e. return false)
was:
When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times. A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;". This is a very unsafe default and is in contradcition with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. "
For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable inour class that does "return false; "
Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
# Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
# "Force" developers to think about it and make this method (and therfor the entire FileInputFormat class) abstract.
# Use a "safe" default (i.e. return false)
> org.apache.hadoop.mapreduce.lib.input.FileInputFormat: isSplitable implements unsafe default behaviour that is different from the documented behaviour.
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-2094
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2094
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: task
> Affects Versions: 0.20.1, 0.20.2, 0.21.0
> Reporter: Niels Basjes
>
> When implementing a custom derivative of FileInputFormat we ran into the effect that a large Gzipped input file would be processed several times.
> A near 1GiB file would be processed around 36 times in its entirety. Thus producing garbage results and taking up a lot more CPU time than needed.
> It took a while to figure out and what we found is that the default implementation of the isSplittable method in [org.apache.hadoop.mapreduce.lib.input.FileInputFormat | http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/input/FileInputFormat.java?view=markup ] is simply "return true;".
> This is a very unsafe default and is in contradiction with the JavaDoc of the method which states: "Is the given filename splitable? Usually, true, but if the file is stream compressed, it will not be. " . The actual implementation effectively does "Is the given filename splitable? Always true, even if the file is stream compressed using an unsplittable compression codec. "
> For our situation (where we always have Gzipped input) we took the easy way out and simply implemented an isSplittable in our class that does "return false; "
> Now there are essentially 3 ways I can think of for fixing this (in order of what I would find preferable):
> # Implement something that looks at the used compression of the file (i.e. do migrate the implementation from TextInputFormat to FileInputFormat). This would make the method do what the JavaDoc describes.
> # "Force" developers to think about it and make this method abstract.
> # Use a "safe" default (i.e. return false)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.