[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-12-03 Thread Yufei Gu (JIRA)


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

Yufei Gu commented on YARN-9041:


The last patch looks good. +1 for the patch v7. Will commit this soon.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: fairscheduler, scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch, 
> YARN-9041.006.patch, YARN-9041.007.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-12-03 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

I'm so sorry for this brought unnecessary trouble to you, [~yufeigu]. 
{quote}1. \{{ * @return list preemptable containers}} should be something like 
{{the list of best preemptable containers for the resource request}}
{quote}
In v7 patch, I had modified the return javadoc comment by your suggestion.
{quote}We still need some comments in both tests to clarify which logic path 
the test are for. For example, we can add comments in 
{{testRelaxLocalityToPreemptLessAM}} to say that it tests the case that there 
is no less-AM-container solution in the remaining nodes.
{quote}
In v7 patch, I renamed the two tests with clarify name and also added comments 
for it.

Thanks for your help very much.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch, 
> YARN-9041.006.patch, YARN-9041.007.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-12-03 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
13s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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 1 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m 
14s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
42s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
34s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
48s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
11m 52s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
16s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
27s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
50s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
44s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
44s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
45s{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} shadedclient {color} | {color:green} 
12m 10s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
26s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
24s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}101m 28s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
28s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}153m 39s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12950469/YARN-9041.007.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux a2b2083b7808 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24 
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / de42555 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/22772/artifact/out/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/22772/testReport/ |
| Max. process+thread count | 891 (vs. ulimit of 1) |
| 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-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-12-03 Thread Yufei Gu (JIRA)


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

Yufei Gu commented on YARN-9041:


Thanks for the patch. Some nits:
# {{ * @return list preemptable containers}} should be something like {{the 
list of best preemptable containers for the resource request}}
# We still need some comments in both tests to clarify which logic path the 
test are for. For example, we can add comments in 
{{testRelaxLocalityToPreemptLessAM}} to say that it tests the case that there 
is no less-AM-container solution in the remaining nodes.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch, 
> YARN-9041.006.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-29 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Hi [~yufeigu], the v6 patch fixed all the issues you mentioned. Pls review 
again, thx~

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch, 
> YARN-9041.006.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-29 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
19s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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 1 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 21m 
42s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
52s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
53s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
13m 41s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
20s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
30s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
48s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
42s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
42s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
45s{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} shadedclient {color} | {color:green} 
13m 17s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
27s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}107m  9s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
28s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}165m  6s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12950114/YARN-9041.006.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux 4915fedeec5a 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct 
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / bad1203 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/22753/artifact/out/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/22753/testReport/ |
| Max. process+thread count | 890 (vs. ulimit of 1) |
| 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-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-29 Thread Yufei Gu (JIRA)


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

Yufei Gu commented on YARN-9041:


Hi [~jiwq], the patch v5 looks good in terms of logic. Some nits:
# Can you rename two tests or write comments to clarify their intentions? I 
suppose the goals of two methods are: One can find less AM containers solution 
in the relax locations and the other can't.
# It is a good practice to put the callee methods under the caller methods.
# Can you refactor to create a new method like this? Please remember to 
reorganize the method java doc.  And we probably don't need 
the comment "// Don't preempt AM containers just to satisfy local requests if 
relax // locality is enabled." in that case.
{code}
  /**
   * Iterate through matching
   *  nodes and identify containers to preempt all on one node, also
   ** optimizing for least number of AM container preemptions. Only nodes
   ** that match the locality level specified in the {@link ResourceRequest}
   ** are considered. However, if this would lead to AM preemption, and 
locality
   ** relaxation is allowed, then the search space is expanded to the 
remaining
   ** nodes.
   *
   * @param rr
   * @param potentialNodes
   * @return
   */
  private PreemptableContainers getBestPreemptableContainers(ResourceRequest 
rr, List potentialNodes) {
PreemptableContainers bestContainers =
identifyContainersToPreemptForOneContainer(potentialNodes, rr);


if (rr.getRelaxLocality()
&& !ResourceRequest.isAnyLocation(rr.getResourceName())
&& bestContainers != null
&& bestContainers.numAMContainers > 0) {
  List remainingNodes = 
scheduler.getNodeTracker().getAllNodes();
  remainingNodes.removeAll(potentialNodes);
  PreemptableContainers spareContainers = 
identifyContainersToPreemptForOneContainer(remainingNodes, rr);
  if (spareContainers != null && spareContainers.numAMContainers < 
bestContainers.numAMContainers) {
bestContainers = spareContainers;
  }
}
return bestContainers;
  }
{code}

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-28 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Thanks for [~Steven Rand] review and help.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-28 Thread Steven Rand (JIRA)


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

