[
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() ?
> 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.
