[
https://issues.apache.org/jira/browse/MAPREDUCE-4511?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13435502#comment-13435502
]
Alejandro Abdelnur commented on MAPREDUCE-4511:
-----------------------------------------------
Looks good, some minor improvements:
*IFileInputStream <init>*:
{code}
if (conf != null){
readahead = conf.getBoolean("mapreduce.ifile.readahead", true);
readaheadLength = conf.getInt("mapreduce.ifile.readahead.bytes",
4 * 1024 * 1024);
} else {
readahead = true;
readaheadLength = 4 * 1024 * 1024;
}
{code}
How about?
{code}
conf = (conf !=null) ? conf : new Configuration();
readahead = conf.getBoolean("mapreduce.ifile.readahead", true);
readaheadLength = conf.getInt("mapreduce.ifile.readahead.bytes",
{code}
Instead using literals, we should have constants keys and default in
MRConfig.java and have the defaults in mapred-default.xml.
*IFileInputStream.getFileDescriptorIfAvail*: Use a local var for FileDescriptor
initialized to null, set it in the IF/ELIF and have a single return at the end
of the method.
*IFileInputSTream.doReadahead()*: the first condition should be 'raPool !=
null', if there is not readahead pool avail, no point in checking if the job is
configured to do it and if there is a FD
*testcase* just tests there are no regressions, right? I think it is OK, would
not be that easy to test it anyway.
> Add IFile readahead
> -------------------
>
> Key: MAPREDUCE-4511
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-4511
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: mrv1, mrv2, performance
> Reporter: Ahmed Radwan
> Assignee: Ahmed Radwan
> Attachments: MAPREDUCE-4511_branch1.patch,
> MAPREDUCE-4511_branch-1_rev2.patch, MAPREDUCE-4511_branch-1_rev3.patch,
> MAPREDUCE-4511_branch-1_rev4.patch, MAPREDUCE-4511_branch-1_rev5.patch,
> MAPREDUCE-4511_trunk.patch, MAPREDUCE-4511_trunk_rev2.patch,
> MAPREDUCE-4511_trunk_rev3.patch, MAPREDUCE-4511_trunk_rev4.patch,
> MAPREDUCE-4511_trunk_rev5.patch
>
>
> This ticket is to add IFile readahead as part of HADOOP-7714.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
