Konstantinos Karanasos added a comment - 03/Nov/16 02:59 Attaching patch.

Hadoop QA added a comment - 03/Nov/16 04:32 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s 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 15s Maven dependency ordering for branch +1 mvninstall 6m 48s trunk passed +1 compile 1m 31s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 1m 26s trunk passed +1 mvneclipse 0m 43s trunk passed +1 findbugs 2m 12s trunk passed +1 javadoc 0m 55s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 11s the patch passed +1 compile 1m 29s the patch passed +1 javac 1m 29s the patch passed -0 checkstyle 0m 28s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 2 new + 13 unchanged - 1 fixed = 15 total (was 14) +1 mvnsite 1m 23s the patch passed +1 mvneclipse 0m 37s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 43s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 50s the patch passed +1 unit 0m 26s hadoop-yarn-server-common in the patch passed. +1 unit 15m 46s hadoop-yarn-server-nodemanager in the patch passed. +1 unit 38m 42s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 84m 0s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common   Should org.apache.hadoop.yarn.server.scheduler.OpportunisticContainerAllocator$PartitionedResourceRequests be a static inner class? At OpportunisticContainerAllocator.java:inner class? At OpportunisticContainerAllocator.java: [lines 160-169] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836713/YARN-5823.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 187c0faa9ada 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 / 7e521c5 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13764/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13764/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-common.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13764/testReport/ modules C: 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/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13764/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Konstantinos Karanasos added a comment - 04/Nov/16 00:04 Attaching new patch – fixing findbug and checkstyle issues.

Hadoop QA added a comment - 04/Nov/16 01:31 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s 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 50s Maven dependency ordering for branch +1 mvninstall 7m 17s trunk passed +1 compile 1m 37s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 1m 29s trunk passed +1 mvneclipse 0m 44s trunk passed +1 findbugs 2m 20s trunk passed +1 javadoc 0m 55s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 15s the patch passed +1 compile 1m 37s the patch passed +1 javac 1m 37s the patch passed +1 checkstyle 0m 30s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server: The patch generated 0 new + 13 unchanged - 1 fixed = 13 total (was 14) +1 mvnsite 1m 24s the patch passed +1 mvneclipse 0m 39s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 43s the patch passed +1 javadoc 0m 53s the patch passed +1 unit 0m 27s hadoop-yarn-server-common in the patch passed. +1 unit 14m 56s hadoop-yarn-server-nodemanager in the patch passed. +1 unit 35m 57s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 82m 20s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12837000/YARN-5823.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cbe578009b89 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 5cad93d Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13776/testReport/ modules C: 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/hadoop-yarn-server Console output https://builds.apache.org/job/PreCommit-YARN-Build/13776/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Arun Suresh added a comment - 04/Nov/16 16:39 - edited Thanks for the patch Konstantinos Karanasos . I am thinking a simpler fix would be to just replace if (!allocation.getContainers().isEmpty()) in line 545 of ApplicationMasterService with if (!allocation.getNMTokens().isEmpty()) This also retains the guarantee that the pullUpdatedNMTokens() and allocation.getNMTokens() is called exactly once per allocate call. Ideally I would also like to pull all the opportunistic containers from the Allocation object as well, but that can be handled as part of YARN-5688

Konstantinos Karanasos added a comment - 04/Nov/16 18:43 Thanks for checking the patch, Arun Suresh . I like what you propose, but the problem would be that the opportunistic allocation has to happen strictly before the guaranteed allocation for that to work. At the current patch, I am doing the guaranteed allocation first, since it is non-blocking. As you say, as part of YARN-5688 we should revisit the order of steps once both guaranteed and opportunistic allocations will be asynchronous. Makes sense?

Arun Suresh added a comment - 04/Nov/16 21:07 At the current patch, I am doing the guaranteed allocation first, since it is non-blocking The reason I had raised YARN-5688 was to ensure the rpc handler for the AM's allocate is not blocked by the Opportunistic allocator. So, It really does not matter if the super.allocate() call happens before or after the opportunistic allocation, since the Scheduler is actually triggered by the node heartbeat thread. If you do not see any other issue with the opportunistic allocation happening before super.allocate() , I'd prefer we keep things as they are.

Konstantinos Karanasos added a comment - 08/Nov/16 03:36 Adding new version of the patch, in which I am calling pullNMTokens() only once, following Arun Suresh 's suggestion. I also included a new test in TestOpportunisticContainerAllocation , which would fail without the present patch.

Hadoop QA added a comment - 08/Nov/16 05:43 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s 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 9s Maven dependency ordering for branch +1 mvninstall 7m 14s trunk passed +1 compile 6m 52s trunk passed +1 checkstyle 0m 50s trunk passed +1 mvnsite 2m 35s trunk passed +1 mvneclipse 1m 39s trunk passed +1 findbugs 3m 32s trunk passed +1 javadoc 2m 18s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 37s the patch passed +1 compile 5m 5s the patch passed +1 javac 5m 5s the patch passed +1 checkstyle 0m 46s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) +1 mvnsite 2m 32s the patch passed +1 mvneclipse 1m 34s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 54s the patch passed +1 javadoc 1m 49s the patch passed +1 unit 0m 37s hadoop-yarn-server-common in the patch passed. -1 unit 15m 57s hadoop-yarn-server-nodemanager in the patch failed. -1 unit 39m 54s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 16m 21s hadoop-yarn-client in the patch passed. +1 asflicense 0m 39s The patch does not generate ASF License warnings. 125m 20s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.queuing.TestQueuingContainerManager   hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService   hadoop.yarn.server.resourcemanager.TestApplicationMasterService Subsystem Report/Notes Docker Image:yetus/hadoop:e809691 JIRA Issue YARN-5823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12837907/YARN-5823.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0d7b0972d67f 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 / 3fff158 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13818/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/13818/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/13818/testReport/ modules C: 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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13818/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Konstantinos Karanasos added a comment - 08/Nov/16 19:54 Rebasing against trunk and fixing the failing test cases.

Hadoop QA added a comment - 08/Nov/16 22:02 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s 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 7m 47s trunk passed +1 compile 6m 57s trunk passed +1 checkstyle 0m 54s trunk passed +1 mvnsite 2m 44s trunk passed +1 mvneclipse 1m 38s trunk passed +1 findbugs 3m 32s trunk passed +1 javadoc 1m 49s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 2m 8s the patch passed +1 compile 5m 44s the patch passed +1 javac 5m 44s the patch passed +1 checkstyle 0m 47s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) +1 mvnsite 2m 46s the patch passed +1 mvneclipse 1m 32s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 59s the patch passed +1 javadoc 1m 48s the patch passed +1 unit 0m 37s hadoop-yarn-server-common in the patch passed. +1 unit 15m 10s hadoop-yarn-server-nodemanager in the patch passed. -1 unit 37m 24s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 16m 36s hadoop-yarn-client in the patch passed. +1 asflicense 0m 49s The patch does not generate ASF License warnings. 124m 8s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestApplicationMasterService Subsystem Report/Notes Docker Image:yetus/hadoop:e809691 JIRA Issue YARN-5823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12838038/YARN-5823.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 278fe5c01b2c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / dbb133c Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13830/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/13830/testReport/ modules C: 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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13830/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Konstantinos Karanasos added a comment - 08/Nov/16 22:41 Attaching the right patch.

Hadoop QA added a comment - 09/Nov/16 01:09 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s 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 7m 16s trunk passed +1 compile 6m 36s trunk passed +1 checkstyle 0m 48s trunk passed +1 mvnsite 2m 35s trunk passed +1 mvneclipse 1m 36s trunk passed +1 findbugs 3m 21s trunk passed +1 javadoc 1m 48s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 33s the patch passed +1 compile 4m 49s the patch passed +1 javac 4m 49s the patch passed -0 checkstyle 0m 47s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 41 unchanged - 2 fixed = 42 total (was 43) +1 mvnsite 2m 30s the patch passed +1 mvneclipse 1m 47s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 12s the patch passed +1 javadoc 1m 47s the patch passed +1 unit 0m 41s hadoop-yarn-server-common in the patch passed. +1 unit 16m 23s hadoop-yarn-server-nodemanager in the patch passed. +1 unit 40m 47s hadoop-yarn-server-resourcemanager in the patch passed. -1 unit 31m 48s hadoop-yarn-client in the patch failed. +1 asflicense 0m 39s The patch does not generate ASF License warnings. 141m 20s Reason Tests Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy   hadoop.yarn.client.api.impl.TestOpportunisticContainerAllocation Timed out junit tests org.apache.hadoop.yarn.client.api.impl.TestDistributedScheduling Subsystem Report/Notes Docker Image:yetus/hadoop:e809691 JIRA Issue YARN-5823 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12838064/YARN-5823.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux af333aea3d7b 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 / 29e3b34 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13832/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13832/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13832/testReport/ modules C: 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 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13832/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Arun Suresh added a comment - 09/Nov/16 08:04 +1 to the latest patch. TestOpportunisticContainerAllocation passes locally for me and the other two test failures are unrelated. Committing this shortly.. Thanks Konstantinos Karanasos

Hudson added a comment - 09/Nov/16 08:40 SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10797 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10797/ ) YARN-5823 . Update NMTokens in case of requests with only opportunistic (arun suresh: rev 283fa33febe043bd7b4fa87546be26c9c5a8f8b5) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestOpportunisticContainerAllocation.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/scheduler/DistributedScheduler.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-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/OpportunisticContainerAllocator.java

Konstantinos Karanasos added a comment - 09/Nov/16 19:13 Thanks for the review and the commit, Arun Suresh !

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