[jira] [Commented] (YARN-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15655814#comment-15655814 ] Hadoop QA commented on YARN-5819: - | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 24s{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 3 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m 40s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 37s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 24s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 41s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 23s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 5s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 24s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 33s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 32s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 32s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 19s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 30 unchanged - 0 fixed = 31 total (was 30) {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 8s{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:red}-1{color} | {color:red} unit {color} | {color:red} 40m 50s{color} | {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 16s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 57m 52s{color} | {color:black} {color} | \\ \\ || Reason || Tests || | Failed junit tests | hadoop.yarn.server.resourcemanager.metrics.TestSystemMetricsPublisher | \\ \\ || Subsystem || Report/Notes || | Docker | Image:yetus/hadoop:e809691 | | JIRA Issue | YARN-5819 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12838471/yarn-5819.YARN-4752.5.patch | | Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle | | uname | Linux 4d6e115b41bc 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 | YARN-4752 / 2140674 | | Default Java | 1.8.0_101 | | findbugs | v3.0.0 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/13865/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/13865/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/13865/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 | | Consol
[jira] [Commented] (YARN-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15655248#comment-15655248 ] Karthik Kambatla commented on YARN-5819: Since updating {{Resource}} is not atomic, it seemed safer to do reads/writes/updates protected by a lock. > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch, > yarn-5819.YARN-4752.2.patch, yarn-5819.YARN-4752.3.patch, > yarn-5819.YARN-4752.4.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15655170#comment-15655170 ] Daniel Templeton commented on YARN-5819: Do you need to synchronize {{getPreemptedResources()}}? Doesn't look like it helps to me. Maybe synchronize and return a copy? > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch, > yarn-5819.YARN-4752.2.patch, yarn-5819.YARN-4752.3.patch, > yarn-5819.YARN-4752.4.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15653242#comment-15653242 ] Hadoop QA commented on YARN-5819: - | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 18s{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 3 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m 12s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 34s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 21s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 39s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 17s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m 56s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 23s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 33s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 29s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 29s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 18s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 1 new + 30 unchanged - 0 fixed = 31 total (was 30) {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 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 8s{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} 42m 29s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 18s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 58m 24s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Image:yetus/hadoop:9560f25 | | JIRA Issue | YARN-5819 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12838296/yarn-5819.YARN-4752.4.patch | | Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle | | uname | Linux 206e4a7ee86a 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 | YARN-4752 / 9568f41 | | Default Java | 1.8.0_101 | | findbugs | v3.0.0 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/13851/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/13851/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/13851/console | | Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org | This message was automatically generated. > Verify fairshare and minshare preemption > > > Key
[jira] [Commented] (YARN-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15652915#comment-15652915 ] Hadoop QA commented on YARN-5819: - | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 21s{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 3 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 11m 24s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 33s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 21s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 39s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 18s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 2s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 23s{color} | {color:green} YARN-4752 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 31s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 31s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 19s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 30 unchanged - 0 fixed = 36 total (was 30) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 38s{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 4s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 19s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} unit {color} | {color:green} 40m 0s{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} 60m 21s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Image:yetus/hadoop:9560f25 | | JIRA Issue | YARN-5819 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12838282/yarn-5819.YARN-4752.3.patch | | Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle | | uname | Linux 95b697768340 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 | YARN-4752 / 9568f41 | | Default Java | 1.8.0_101 | | findbugs | v3.0.0 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/13848/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/13848/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/13848/console | | Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org | This message was automatically generated. > Verify fairshare and minshare preemption > > > Key
[jira] [Commented] (YARN-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15652786#comment-15652786 ] Karthik Kambatla commented on YARN-5819: Patch v3 incorporates most recent review feedback except the following bits. bq. In FSPreemptionThread.run(), the call to containerNode.removeContainerForPreemption(container) is pretty important to keep the FSSchedulerNode state sane. Do you need to do anything to make sure it doesn't get skipped because of an exception? In general, I'm a little nervous about the bookkeeping on the containers to preempt; it seems like something that could quietly get out of sync and cause issues. Very valid point. The root of this is splitting the logic of adding and removing containers on a node to be preempted across two threads - {{FSPreemptionThread}} marks them and {{PreemptContainersTask}} attempts preemption and removes them from the collection. The latter is triggered via {{FSPreemptionThread.run()}} -> {{preemptContainers()}} -> {{PreemptContainersTask.run()}}. In the current path, there is no exception-throwing code. However, one might add it in the future. To future-proof this, I could add a try-finally block to the task run method. That still leaves preemptContainers and I don't think a try-finally block is enough there. Ideally, I would like to call out that these two methods cannot throw Exceptions. Would javadoc-ing that be enough? bq. {{FSAppAttempt.preemptedResources}} should be final, as should its neighbors. The neighbors cannot be the way they are today - we update the reference to point to different objects at different times. bq. The import of org.junit.After in TestFairSchedulerPreemption is grouped wrong. The spacing alone was wrong. My IDE retains alphabetic order irrespective of whether the import is static. I thought that was normal. Should I change anything there? bq. {{TestFairSchedulerPreemption.writeAllocFile()}} never throws an IOException. It actually does when we call {{new FileWriter()}}. > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch, > yarn-5819.YARN-4752.2.patch, yarn-5819.YARN-4752.3.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15651747#comment-15651747 ] Daniel Templeton commented on YARN-5819: bq. Is there a good reason to not use for-loops here? It doesn't pass my smell test. I wasn't able to turn up any referenceable condemnation of what you've done. And my suggested alternative doesn't work, because the expression has to be an {{Iterable}}, not an {{Iterator}}. I don't have any argument left, other than to appeal to your humanity; think about the children. In any case, your {{queue}} variable hides a field from {{SchedulerApplicationAttempt}}. In {{FSPreemptionThread.run()}}, the call to {{containerNode.removeContainerForPreemption(container)}} is pretty important to keep the {{FSSchedulerNode}} state sane. Do you need to do anything to make sure it doesn't get skipped because of an exception? In general, I'm a little nervous about the bookkeeping on the containers to preempt; it seems like something that could quietly get out of sync and cause issues. {{FSAppAttempt.preemptedResources}} should be final, as should its neighbors. You need javadoc for the last three methods in {{FSSchedulerNode}}. {{FSSchedulerNode.containersForPreemption}} should be final. In {{FSSchedulerTestBase}}, some javadoc for {{rmNodes}} and {{addNode()}} would be helpful. The import of {{org.junit.After}} in {{TestFairSchedulerPreemption}} is grouped wrong. {{TestFairSchedulerPreemption.writeAllocFile()}} never throws an {{IOException}}. {{TestFairSchedulerPreemption.writeAllocFile()}} is now every bit as ugly as I feared it would be. What if you pull out the two different conditionals into separate functions, like: {code} private void writeAllocFile() { ... out.println(""); out.println(""); writePreemptionParams(out); // Child-1 out.println(""); writeResourceParams(out); out.println(""); // Child-2 out.println(""); writeResourceParams(out); out.println(""); out.println(""); // end of preemptable queue // Queue with preemption disallowed out.println(""); out.println("false" + ""); writePreemptionParams(out); // Child-1 out.println(""); writeResourceParams(out); out.println(""); // Child-2 out.println(""); writeResourceParams(out); out.println(""); out.println(""); // end of nonpreemptable queue out.println(""); ... } private void writePreemptionParams(PrintWriter out) { if (fairsharePreemption) { out.println("1" + ""); out.println("0" + ""); } else { out.println("0" + ""); } } private void writeResourceParams(PrintWriter out) { if (!fairsharePreemption) { out.println("4096mb,4vcores"); } } {code} Happy medium? > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch, yarn-5819.YARN-4752.2.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15649685#comment-15649685 ] Hadoop QA commented on YARN-5819: - | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 24s{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 3 new or modified test files. {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m 55s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 47s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 26s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 50s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 19s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 11s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 27s{color} | {color:green} YARN-4752 passed {color} | | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 44s{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 24s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 30 unchanged - 0 fixed = 36 total (was 30) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 46s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m 17s{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 17s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 25s{color} | {color:green} the patch passed {color} | | {color:red}-1{color} | {color:red} unit {color} | {color:red} 44m 46s{color} | {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 16s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 64m 25s{color} | {color:black} {color} | \\ \\ || Reason || Tests || | Failed junit tests | hadoop.yarn.server.resourcemanager.TestWorkPreservingRMRestart | | | hadoop.yarn.server.resourcemanager.scheduler.TestSchedulingWithAllocationRequestId | \\ \\ || Subsystem || Report/Notes || | Docker | Image:yetus/hadoop:9560f25 | | JIRA Issue | YARN-5819 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12837913/yarn-5819.YARN-4752.2.patch | | Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle | | uname | Linux 8b455096f1a0 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 | YARN-4752 / 9568f41 | | Default Java | 1.8.0_101 | | findbugs | v3.0.0 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/13837/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/13837/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/13837/testReport/ | | modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn
[jira] [Commented] (YARN-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15646541#comment-15646541 ] Karthik Kambatla commented on YARN-5819: [~templedf] - would you like me to post the patch on Github for easier reviews? > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch, yarn-5819.YARN-4752.2.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15646534#comment-15646534 ] Karthik Kambatla commented on YARN-5819: Thanks for the prompt and thorough first pass, [~templedf]. The second patch captures most of your suggestions except a couple. bq. Maybe I'm old, but I don't like non-incremental for loops: Is there a good reason to not use for-loops here? While I am not particular on for/while loop, readability is subjective. I actually find the for loop much more readable than the while loop and it avoids accidentally missing the queue increment. bq. There's an extra space after the cast in setupCluster() The [Oracle/Sun code conventions|http://www.oracle.com/technetwork/java/codeconventions-150003.pdf] recommend a blank space after the cast. > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch, yarn-5819.YARN-4752.2.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15645799#comment-15645799 ] Daniel Templeton commented on YARN-5819: This is just a first-pass review. After we settle on YARN-5783, I'll take a closer look. Maybe I'm old, but I don't like non-incremental _for_ loops: {code} -FSQueue queue = getQueue(); -while (!queue.getQueueName().equals("root")) { +for (FSQueue queue = getQueue(); +!queue.getQueueName().equals("root"); +queue = queue.getParent()) { {code} If you want to be safer about the _while_ loop, create an {{Iterator}}. In {{FSPremeptionThread}} you left some dead code: {code} //boolean preemptable = app.canContainerBePreempted(container); {code} In {{TestFairSchedulerPreemption}}, {{app1}} and {{app2}} would be nicer with more meaningful names, e.g. {{greedyApp}} and {{starvingApp}}. Is it worth it to combine the two config file writing methods into a single one that takes a boolean parameter. It's more succinct, but I'm not if sure the hit to readability is worth it. There's an extra space after the cast in {{setupCluster()}}: {code} scheduler = (FairScheduler) resourceManager.getResourceScheduler(); {code} Any way to share the {{addNode()}} and {{sendNodeUpdateEvents()}} methods with {{TestFSAppStarvation}}? Drop the hyphen from the {{@throws}} message in {{submitApps()}}. Might be nice to put the parameters references in {{@code}} tags. {{preemptable}} and {{nonpreemptable}} might be better names than {{allowed}} and {{disallowed}}. It took me a little while to figure out what was allowed and disallowed. > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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-5819) Verify fairshare and minshare preemption
[ https://issues.apache.org/jira/browse/YARN-5819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631873#comment-15631873 ] Karthik Kambatla commented on YARN-5819: This patch applies on top of YARN-5783. > Verify fairshare and minshare preemption > > > Key: YARN-5819 > URL: https://issues.apache.org/jira/browse/YARN-5819 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler >Affects Versions: 2.9.0 >Reporter: Karthik Kambatla >Assignee: Karthik Kambatla > Attachments: yarn-5819.YARN-4752.1.patch > > > JIRA to track the unit test(s) verifying both fairshare and minshare > preemption. The tests should verify: > # preemption within a single leaf queue > # preemption between sibling leaf queues > # preemption between non-sibling leaf queues > # {{allowPreemption = false}} should disallow preemption from a queue -- 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