[
https://issues.apache.org/jira/browse/MAPREDUCE-1854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877730#action_12877730
]
Konstantin Boudnik commented on MAPREDUCE-1854:
-----------------------------------------------
Some comments:
- changing visibility from 'package private' to 'public' for testing purpose
isn't advisable. Consider injecting a getter with public access
- same here:
{noformat}
- static class TaskTrackerHealthStatus implements Writable {
+ public static class TaskTrackerHealthStatus implements Writable {
{noformat}
- AbstractTestCase sounds like a utility methods' class to me. Unless a common
parent is really required from some design perspective I won't recommend clog
class hierarchy with unnecessary inheritance.
- using hard-coded paths like {{/tmp/}} restricts tests applicability. Would it
be better to use configurable location, i.e. mapred data directory or something?
- JUnit v3 imports +import junit.framework.Assert
- using {{StringBuffer}} to append a couple of tokens and convert the result to
a new {{String}} seems excessive. Why not to use {{String}}?
{noformat}
+ StringBuffer localFile = new StringBuffer();
+ localFile.append(scriptDir).append(File.separator).append(scriptName);
+ cmdArgs.add(localFile.toString());
{noformat}
- remove commented out lines of code which seem like a debugging leftovers
- try to generate patch with '--no-prefix' to avoid extra prefixes in the file
paths
- this JavaDoc seems incomplete
{noformat}
+ * This directly calls the JobTracker public with no modifications
+ * @param trackerID uniquely indentifies the task tracker
+ * @return
+ * @throws IOException is thrown in case of RPC error
{noformat}
- there some unused imports
- are changes in AbstractDaemonCluster.java related to this patch?
- looks like the change in DaemonProtocolAspect.aj is unrelated to this patch,
isn't it? ;)
- same about ClusterProcessManager, HadoopDaemonRemoteCluster, and RemoteProcess
As you're clearly using Git (this isn't SVN - it is a great SCM system!) for
the development work try to have a separate branch for any JIRA you're working
on. But this you'll avoid any mess and accidental inclusion of irrelevant
files.
- writing new script every time we need to do some sort of ssh command looks
bad. I have a couple alternative thoughts:
** using pure Java ssh client like JSch
** creating a wrapper around ssh command using Shell class (in case the above
is impossible because of license issues or something)
I think this is enough for the starter :)
> [herriot] Automate health script system test
> --------------------------------------------
>
> Key: MAPREDUCE-1854
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-1854
> Project: Hadoop Map/Reduce
> Issue Type: New Feature
> Components: test
> Environment: Herriot framework
> Reporter: Balaji Rajagopalan
> Assignee: Balaji Rajagopalan
> Attachments: health_script_5.txt
>
> Original Estimate: 120h
> Remaining Estimate: 120h
>
> 1. There are three scenarios, first is induce a error from health script,
> verify that task tracker is blacklisted.
> 2. Make the health script timeout and verify the task tracker is blacklisted.
> 3. Make an error in the health script path and make sure the task tracker
> stays healthy.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
