hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Krishna Ramachandran (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MAPREDUCE-2020) Use new FileContext APIs for all mapreduce components
Date Tue, 31 Aug 2010 18:31:54 GMT

    [ https://issues.apache.org/jira/browse/MAPREDUCE-2020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12904710#action_12904710
] 

Krishna Ramachandran commented on MAPREDUCE-2020:
-------------------------------------------------

Thanks Greg!

I will include these in the next rev
Krishna

On 8/30/10 6:21 PM, "Greg Roelofs (JIRA)" <jira@apache.org> wrote:



    [ https://issues.apache.org/jira/browse/MAPREDUCE-2020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12904479#action_12904479
]

Greg Roelofs commented on MAPREDUCE-2020:
-----------------------------------------

Looks good so far.  There are some cosmetic issues and some leftover TODO items, but nothing
major and only one or two medium-level items.

general:
 * remove all trailing whitespace from _new_ code (old code is full of it and makes diffs
huge, but no excuse for adding to problem)
 * don't replicate, share/reuse (if possible) - or, if adding new-API variant, deprecate old-API
variant at same time

JobTracker.java:
 * @@ -1575,12 +1584,10 @@
   ** commented-out error message
   ** potential conversion of non-access exception to AccessControlException
 * @@ -1599,11 +1606,11 @@
   ** TODO
 * @@ -2380,7 +2387,7 @@
   ** debug converted to info
 * @@ -3075,8 +3082,9 @@
   ** {{filecontext.mkdir(jobDir, new FsPermission(SYSTEM_DIR_PERMISSION), true);}} - prefer
try/catch block over uncaught exceptions, if for no other reason than to customize error message
 * @@ -4759,6 +4767,19 @@
   ** TODO

UserLogCleaner.java:
 * @@ -59,7 +60,7 @@
   ** wrap at 80, and _don't_ do so at class-member dot if don't have to (ugh!) - member reference
is highest-precedence operator of anything in Java/C/C++, which means it's the _least_ preferred
break-point per Apache code conventions

BAD:
{noformat}
     logAsyncDisk = new MRAsyncDiskService(FileContext.getLocalFSFileContext(conf), TaskLog
         .getUserLogDir().toString());
{noformat}

FAIR:
{noformat}
     logAsyncDisk = new MRAsyncDiskService(
         FileContext.getLocalFSFileContext(conf),
         TaskLog.getUserLogDir().toString());
{noformat}

GOOD:
{noformat}
     logAsyncDisk =
         new MRAsyncDiskService(FileContext.getLocalFSFileContext(conf),
                                TaskLog.getUserLogDir().toString());
{noformat}

ALSO GOOD:
{noformat}
     logAsyncDisk =
         new MRAsyncDiskService(FileContext.getLocalFSFileContext(conf),
         TaskLog.getUserLogDir().toString());
{noformat}

JobHistory.java:
 * @@ -180,6 +184,40 @@
   ** in general, avoid duplicating code:  either share/reuse or, if providing new-API version
(as here), deprecate old one when new one added
   ** blank line required between methods
 * @@ -190,6 +228,15 @@
   ** blank line required between methods

MRAsyncDiskService.java:
 * @@ -56,9 +60,13 @@
   ** {{FsPermission.createImmutable((short) 0700); // rwx------}} - see Localizer.PermissionsHandler.sevenZeroZero:
 use?
 * @@ -88,7 +96,71 @@
   ** deprecate old ctor
   ** {{* be absolte paths,}} - fix spelling (both cases)
   ** {{this.volumes = new String[nonCanonicalVols.length];}} - ugh, lose the "this." for
everything that doesn't need it (i.e., everything that's not a duplicate name of a method
argument)
 * @@ -98,10 +170,12 @@
   ** wrap per coding conventions
 * @@ -114,13 +188,14 @@
   ** commented-out line
 * @@ -246,18 +321,9 @@
   ** not equivalent semantics:  if throw due to perms or whatever, will be converted to FileNotFoundException
 * @@ -296,10 +362,10 @@
   ** wrap per coding conventions

MiniMRCluster.java:
 * @@ -375,7 +377,7 @@
   ** got rid of one trailing space but left the other one??
 * @@ -388,6 +390,18 @@
   ** don't replicate, reuse!  in this case, namenode-arg configureJobConf() can call this
new variant and just add extra namenode config call first (or last, whatever)
   ** nuke new trailing spaces

TestJobTrackerStart.java:
 * @@ -29,7 +30,9 @@
   ** what is purpose of dual call to configureJobConf() ?



--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.




> Use new FileContext APIs for all mapreduce components 
> ------------------------------------------------------
>
>                 Key: MAPREDUCE-2020
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-2020
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>    Affects Versions: 0.22.0
>            Reporter: Krishna Ramachandran
>            Assignee: Krishna Ramachandran
>         Attachments: mapred-2020-1.patch, mapred-2020.patch
>
>
> Migrate mapreduce components to using improved FileContext APIs implemented in
> HADOOP-4952 and 
> HADOOP-6223

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message