Steven Rand commented on YARN-9041:
---

bq. If we not allowed relax locality, it will executes three statements before 
used this patch. Otherwise it executes only one statement after used this 
patch. So I think reorder the conditions can improve the performance.

Yes, but it could also be true that {{bestContainers}} is {{null}}, which would 
short-circuit the other three checks, or that 
{{ResourceRequest.isAnyLocation(rr.getResourceName())}} is true, which would 
also short-circuit the other three. It's not immediately clear to me which 
condition is most likely to not be met / which one makes the most sense to put 
first in the hope of short-circuiting the others.

Anyway though, all four checks should be very cheap since all just involve 
looking at some object that's already in memory, and none have to make RPC 
calls or do any computation. So I'm okay with any order.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-28 Thread Yufei Gu (JIRA)


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

Yufei Gu commented on YARN-9041:


There is an error in test build which isn't related to your patch. I'll review 
later.
{code}
[ERROR] Failed to execute goal 
org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M1:test (default-test) on 
project hadoop-yarn-server-resourcemanager: There was a timeout or other error 
in the fork -> [Help 1]
{code}

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-28 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Not sure why Jenkins gives -1 even all UTs have passed, I have seen this for a 
couple of times, it should not be caused by 005 patch.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-28 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
19s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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 1 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 21m 
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
53s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
13m 45s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
16s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
29s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
46s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
44s{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} shadedclient {color} | {color:green} 
13m 35s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
21s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
26s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}116m 38s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
32s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}174m 39s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12949798/YARN-9041.005.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux d4e9509c1614 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct 
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 34a914b |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/22737/artifact/out/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/22737/testReport/ |
| Max. process+thread count | 878 (vs. ulimit of 1) |
| 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-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-27 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
17s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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 1 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m 
13s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
45s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
49s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
13m 39s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
30s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
48s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
41s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}  
0m 34s{color} | {color:orange} 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
 The patch generated 1 new + 4 unchanged - 0 fixed = 5 total (was 4) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
43s{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} shadedclient {color} | {color:green} 
14m  6s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
28s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}107m 21s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
23s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}166m 14s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12949788/YARN-9041.004.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux 8b9b44844a2a 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct 
5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 34a914b |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-YARN-Build/22735/artifact/out/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/22735/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
 |
|  Test Results | 

[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-27 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Updated 004 patch to fix the checkstyle issue and change the new UT logic to 
right.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch, YARN-9041.004.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-27 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
15s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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 1 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m 
14s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
51s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  1m  
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
15m 25s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
31s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
37s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
54s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
54s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}  
0m 36s{color} | {color:orange} 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
 The patch generated 2 new + 4 unchanged - 0 fixed = 6 total (was 4) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
56s{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} shadedclient {color} | {color:green} 
14m 41s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
33s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}112m  9s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  1m 
13s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}176m  7s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | 
hadoop.yarn.server.resourcemanager.TestApplicationMasterServiceCapacity |
|   | hadoop.yarn.server.resourcemanager.TestApplicationMasterServiceFair |
|   | 
hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerAutoCreatedQueuePreemption
 |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12949699/YARN-9041.003.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux 1122c3f4f4e1 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 
10:45:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / 96c104d |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| checkstyle | 

