hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alejandro Abdelnur (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MAPREDUCE-4511) Add IFile readahead
Date Wed, 15 Aug 2012 20:38:38 GMT

    [ 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

        

Mime
View raw message