Hadoop YARN
  1. Hadoop YARN
  2. YARN-443

allow OS scheduling priority of NM to be different than the containers it launches

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.3-alpha, 0.23.6
    • Fix Version/s: 3.0.0, 0.23.7, 2.0.4-alpha
    • Component/s: nodemanager
    • Labels:
      None

      Description

      It would be nice if we could have the nodemanager run at a different OS scheduling priority than the containers so that you can still communicate with the nodemanager if the containers out of control.

      On linux we could launch the nodemanager at a higher priority, but then all the containers it launches would also be at that higher priority, so we need a way for the container executor to launch them at a lower priority.

      I'm not sure how this applies to windows if at all.

      1. YARN-443.patch
        5 kB
        Thomas Graves
      2. YARN-443.patch
        6 kB
        Thomas Graves
      3. YARN-443-branch-0.23.patch
        6 kB
        Thomas Graves
      4. YARN-443-branch-0.23.patch
        7 kB
        Thomas Graves
      5. YARN-443-branch-2.patch
        7 kB
        Thomas Graves
      6. YARN-443.patch
        8 kB
        Thomas Graves
      7. YARN-443.patch
        14 kB
        Thomas Graves
      8. YARN-443-branch-0.23.patch
        13 kB
        Thomas Graves
      9. YARN-443-branch-2.patch
        10 kB
        Thomas Graves
      10. YARN-443.patch
        15 kB
        Thomas Graves
      11. YARN-443-branch-0.23.patch
        13 kB
        Thomas Graves
      12. YARN-443-branch-2.patch
        11 kB
        Thomas Graves
      13. YARN-443.patch
        15 kB
        Thomas Graves
      14. YARN-443.patch
        15 kB
        Thomas Graves

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1367 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1367/)
          YARN-443. allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411)

          Result = SUCCESS
          tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411
          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/conf/YarnConfiguration.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1367 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1367/ ) YARN-443 . allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411 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/conf/YarnConfiguration.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1339 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1339/)
          YARN-443. allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411)

          Result = SUCCESS
          tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411
          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/conf/YarnConfiguration.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1339 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1339/ ) YARN-443 . allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411 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/conf/YarnConfiguration.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #548 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/548/)
          YARN-443. allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454412)

          Result = SUCCESS
          tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454412
          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/conf/YarnConfiguration.java
          • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
          • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
          • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
          • /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #548 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/548/ ) YARN-443 . allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454412) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454412 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/conf/YarnConfiguration.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java /hadoop/common/branches/branch-0.23/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #150 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/150/)
          YARN-443. allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411)

          Result = SUCCESS
          tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411
          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/conf/YarnConfiguration.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #150 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/150/ ) YARN-443 . allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411 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/conf/YarnConfiguration.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Hide
          Thomas Graves added a comment -

          Note that this jira added a new config, which defaults to not be set.

          <name>yarn.nodemanager.container-executor.os.sched.priority.adjustment</name>

          Show
          Thomas Graves added a comment - Note that this jira added a new config, which defaults to not be set. <name>yarn.nodemanager.container-executor.os.sched.priority.adjustment</name>
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3440 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3440/)
          YARN-443. allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411)

          Result = SUCCESS
          tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411
          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/conf/YarnConfiguration.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java
          • /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3440 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3440/ ) YARN-443 . allow OS scheduling priority of NM to be different than the containers it launches (tgraves) (Revision 1454411) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1454411 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/conf/YarnConfiguration.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestContainerExecutor.java /hadoop/common/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java
          Hide
          Thomas Graves added a comment -

          I intended to leave that log message in there because if anything comes back on stderr from try running the container, it goes into the exception but then its never logged. I'll actually go ahead and remove that and file a follow up jira as ideally any message would go into the diagnostics that get returned to the user also.

          I'll fix the typo for than and commit.

          Show
          Thomas Graves added a comment - I intended to leave that log message in there because if anything comes back on stderr from try running the container, it goes into the exception but then its never logged. I'll actually go ahead and remove that and file a follow up jira as ideally any message would go into the diagnostics that get returned to the user also. I'll fix the typo for than and commit.
          Hide
          Bikas Saha added a comment -

          The LCE changes look good too. Dont think theres anything wrong there. Mostly minor changes.

          Show
          Bikas Saha added a comment - The LCE changes look good too. Dont think theres anything wrong there. Mostly minor changes.
          Hide
          Bikas Saha added a comment -

          +1 looks good to me subject to above LCE caveat

          Minor nits
          Did you intend to keep this log message?

          +      logOutput(e.getMessage());
          

          You mean "than"

          +   * On Linux, higher values mean run the containers at a less 
          +   * favorable priority then the NM. 
          
          Show
          Bikas Saha added a comment - +1 looks good to me subject to above LCE caveat Minor nits Did you intend to keep this log message? + logOutput(e.getMessage()); You mean "than" + * On Linux, higher values mean run the containers at a less + * favorable priority then the NM.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12572619/YARN-443.patch
          against trunk revision .

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

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

          +1 tests included appear to have a timeout.

          +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12572619/YARN-443.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 tests included appear to have a timeout. +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/479//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/479//console This message is automatically generated.
          Hide
          Thomas Graves added a comment -

          thanks Bikas, I'll remove that if and upload a new trunk patch shortly.

          Show
          Thomas Graves added a comment - thanks Bikas, I'll remove that if and upload a new trunk patch shortly.
          Hide
          Bikas Saha added a comment -

          I am not that familiar with the LCE code but it looks good overall. There seems to be some command line generation code duplication in the LCE code but I dont think its for this jira.
          I dont think we need an if(Shell.Windows) in the test for LCE.java, right?

          Show
          Bikas Saha added a comment - I am not that familiar with the LCE code but it looks good overall. There seems to be some command line generation code duplication in the LCE code but I dont think its for this jira. I dont think we need an if(Shell.Windows) in the test for LCE.java, right?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12572602/YARN-443.patch
          against trunk revision .

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

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

          +1 tests included appear to have a timeout.

          +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12572602/YARN-443.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 tests included appear to have a timeout. +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/478//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/478//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12572597/YARN-443.patch
          against trunk revision .

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

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

          -1 one of tests included doesn't have a timeout.

          +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12572597/YARN-443.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. -1 one of tests included doesn't have a timeout. +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/477//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/477//console This message is automatically generated.
          Hide
          Thomas Graves added a comment -

          added timeouts to tests

          Show
          Thomas Graves added a comment - added timeouts to tests
          Hide
          Thomas Graves added a comment -

          need to add timeouts to tests

          Show
          Thomas Graves added a comment - need to add timeouts to tests
          Hide
          Thomas Graves added a comment -

          All patches updated with changes from review and all include tests.

          Show
          Thomas Graves added a comment - All patches updated with changes from review and all include tests.
          Hide
          Thomas Graves added a comment -

          Thanks for the review Bikas, I'll remove the bits for modifying the windows stuff and update based on your comments. I agree, I wish it was more common but didn't want to take that on here.

          I can update the comment to say higher values mean less favorable priority on Linux. We could make that the standard if windows wants to handle converting that. I changed the name to be the adjustment, one because it fit as parameter to nice but also in an attempt to make it less platform specific. I was hoping it would allows us to say things like positive numbers in adjustment on any platform make the container run less favorable to NM and then each platform could interpret that. We don't have to state that now though.

          It doesn't necessarily have to be +ve either. If you nodemanager has root priveleges it can be negative on linux although I don't know of anyone who would do that. I also didn't want to restrict that for other platforms.

          Show
          Thomas Graves added a comment - Thanks for the review Bikas, I'll remove the bits for modifying the windows stuff and update based on your comments. I agree, I wish it was more common but didn't want to take that on here. I can update the comment to say higher values mean less favorable priority on Linux. We could make that the standard if windows wants to handle converting that. I changed the name to be the adjustment, one because it fit as parameter to nice but also in an attempt to make it less platform specific. I was hoping it would allows us to say things like positive numbers in adjustment on any platform make the container run less favorable to NM and then each platform could interpret that. We don't have to state that now though. It doesn't necessarily have to be +ve either. If you nodemanager has root priveleges it can be negative on linux although I don't know of anyone who would do that. I also didn't want to restrict that for other platforms.
          Hide
          Bikas Saha added a comment -

          winutils will not support nice. and windows support does not use cygwin. infact, it removes cygwin. so the above patch will likely cause all tests to fail on Windows
          Why dont we set a local variable with the result of the scheduling configuration? And use that in the non-Windows part of the command line generation? Later we can use that value for the windows part when it gets supported. YARN-456.

          Minor nit, how about a default value in config instead of the hard coded 0 for priority?

          Also if the comments describing the config could be made easier for someone implementing YARN-456 or using the feature. something like. The value must be a +ve integer and higher values mean lower priority? Then the current comments could explain how the value is used in linux.

          I wish linuxcontainerexecutor.java did not need to replicate almost all the changes made in defaultcontainerexecutor

          Show
          Bikas Saha added a comment - winutils will not support nice. and windows support does not use cygwin. infact, it removes cygwin. so the above patch will likely cause all tests to fail on Windows Why dont we set a local variable with the result of the scheduling configuration? And use that in the non-Windows part of the command line generation? Later we can use that value for the windows part when it gets supported. YARN-456 . Minor nit, how about a default value in config instead of the hard coded 0 for priority? Also if the comments describing the config could be made easier for someone implementing YARN-456 or using the feature. something like. The value must be a +ve integer and higher values mean lower priority? Then the current comments could explain how the value is used in linux. I wish linuxcontainerexecutor.java did not need to replicate almost all the changes made in defaultcontainerexecutor
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12572561/YARN-443.patch
          against trunk revision .

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

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

          -1 one of tests included doesn't have a timeout.

          +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12572561/YARN-443.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. -1 one of tests included doesn't have a timeout. +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/476//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/476//console This message is automatically generated.
          Hide
          Thomas Graves added a comment -

          added a couple of unit tests. Still need to update the branch-2 and branch-0.23 patches to include the tests.

          Show
          Thomas Graves added a comment - added a couple of unit tests. Still need to update the branch-2 and branch-0.23 patches to include the tests.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12572548/YARN-443.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12572548/YARN-443.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/475//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/475//console This message is automatically generated.
          Hide
          Thomas Graves added a comment -

          trunk patch. Note that I have nice -n applying to the winutils.exe call to, but unfortunately haven't tested it as I don't have a windows box.

          Bikas if you have cycles to test I would appreciate it, if not I can remove the windows part and file a follow on jira.

          Show
          Thomas Graves added a comment - trunk patch. Note that I have nice -n applying to the winutils.exe call to, but unfortunately haven't tested it as I don't have a windows box. Bikas if you have cycles to test I would appreciate it, if not I can remove the windows part and file a follow on jira.
          Hide
          Thomas Graves added a comment -

          patches for branch-2 and branch-0.23.

          Show
          Thomas Graves added a comment - patches for branch-2 and branch-0.23.
          Hide
          Thomas Graves added a comment -

          I decided to go with the nice method as it should apply across all the platforms and I don't see setting it to a negative value that useful. If its really needed you can run your nodemanager with root permissions.

          If the windows port isn't using cygwin or you think it will cause problems let me know.

          The config in the patch is named: container-executor.os.sched.priority.adjustment

          It basically does a nice -n using the config before launching the container.

          Attaching the branch-0.23 patch which I've tested. I need to create the trunk version with the windows stuff included.

          Show
          Thomas Graves added a comment - I decided to go with the nice method as it should apply across all the platforms and I don't see setting it to a negative value that useful. If its really needed you can run your nodemanager with root permissions. If the windows port isn't using cygwin or you think it will cause problems let me know. The config in the patch is named: container-executor.os.sched.priority.adjustment It basically does a nice -n using the config before launching the container. Attaching the branch-0.23 patch which I've tested. I need to create the trunk version with the windows stuff included.
          Hide
          Thomas Graves added a comment -

          after thinking about it some more the issue with using nice means the NM needs to have sudo access on linux to do negatives values (more favorable priorities) and I was hoping to leave that open as an option. I'll go investigate other possibilities.

          Show
          Thomas Graves added a comment - after thinking about it some more the issue with using nice means the NM needs to have sudo access on linux to do negatives values (more favorable priorities) and I was hoping to leave that open as an option. I'll go investigate other possibilities.
          Hide
          Thomas Graves added a comment -

          The container-executor generally takes in params on command line. One option though initialize_app (which is basically localize) already takes a variable number of arguments though. So that would require me to make the priority a mandatory parameter and then I would have to come up with a special value for if its not set.

          Yeah I'm not a fan of env variables either so would rather not do that.

          It looks like nice should work on cygwin also so I think I can just use that and append it to the command before calling, any objections? For example DefaultContainerExecutor command could become " nice -n <priority> bash WRAPPER_LAUNCH_SCRIPT.....

          The sched.priority was meant to stand for os scheduling priority. I can change it to whatever people agree on would make the most sense.

          Show
          Thomas Graves added a comment - The container-executor generally takes in params on command line. One option though initialize_app (which is basically localize) already takes a variable number of arguments though. So that would require me to make the priority a mandatory parameter and then I would have to come up with a special value for if its not set. Yeah I'm not a fan of env variables either so would rather not do that. It looks like nice should work on cygwin also so I think I can just use that and append it to the command before calling, any objections? For example DefaultContainerExecutor command could become " nice -n <priority> bash WRAPPER_LAUNCH_SCRIPT..... The sched.priority was meant to stand for os scheduling priority. I can change it to whatever people agree on would make the most sense.
          Hide
          Bikas Saha added a comment -

          How about not having "sched" in the param name? We are launching containers at different priorities as opposed to scheduling. Or did you mean OS scheduling priority?

          Show
          Bikas Saha added a comment - How about not having "sched" in the param name? We are launching containers at different priorities as opposed to scheduling. Or did you mean OS scheduling priority?
          Hide
          Bikas Saha added a comment -

          The native binary on windows is called winutils,exe. It does a lot more than container launch (eg file systems operations). Though it does not yet do everything that linux-executor does (cgroups/security).
          How are other configurable parameters passed onto the linux-executor? Though I am not a fan of env vars, we could use env vars as a method to pass config parameters to different executors?

          Show
          Bikas Saha added a comment - The native binary on windows is called winutils,exe. It does a lot more than container launch (eg file systems operations). Though it does not yet do everything that linux-executor does (cgroups/security). How are other configurable parameters passed onto the linux-executor? Though I am not a fan of env vars, we could use env vars as a method to pass config parameters to different executors?
          Hide
          Thomas Graves added a comment -

          complete patch where code is in container-executor.c. Attaching here since it was done for reference.

          Show
          Thomas Graves added a comment - complete patch where code is in container-executor.c. Attaching here since it was done for reference.
          Hide
          Thomas Graves added a comment -

          Bikas, where is the windows container executor? I looked in the branch-trunk-win but didn't see it.

          I'm ok with moving it up to yarn-site. I see some benefit for cross platforms, but I think it also makes it more complex. I'll make a config something like yarn.nodemanager.container-executor.sched.priority of type int. I'm going to change it to see if I can call nice from the java containerexecutor code as it builds up the command rather then inside the container-executor.c code.

          Show
          Thomas Graves added a comment - Bikas, where is the windows container executor? I looked in the branch-trunk-win but didn't see it. I'm ok with moving it up to yarn-site. I see some benefit for cross platforms, but I think it also makes it more complex. I'll make a config something like yarn.nodemanager.container-executor.sched.priority of type int. I'm going to change it to see if I can call nice from the java containerexecutor code as it builds up the command rather then inside the container-executor.c code.
          Hide
          Bikas Saha added a comment -

          I like Arun's suggestion of making this process priority choice explicit in yarn-site. Unless this gets exposed in the configuration/api/java-code it will be non-transparent to cross platform compatibility.

          Show
          Bikas Saha added a comment - I like Arun's suggestion of making this process priority choice explicit in yarn-site. Unless this gets exposed in the configuration/api/java-code it will be non-transparent to cross platform compatibility.
          Hide
          Thomas Graves added a comment -

          Note the patch attached isn't quite complete either. I want to do it for both initializing apps and launching containers. I think the delete files and signal container can stay at the same priority as NM.

          Show
          Thomas Graves added a comment - Note the patch attached isn't quite complete either. I want to do it for both initializing apps and launching containers. I think the delete files and signal container can stay at the same priority as NM.
          Hide
          Thomas Graves added a comment -

          Can you elaborate on your proposal. Are you saying use a yarn config and then pass an argument into the container executor or do something actually higher up like call nice when calling the container-executor? What do you propose for the DefaultContainerExecutor since it could run across multiple platforms and all it does is call a bash script?

          I figured with it in the container-executor itself each one could more easily decide if it applies and how exactly to implement it. I also like the fact that I can change it without rebooting the NM.

          Show
          Thomas Graves added a comment - Can you elaborate on your proposal. Are you saying use a yarn config and then pass an argument into the container executor or do something actually higher up like call nice when calling the container-executor? What do you propose for the DefaultContainerExecutor since it could run across multiple platforms and all it does is call a bash script? I figured with it in the container-executor itself each one could more easily decide if it applies and how exactly to implement it. I also like the fact that I can change it without rebooting the NM.
          Hide
          Arun C Murthy added a comment -

          Thomas, wouldn't it be better to come from yarn-site.xml so that it's more visible to admins/users who are configuring clusters? This way we wudn't need to set this in container-executor.cfg separately for the OS...

          Show
          Arun C Murthy added a comment - Thomas, wouldn't it be better to come from yarn-site.xml so that it's more visible to admins/users who are configuring clusters? This way we wudn't need to set this in container-executor.cfg separately for the OS...
          Hide
          Thomas Graves added a comment -

          I chose to do this in the container-executor itself and the config goes into the container-executor.cfg. This made the change more straight forward in that we didn't have to change the args passed to container-executor. Having the config in the container-executor.cfg also allows you to change it without restarting the NM.

          new config is: process.sched.priority. It can be set to anything that setpriority takes, including negative values since container-executor has root permissions. If the config is left out it retains the same behavior of having priority 0.

          Show
          Thomas Graves added a comment - I chose to do this in the container-executor itself and the config goes into the container-executor.cfg. This made the change more straight forward in that we didn't have to change the args passed to container-executor. Having the config in the container-executor.cfg also allows you to change it without restarting the NM. new config is: process.sched.priority. It can be set to anything that setpriority takes, including negative values since container-executor has root permissions. If the config is left out it retains the same behavior of having priority 0.
          Hide
          Bikas Saha added a comment -

          Yes, this can be done via the methods used in the current windows container launch code.

          Show
          Bikas Saha added a comment - Yes, this can be done via the methods used in the current windows container launch code.

            People

            • Assignee:
              Thomas Graves
              Reporter:
              Thomas Graves
            • Votes:
              0 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development