[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-27 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Hi [~yufeigu], thanks for your detailed review. I added the UT in the 003 
patch. Please help to review.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch, 
> YARN-9041.003.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-27 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Hi [~Steven Rand], thanks for your detailed review.
{quote}I'm curious, what's the motivation for reordering the conditions in the 
{{if}} block?
{quote}
If we not allowed relax locality, it will executes three statements before used 
this patch. Otherwise it executes only one statement after used this patch. So 
I think reorder the conditions can improve the performance.

I hope my explanation can give u some help.

 

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-26 Thread Steven Rand (JIRA)


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

Steven Rand commented on YARN-9041:
---

Yes, the v2 patch resolves my concern -- thanks [~jiwq] for fixing that.

I'm curious, what's the motivation for reordering the conditions in the {{if}} 
block?

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-26 Thread Yufei Gu (JIRA)


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

Yufei Gu commented on YARN-9041:


Hi [~jiwq], thanks for the patch. I like the idea to shrink the search space, 
and your patch v2 seems to solve the concern raised by [~Steven Rand]. However, 
it is necessary to provide a unit test case for the change. 

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-21 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
16s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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:red}-1{color} | {color:red} test4tests {color} | {color:red}  0m  
0s{color} | {color:red} 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. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m 
18s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
47s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
48s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
12m 54s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
14s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
32s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
38s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
38s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
30s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
43s{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} shadedclient {color} | {color:green} 
12m 32s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
27s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}104m 11s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
27s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}157m 48s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12949006/YARN-9041.002.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux e8d7f359298c 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 
17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / c8b3dfa |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/22658/artifact/out/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/22658/testReport/ |
| Max. process+thread count | 968 (vs. ulimit of 1) |
| modules | C: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 U: 

[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-21 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

Thanks for [~Steven Rand] detailed review. You are right, I ignored the 
problem. I uploaded the 002 patch fixed it.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-20 Thread Steven Rand (JIRA)


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

Steven Rand commented on YARN-9041:
---

I'm not sure that this is correct. I think that it can lead to failure to 
preempt in cases where we should be preempting. This will happen if the initial 
{{potentialNodes}} contain preemptible containers, but the remaining nodes 
don't.

Example to illustrate what I'm thinking:

* We have nodes A, B, and C
* At first {{potentialNodes}} includes only node A because we're preempting for 
a node-local request for that node
* We find that we can preempt a container on node A, but it's an 
ApplicationMaster
* With this patch, we change the search space to be only nodes B and C (without 
the patch, the search space becomes A, B, and C)
* There are no preemptible containers on nodes B and C

The outcome in this example is that we don't preempt at all. However, what we 
want to do is preempt the AM container on node A.

Hopefully that makes sense, but let me know if I'm misunderstanding.

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-20 Thread Wanqiang Ji (JIRA)


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

Wanqiang Ji commented on YARN-9041:
---

The UT failure should be irrelevant, I tested locally it can work correctly.

Hi [~yufeigu] and [~tangzhankun]

Can you help to review?

> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
>  Issue Type: Improvement
>  Components: scheduler preemption
>Reporter: Wanqiang Ji
>Assignee: Wanqiang Ji
>Priority: Major
> Attachments: YARN-9041.001.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM 
> preemption, and locality relaxation is allowed, then the search space is 
> expanded to all nodes changed to the remaining nodes. The remaining nodes are 
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
>  # rr.getRelaxLocality()
>  # !ResourceRequest.isAnyLocation(rr.getResourceName())
>  # bestContainers != null
>  # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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



[jira] [Commented] (YARN-9041) Optimize FSPreemptionThread#identifyContainersToPreempt method

2018-11-20 Thread Hadoop QA (JIRA)


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

Hadoop QA commented on YARN-9041:
-

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
12s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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:red}-1{color} | {color:red} test4tests {color} | {color:red}  0m  
0s{color} | {color:red} 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. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m 
16s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
41s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
36s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
44s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
12m 23s{color} | {color:green} branch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
16s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
31s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
39s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
28s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
40s{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} shadedclient {color} | {color:green} 
11m 43s{color} | {color:green} patch has no errors when building and testing 
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
15s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
27s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}101m 18s{color} 
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
23s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}153m 14s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f |
| JIRA Issue | YARN-9041 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/12948932/YARN-9041.001.patch |
| Optional Tests |  dupname  asflicense  compile  javac  javadoc  mvninstall  
mvnsite  unit  shadedclient  findbugs  checkstyle  |
| uname | Linux 2ea47ba1f428 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24 
10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/patchprocess/precommit/personality/provided.sh |
| git revision | trunk / a41b648 |
| maven | version: Apache Maven 3.3.9 |
| Default Java | 1.8.0_181 |
| findbugs | v3.1.0-RC1 |
| unit | 
https://builds.apache.org/job/PreCommit-YARN-Build/22643/artifact/out/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/22643/testReport/ |
| Max. process+thread count | 945 (vs. ulimit of 1) |
| modules | C: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
 U: