Details
-
Type:
Sub-task
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 0.20.205.0
-
Fix Version/s: 1.1.0
-
Component/s: tasktracker
-
Labels:None
-
Hadoop Flags:Reviewed
-
Target Version/s:
Description
MR-2415 had some tabs and weird indenting and spacing. Also would be more clear if LTC explicitly overrides createLogDir. Let's clean this up.
Activity
Updated patch attached. More cleanup. Also:
- Removes the reference to mapred.local.dir in task-controller.cfg from the cluster setup docs since it's no longer used
- This also removes the call to TaskController#setup in TT#initialize, this is already called by the constructor. For the DefaultTaskController we dont' need to reset the local storate and allocator refs as they haven't changed, and for the LTC we don't need to do the basic perms check on each init.
Show
Eli Collins
added a comment - Updated patch attached. More cleanup. Also:
Removes the reference to mapred.local.dir in task-controller.cfg from the cluster setup docs since it's no longer used
This also removes the call to TaskController#setup in TT#initialize, this is already called by the constructor. For the DefaultTaskController we dont' need to reset the local storate and allocator refs as they haven't changed, and for the LTC we don't need to do the basic perms check on each init.
Looks good. Did you run MR unit tests and the task-controller tests locally?
Show
Todd Lipcon
added a comment - Looks good. Did you run MR unit tests and the task-controller tests locally?
Updated patch attached. Minor update on the last one. I ran the tt test as well as the full suite and ran some jobs on 3 nodes w the the ltc enabled. test-patch results comming.
Show
Eli Collins
added a comment - Updated patch attached. Minor update on the last one. I ran the tt test as well as the full suite and ran some jobs on 3 nodes w the the ltc enabled. test-patch results comming.
[exec]
[exec] -1 overall.
[exec]
[exec] +1 @author. The patch does not contain any @author tags.
[exec]
[exec] -1 tests included. The patch doesn't appear to include any new or modified tests.
[exec] Please justify why no tests are needed for this patch.
[exec]
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec]
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec]
[exec] -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings.
[exec]
The findbugs warning came from a previous change (see them w/ a null patch). Existing tests suffice since this is cleanup.
Show
Eli Collins
added a comment -
[exec]
[exec] -1 overall.
[exec]
[exec] +1 @author. The patch does not contain any @author tags.
[exec]
[exec] -1 tests included. The patch doesn't appear to include any new or modified tests.
[exec] Please justify why no tests are needed for this patch.
[exec]
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec]
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec]
[exec] -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings.
[exec]
The findbugs warning came from a previous change (see them w/ a null patch). Existing tests suffice since this is cleanup.
+1. Looks good.
Show
Todd Lipcon
added a comment - +1. Looks good.
Thanks Todd. I've committed this.
Show
Eli Collins
added a comment - Thanks Todd. I've committed this.
Closed upon release of Hadoop-1.1.0.
Show
Matt Foley
added a comment - Closed upon release of Hadoop-1.1.0.
Patch attached.