Konstantinos Karanasos added a comment - 08/Dec/16 03:38 Attaching patch.

Arun Suresh added a comment - 08/Dec/16 18:41 Thanks for the patch Konstantinos Karanasos Couple of comments: You can probably completely remove the OpportunisticContainerAllocatorAMService::createMin/MaxResource, and replace it with rmContext.getScheduler().getMinimumResourceCapability() and rmContext.getScheduler().getMaximumResourceCapability() . This will ensure the values are in sync with the actual scheduler being used. Lets not remove the incr resources from the yarn_server_common_service_protos.proto , since there exists a yarn.scheduler.increment-allocation-mb/vcore . It is used by the FairScheduler, but not the capacity scheduler yet, and the normalize function actually has an argument for it. I feel, it would be better if we just add getIncrementResourceCapability() in YarnScheduler interface, and in AbstractYarnScheduler, put in a default implementation that delegates to getMinimumResourceCapability() . FairScheduler already has this, so you just have to put an @Override tag on the method. In NodeManagerMetrics , if you are changing the opportunisticContainerStarted method to startOpportunisticContainer , you should also correspondingly change opportunisticContainerCompleted to completeOpportunisticContainer . Also, lets no not move the placement of the methods, since it can cause merge issues during cherry-pick and the diffs become larger and difficult to make sense of sometimes.

Hadoop QA added a comment - 08/Dec/16 19:59 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 6m 58s trunk passed +1 compile 4m 57s trunk passed +1 checkstyle 0m 47s trunk passed +1 mvnsite 2m 17s trunk passed +1 mvneclipse 1m 18s trunk passed +1 findbugs 3m 42s trunk passed +1 javadoc 1m 34s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 38s the patch passed +1 compile 4m 38s the patch passed +1 cc 4m 38s the patch passed +1 javac 4m 38s the patch passed -0 checkstyle 0m 52s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 250 unchanged - 0 fixed = 252 total (was 250) +1 mvnsite 2m 27s the patch passed +1 mvneclipse 1m 26s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 31s the patch passed +1 javadoc 1m 35s the patch passed -1 unit 0m 35s hadoop-yarn-api in the patch failed. +1 unit 0m 35s hadoop-yarn-server-common in the patch passed. -1 unit 14m 7s hadoop-yarn-server-nodemanager in the patch failed. -1 unit 43m 14s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 106m 57s Reason Tests Failed junit tests hadoop.yarn.conf.TestYarnConfigurationFields   hadoop.yarn.server.nodemanager.webapp.TestNMWebServices   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.webapp.TestRMWebServices   hadoop.yarn.server.resourcemanager.TestResourceTrackerService   hadoop.yarn.webapp.TestRMWithCSRFFilter   hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesNodes Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5982 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12842271/YARN-5982.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux 8c1e8f0f3919 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c265515 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14228/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14228/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14228/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14228/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14228/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14228/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Konstantinos Karanasos added a comment - 08/Dec/16 23:28 Thanks for the review, Arun Suresh . I addressed your comments. I put back to the proto file the increment resource, but I did not move it from the FairScheduler configuration to the general YarnConfiguration. Let's do that in a separate JIRA if it is needed. I also fixed the checkstyle issues and the related test case that was failing.

Hadoop QA added a comment - 09/Dec/16 01:28 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 7m 45s trunk passed +1 compile 5m 8s trunk passed +1 checkstyle 0m 47s trunk passed +1 mvnsite 2m 26s trunk passed +1 mvneclipse 1m 18s trunk passed +1 findbugs 4m 1s trunk passed +1 javadoc 2m 3s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 58s the patch passed +1 compile 4m 41s the patch passed +1 javac 4m 41s the patch passed -0 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 234 unchanged - 0 fixed = 235 total (was 234) +1 mvnsite 2m 21s the patch passed +1 mvneclipse 1m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 4m 26s the patch passed +1 javadoc 1m 45s the patch passed +1 unit 0m 32s hadoop-yarn-api in the patch passed. +1 unit 2m 25s hadoop-yarn-common in the patch passed. -1 unit 13m 37s hadoop-yarn-server-nodemanager in the patch failed. -1 unit 42m 30s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 109m 37s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.webapp.TestNMWebServices   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.webapp.TestRMWebServices   hadoop.yarn.server.resourcemanager.TestResourceTrackerService   hadoop.yarn.webapp.TestRMWithCSRFFilter   hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesNodes Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5982 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12842443/YARN-5982.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 826cddf84200 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 13d8e55 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14236/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14236/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14236/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14236/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14236/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Konstantinos Karanasos added a comment - 09/Dec/16 22:30 Fixing remaining issues, including a checkstyle problem and a failing test. The remaining failing tests are unrelated (to be fixed as part of HADOOP-13852 ).

Hadoop QA added a comment - 10/Dec/16 00:25 +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 8m 7s trunk passed +1 compile 5m 17s trunk passed +1 checkstyle 0m 50s trunk passed +1 mvnsite 2m 38s trunk passed +1 mvneclipse 1m 21s trunk passed +1 findbugs 4m 32s trunk passed +1 javadoc 1m 56s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 46s the patch passed +1 compile 4m 35s the patch passed +1 javac 4m 35s the patch passed +1 checkstyle 0m 44s the patch passed +1 mvnsite 2m 19s the patch passed +1 mvneclipse 1m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 4m 23s the patch passed +1 javadoc 1m 42s the patch passed +1 unit 0m 31s hadoop-yarn-api in the patch passed. +1 unit 2m 37s hadoop-yarn-common in the patch passed. +1 unit 13m 20s hadoop-yarn-server-nodemanager in the patch passed. +1 unit 39m 38s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 107m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5982 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12842624/YARN-5982.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 2a7a30e71072 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5bd7dec Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14246/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14246/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Arun Suresh added a comment - 10/Dec/16 00:39 Committing this to trunk shortly. Thanks for working on this Konstantinos Karanasos ..

Arun Suresh added a comment - 10/Dec/16 00:44 Committed to trunk..

Hudson added a comment - 10/Dec/16 01:00 SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10981 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10981/ ) YARN-5982 . Simplify opportunistic container parameters and metrics. (arun suresh: rev b0aace21b1ef3436ba9d516186208fee9a9ceef2) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/scheduler/ContainerScheduler.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/metrics/NodeManagerMetrics.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/NodesPage.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerNode.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/dao/NodeInfo.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesNodes.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/OpportunisticContainerAllocatorAMService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java

Arun Suresh added a comment - 29/Dec/16 23:08 Committing this to branch-2