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

Hong Tang commented on MAPREDUCE-1526:
--------------------------------------

Mostly good. Detailed comments:

Statistics.java:
- Use job seq id instead of ORIGNAME+random number as key to track the job 
stats.
- JobStats.setNoOfMaps should not be public. Better if you have a JobStats 
constructor that takes two parameters, and eliminate the need of the setters or 
the empty constructor.
- You should avoid returning the whole jobMaps in ClusterStats, it became worse 
when you save the returned map in the member field of StressJobFactory. It 
seems sufficient to avoid them if you have the following methods in 
ClusterStatus
-- int getNumRunningJobs()
-- Collection<JobStats> getRunningJobStats()
- The following code looks wrong. Why is an empty JobStats object created? 
Shouldn't you search from internal JobStats map, and only call listeners if an 
instance of JobStats is found? Also, the if statement seems redundant.
{noformat}
      try {
        //Job is completed notify all the listeners.
        if (jobStatListeners.size() > 0) {
          for (StatListener<JobStats> l : jobStatListeners) {
            JobStats stats = new JobStats();
            l.update(stats);
          }
        }
{noformat}

StressJobFactory.java:
- Why do we need to add "volatile" to loadStatus?
- I think the following statement should be Log.debug() instead of Log.info() 
(and be protected by a check of LOG.isDebugEnabled()):
{noformat}
-    if (LOG.isDebugEnabled()) {
-      LOG.info(
+    LOG.info(
         System.currentTimeMillis() + " Overloaded is " + Boolean.toString(
           overloaded) + " incompleteMapTasks " + relOp + " " +
           OVERLAOD_MAPTASK_MAPSLOT_RATIO + "*mapSlotCapacity" + "(" +
           incompleteMapTasks + " " + relOp + " " +
           OVERLAOD_MAPTASK_MAPSLOT_RATIO + "*" +
           clusterStatus.getMaxMapTasks() + ")");
-    }
+
{noformat}

Misc:
- Some indentation problem in JobSubmitter.java

> Cache the job related information while submitting the job , this would avoid 
> many RPC calls to JobTracker.
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-1526
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-1526
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: contrib/gridmix
>            Reporter: rahul k singh
>         Attachments: 1526-yahadoop-20-101.patch
>
>


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

Reply via email to