[jira] [Commented] (MAPREDUCE-4275) Plugable process tree

["Bikas Saha (JIRA)" <ji...@apache.org>]

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

Bikas Saha commented on MAPREDUCE-4275:
---------------------------------------

Thanks for the patch and sorry for not getting to it earlier.
In future, could you please use the --no-prefix tag so that the diff does not have a/hadoop-mapreduce..../.. and is easier to apply.

1) In general there are a bunch of extra spaces all over the place that would be good to remove.
2) The comment looks like it has a typo in ResourceCalculatorPlugin.java
{code}
  /**
   * Get the ResourceCalculatorProcessTree configure it. This method will try
   * and return a ResourceCalculatorProcessTree available for this system.
{code}
3. ContainerMonitorImpl.java
There is an indentation issue in the first if. The second if statement is leaking Linux semantics into the code by trying to get process tree for pid 1. IMO, the fact that resourceCalculatorPlugin exists should be enough to ensure that underlying interfaces have been correctly implemented.
{code}
	if (resourceCalculatorPlugin == null) {
	        LOG.info("ResourceCalculatorPlugin is unavaiable on this system. "
	            + this.getClass().getName() + " is disabled.");
	        return false;
	}

    if (resourceCalculatorPlugin.getResourceCalculatorProcessTree("1") == null ) {
      LOG.info("ProcessTree implementation is missing on this system. "
          + this.getClass().getName() + " is disabled.");
      return false;
    }
{code}
After removal of the following code can ContainerExecutor.isSetSidAvailable be removed too? Any more cleanup possible?
{code}
                ProcfsBasedProcessTree pt =
                    new ProcfsBasedProcessTree(pId,
                        ContainerExecutor.isSetsidAvailable);
{code}

Other than these, it look good.
                
> Plugable process tree
> ---------------------
>
>                 Key: MAPREDUCE-4275
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-4275
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: nodemanager
>    Affects Versions: 3.0.0
>         Environment: FreeBSD 64 bit
>            Reporter: Radim Kolar
>         Attachments: plugable-pstree-1.txt, plugable-pstree.txt
>
>
> Trunk version of Pluggable process tree. Work based on MAPREDUCE-4204

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira