[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-13 Thread Wangda Tan (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15822091#comment-15822091
 ] 

Wangda Tan commented on YARN-6081:
--

Thanks for review and commit! [~sunilg], [~eepayne].

> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
> Fix For: 2.9.0, 3.0.0-alpha2
>
> Attachments: YARN-6081.001.patch, YARN-6081.002.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-11 Thread Sunil G (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15820030#comment-15820030
 ] 

Sunil G commented on YARN-6081:
---

Thanks [~leftnoteasy] for the updated patch, and thanks [~eepayne] for the 
review.

+1 from end also on latest patch. I will it commit later today.

> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
> Attachments: YARN-6081.001.patch, YARN-6081.002.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-11 Thread Eric Payne (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15819452#comment-15819452
 ] 

Eric Payne commented on YARN-6081:
--

+1 LGTM. The failed test ({{TestRMRestart}}) is not related to this patch.

> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
> Attachments: YARN-6081.001.patch, YARN-6081.002.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-11 Thread Hadoop QA (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15819374#comment-15819374
 ] 

Hadoop QA commented on YARN-6081:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
20s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 4 new or modified test 
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 14m 
49s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
37s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green}  0m 
19s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
10s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
23s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
38s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
34s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}  
0m 30s{color} | {color:orange} 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
 The patch generated 17 new + 931 unchanged - 3 fixed = 948 total (was 934) 
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
36s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green}  0m 
15s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
16s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
21s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 41m 52s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
22s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 66m 33s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.yarn.server.resourcemanager.TestRMRestart |
\\
\\
|| Subsystem || Report/Notes ||
| Docker |  Image:yetus/hadoop:a9ad5d6 |
| JIRA Issue | YARN-6081 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12847081/YARN-6081.002.patch |
| Optional Tests |  asflicense  compile  javac  javadoc  mvninstall  mvnsite  
unit  findbugs  checkstyle  |
| uname | Linux f62055017949 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 
15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh 
|
| git revision | trunk / e648b6e |
| Default Java | 1.8.0_111 |
| findbugs | v3.0.0 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-YARN-Build/14641/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/14641/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/14641/testReport/ |
| modules | C: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 U: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 |
| Console output | 

[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-11 Thread Eric Payne (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15819338#comment-15819338
 ] 

Eric Payne commented on YARN-6081:
--

Thanks [~leftnoteasy] for fixing this. I am reviewing today. I will update 
later today or early tomorrow.

> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
> Attachments: YARN-6081.001.patch, YARN-6081.002.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-11 Thread Sunil G (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15818018#comment-15818018
 ] 

Sunil G commented on YARN-6081:
---

Thanks [~leftnoteasy]. Good catch!

We decrement pending resources only if container is allocated (not reserved). 
So ideally we have to deduct reserved memory from pending resource if any. 
Ideally makes sense for me.

Few comments:
1. {{getTotalPendingResourcesConsideringUserLimit}}, Not part of this patch. 
Could have a java doc comment there as well. So it ll be make javadoc also more 
 better?
2. 
{code}
Resource pending = app.getAppAttemptResourceUsage().getPending(
partition);
if (deductReservedFromPending) {
  pending = Resources.subtract(pending,
  app.getAppAttemptResourceUsage().getReserved(partition));
}
{code}

I have one doubt here. {{pending}} holds a reference of pending resource of 
appAttemptResource usage. Inside {{if(deductReservedFromPending)}} block, that 
reference is getting updated. Is that intentional?

3.  
{code}
pending = Resources.max(resourceCalculator, lastClusterResource,
pending, Resources.none());
{code}
A quick doubt. Why are we using lastClusterResource here?

4. {{testPreemptionNotHappenForSingleReservedQueue}}, comment near verify block 
is confusing.
5. In {{testPendingResourcesConsideringUserLimit}}, could we also try to assert 
the app's pending and reserved too?


> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
> Attachments: YARN-6081.001.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-10 Thread Hadoop QA (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15817153#comment-15817153
 ] 

Hadoop QA commented on YARN-6081:
-

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
14s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green}  0m 
 0s{color} | {color:green} The patch appears to include 4 new or modified test 
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 15m 
11s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
43s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green}  0m 
18s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m  
5s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
22s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
35s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}  
0m 31s{color} | {color:orange} 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
 The patch generated 13 new + 888 unchanged - 3 fixed = 901 total (was 891) 
{color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
37s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green}  0m 
14s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 40m  
1s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch 
passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
17s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 64m 48s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker |  Image:yetus/hadoop:a9ad5d6 |
| JIRA Issue | YARN-6081 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12846734/YARN-6081.001.patch |
| Optional Tests |  asflicense  compile  javac  javadoc  mvninstall  mvnsite  
unit  findbugs  checkstyle  |
| uname | Linux 25d2db55cd11 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 / 4db119b |
| Default Java | 1.8.0_111 |
| findbugs | v3.0.0 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-YARN-Build/14634/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
 |
|  Test Results | 
https://builds.apache.org/job/PreCommit-YARN-Build/14634/testReport/ |
| modules | C: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 U: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 |
| Console output | 
https://builds.apache.org/job/PreCommit-YARN-Build/14634/console |
| Powered by | Apache Yetus 0.5.0-SNAPSHOT   http://yetus.apache.org |


This message was automatically generated.



> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 

[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-10 Thread Wangda Tan (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15816996#comment-15816996
 ] 

Wangda Tan commented on YARN-6081:
--

[~sunilg], [~eepayne]. Could you please review this fix? It will be better to 
be committed before YARN-5864.

> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
> Attachments: YARN-6081.001.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Commented] (YARN-6081) LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved from pending to avoid unnecessary preemption of reserved container

2017-01-10 Thread Wangda Tan (JIRA)

[ 
https://issues.apache.org/jira/browse/YARN-6081?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15816990#comment-15816990
 ] 

Wangda Tan commented on YARN-6081:
--

This is the test case to reproduce the problem:

{code}
  @Test
  public void testPreemptionNotHappenForSingleReservedQueue() {
Logger rootLogger = LogManager.getRootLogger();
rootLogger.setLevel(Level.DEBUG);

int[][] qData = new int[][]{
//  /   A   B   C
{ 100, 40, 40, 20 },  // abs
{ 100, 100, 100, 100 },  // maxCap
{ 100,  70,  0,  0 },  // used
{  10, 30,  0,  0 },  // pending
{   0,  50,  0,  0 },  // reserved
{   1,  1,  0,  0 },  // apps
{  -1,  1,  1,  1 },  // req granularity
{   3,  0,  0,  0 },  // subqueues
};
ProportionalCapacityPreemptionPolicy policy = buildPolicy(qData);
policy.editSchedule();
// ensure all pending rsrc from A get preempted from other queues
verify(mDisp, times(0)).handle(argThat(new IsPreemptionRequestFor(appA)));
  }
{code}

Please note that there's only one active queue. But preemption policy still 
preempt container from it.

> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> 
>
> Key: YARN-6081
> URL: https://issues.apache.org/jira/browse/YARN-6081
> Project: Hadoop YARN
>  Issue Type: Bug
>Reporter: Wangda Tan
>Assignee: Wangda Tan
>Priority: Critical
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org