Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-178

Fix custom ProcessTree instance creation

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 0.23.5, 3.0.0-alpha1
    • Fix Version/s: 2.0.3-alpha, 0.23.5
    • Component/s: None
    • Labels:
      None

      Description

      1. In current pluggable resourcecalculatorprocesstree is not passed root process id to custom implementation making it unusable.

      2. pstree do not extend Configured as it should

      Added constructor with pid argument with testsuite. Also added test that pstree is correctly configured.

      1. pstree-instance.txt
        6 kB
        Radim Kolar
      2. pstree-instance2.txt
        6 kB
        Radim Kolar

        Activity

        Hide
        hudson Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1236 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1236/)
        YARN-178. Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698)

        Result = FAILURE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Show
        hudson Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1236 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1236/ ) YARN-178 . Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Hide
        hudson Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1206 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1206/)
        YARN-178. Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Show
        hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1206 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1206/ ) YARN-178 . Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Hide
        hudson Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #415 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/415/)
        YARN-178. Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401701)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401701
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java
        • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Show
        hudson Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #415 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/415/ ) YARN-178 . Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401701) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401701 Files : /hadoop/common/branches/branch-0.23/hadoop-yarn-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Hide
        hudson Hudson added a comment -

        Integrated in Hadoop-Yarn-trunk #16 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/16/)
        YARN-178. Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Show
        hudson Hudson added a comment - Integrated in Hadoop-Yarn-trunk #16 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/16/ ) YARN-178 . Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Hide
        hudson Hudson added a comment -

        Integrated in Hadoop-trunk-Commit #2921 (See https://builds.apache.org/job/Hadoop-trunk-Commit/2921/)
        YARN-178. Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698
        Files :

        • /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java
        • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Show
        hudson Hudson added a comment - Integrated in Hadoop-trunk-Commit #2921 (See https://builds.apache.org/job/Hadoop-trunk-Commit/2921/ ) YARN-178 . Fix custom ProcessTree instance creation (Radim Kolar via bobby) (Revision 1401698) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1401698 Files : /hadoop/common/trunk/hadoop-yarn-project/CHANGES.txt /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/TestResourceCalculatorProcessTree.java
        Hide
        revans2 Robert Joseph Evans added a comment -

        Thanks Radim,

        I put this into trunk, branch-2, and branch-0.23

        Show
        revans2 Robert Joseph Evans added a comment - Thanks Radim, I put this into trunk, branch-2, and branch-0.23
        Hide
        revans2 Robert Joseph Evans added a comment -

        Sorry I got distracted by other things. Checking this in now.

        Show
        revans2 Robert Joseph Evans added a comment - Sorry I got distracted by other things. Checking this in now.
        Hide
        bikassaha Bikas Saha added a comment -

        Sure. Please go ahead.

        Show
        bikassaha Bikas Saha added a comment - Sure. Please go ahead.
        Hide
        hadoopqa Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12550313/pstree-instance2.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/122//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/122//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12550313/pstree-instance2.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/122//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/122//console This message is automatically generated.
        Hide
        revans2 Robert Joseph Evans added a comment -

        Makes since. Bikas, unless you have a strong objection I will check this in this afternoon.

        Show
        revans2 Robert Joseph Evans added a comment - Makes since. Bikas, unless you have a strong objection I will check this in this afternoon.
        Hide
        hsn Radim Kolar added a comment -

        let patch be as it is. current code is using constructor for linux variant.

        Show
        hsn Radim Kolar added a comment - let patch be as it is. current code is using constructor for linux variant.
        Hide
        revans2 Robert Joseph Evans added a comment -

        I am fine with either setting it through a contructor or through a setter method. I agree with Radim that the constructor makes it more obvious that it is not supposed to change, but other places tend to use setter methods instead, like with Configuration which is also not supposed to change.

        Show
        revans2 Robert Joseph Evans added a comment - I am fine with either setting it through a contructor or through a setter method. I agree with Radim that the constructor makes it more obvious that it is not supposed to change, but other places tend to use setter methods instead, like with Configuration which is also not supposed to change.
        Hide
        hsn Radim Kolar added a comment -

        you need constructor which is shared by all subclasses. Alternatively, it could be method instead of constructor but because root process pid do not change then constructor is better.

        Show
        hsn Radim Kolar added a comment - you need constructor which is shared by all subclasses. Alternatively, it could be method instead of constructor but because root process pid do not change then constructor is better.
        Hide
        bikassaha Bikas Saha added a comment -

        Then why add it to the constructor of the abstract base class?

        Show
        bikassaha Bikas Saha added a comment - Then why add it to the constructor of the abstract base class?
        Hide
        hsn Radim Kolar added a comment -

        you need to know root pid of process tree to construct it. base class is not using it because its abstract

        Show
        hsn Radim Kolar added a comment - you need to know root pid of process tree to construct it. base class is not using it because its abstract
        Hide
        bikassaha Bikas Saha added a comment -

        The change looks reasonable. What is not clear is why this is being made? Nothing seems to use the pid in the base class nor is there any other code that uses it.

        Show
        bikassaha Bikas Saha added a comment - The change looks reasonable. What is not clear is why this is being made? Nothing seems to use the pid in the base class nor is there any other code that uses it.
        Hide
        hsn Radim Kolar added a comment -

        i need this to be merged into branch-0.23 as well

        Show
        hsn Radim Kolar added a comment - i need this to be merged into branch-0.23 as well
        Hide
        hsn Radim Kolar added a comment -

        reformated tabs to spaces, rethrow an exception during instance creation

        Show
        hsn Radim Kolar added a comment - reformated tabs to spaces, rethrow an exception during instance creation
        Hide
        revans2 Robert Joseph Evans added a comment -

        Oh I didn't notice this before, but the test file has '\t' tab characters in it. Could you reformat it to remove the tabs and have it correspond to the two spaces per indentation?

        Show
        revans2 Robert Joseph Evans added a comment - Oh I didn't notice this before, but the test file has '\t' tab characters in it. Could you reformat it to remove the tabs and have it correspond to the two spaces per indentation?
        Hide
        hadoopqa Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12550307/pstree-instance.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/110//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/110//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12550307/pstree-instance.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/110//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/110//console This message is automatically generated.
        Hide
        revans2 Robert Joseph Evans added a comment -

        I have one small issue with the patch. In RefelctionUtils.newInstance when it catches an exception it wraps it in a RuntimeException and then throws it, this patch will instead output a small log message. I would prefer to see it do the same thing as reflectionUtils for compatibility reasons. Other then that the patch looks good to me.

        Show
        revans2 Robert Joseph Evans added a comment - I have one small issue with the patch. In RefelctionUtils.newInstance when it catches an exception it wraps it in a RuntimeException and then throws it, this patch will instead output a small log message. I would prefer to see it do the same thing as reflectionUtils for compatibility reasons. Other then that the patch looks good to me.

          People

          • Assignee:
            hsn Radim Kolar
            Reporter:
            hsn Radim Kolar
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development