[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14275686#comment-14275686 ] Eric Payne commented on YARN-2932: -- [~leftnoteasy], thanks very much for your review and comments: bq. 1. Rename {{isQueuePreemptable}} to {{getQueuePreemptable}} for getter/setter consistency in {{CapacitySchedulerConfiguration}} Renamed. bq. 2. Should consider queue reinitialize when queue preemptable in configuration changes (See {{TestQueueParsing}}). And it's best to add a test for verify that. I'm sorry. I don't understand what you mean by the use of the word consider. Calling {{CapacityScheduler.reinitialize}} will follow the queue hierarchy down and eventually call {{AbstractCSQueue#setupQueueConfigs}} for every queue, so I don't think there is any additional code needed, unless I'm missing something. Were you just saying that I need to add a test case for that? {quote} 3. It's better to remove the {{defaultVal}} parameter in {{CapacitySchedulerConfiguration.isPreemptable}}: {code} public boolean isQueuePreemptable(String queue, boolean defaultVal) {code} And the default_value should be placed in {{CapacitySchedulerConfiguration}}, like other queue configuration options. I understand what you trying to do is moving some logic from queue to {{CapacitySchedulerConfiguration}}, but I still think it's better to keep the {{CapacitySchedulerConfiguration}} simply gets some values from configuration file. {quote} The problem is that without the {{defaultval}} parameter, {{AbstractCSQueue#isQueuePathHierarchyPreemptable}} can't tell if the queue has explicitly set its preemptability or if it is just returning the default. For example: {code} root: disable_preemption = true root.A: disable_preemption (the property is not set) root.B: disable_preemption = false (the property is explicitly set to false) {code} Let's say the {{getQueuePreemptable}} interface is changed to remove the {{defaultVal}} parameter, and that when {{getQueuePreemptable}} calls {{getBoolean}}, it uses {{false}} as the default. # {{getQueuePreemptable}} calls {{getBoolean}} on {{root}} ## {{getBoolean}} returns {{true}} because the {{disable_preemption}} property is set to {{true}} ## {{getQueuePreemptable}} inverts {{true}} and returns {{false}} (That is, {{root}} has preemption disabled, so it is not preemptable). # {{getQueuePreemptable}} calls {{getBoolean}} on {{root.A}} ## {{getBoolean}} returns {{false}} because there is no {{disable_preemption}} property set for this queue, so {{getBoolean}} returns the default. ## {{getQueuePreemptable}} inverts {{false}} and returns {{true}} # {{getQueuePreemptable}} calls {{getBoolean}} on {{root.B}} ## {{getBoolean}} returns {{false}} because {{disable_preemption}} property is set to {{false}} for this queue ## {{getQueuePreemptable}} inverts {{false}} and returns {{true}} At this point, {{isQueuePathHierarchyPreemptable}} needs to know if it should use the default preemption from {{root}} or if it should use the value from each child queue. In the case of {{root.A}}, the value from {{root}} ({{false}}) should be used because {{root.A}} does not have the property set. In the case of {{root.B}}, the value should be the one returned for {{root.B}} ({{true}}) because it is explicitly set. But, since both {{root.A}} and {{root.B}} both returned {{true}}, {{isQueuePathHierarchyPreemptable}} can't tell the difference. Does that make sense? Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt, YARN-2932.v2.txt, YARN-2932.v3.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14276101#comment-14276101 ] Wangda Tan commented on YARN-2932: -- [~eepayne], Thanks for response, *Re 2:* You're partially correct, queue finally calls setupQueueConfig when reinitialize is invoked. The CapacityScheduler reinitialization is creating a new set of queues, and copy new parameters to your old queues via {code} setupQueueConfigs( clusterResource, newlyParsedLeafQueue.capacity, newlyParsedLeafQueue.absoluteCapacity, newlyParsedLeafQueue.maximumCapacity, newlyParsedLeafQueue.absoluteMaxCapacity, ... {code} So you need put the parameter you wants to update to setupQueueConfig as well. Without that, queue will not be refreshed. I didn't find any changes to parameter of setupQueueConfig, so I guess so, it's better to add a test to verify it. *Re 3:* You can take a look at how AbstractCSQueue initialize labels, {code} // get labels this.accessibleLabels = cs.getConfiguration().getAccessibleNodeLabels(getQueuePath()); // inherit from parent if labels not set if (this.accessibleLabels == null parent != null) { this.accessibleLabels = parent.getAccessibleNodeLabels(); } {code} I think they have similar logic -- For node label is trying to get value from configuration, if not set, inherit from parent. With this, you can make getPreemptable interface without defaultVal in CapacitySchedulerConfiguration. Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt, YARN-2932.v2.txt, YARN-2932.v3.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14274118#comment-14274118 ] Hadoop QA commented on YARN-2932: - {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12691699/YARN-2932.v3.txt against trunk revision ae7bf31. {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 3 new or modified test files. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:red}-1 javadoc{color}. The javadoc tool appears to have generated 1 warning messages. See https://builds.apache.org/job/PreCommit-YARN-Build/6312//artifact/patchprocess/diffJavadocWarnings.txt for details. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:red}-1 core tests{color}. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: org.apache.hadoop.yarn.server.resourcemanager.TestWorkPreservingRMRestart Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6312//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6312//console This message is automatically generated. Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt, YARN-2932.v2.txt, YARN-2932.v3.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14274448#comment-14274448 ] Wangda Tan commented on YARN-2932: -- 1. Rename isQueuePreemptable to getQueuePreemptable for getter/setter consistentency in CapacitySchedulerConfiguration 2. Should consider queue reinitialize when queue preemptable in configuration changes (See {{TestQueueParsing}}). And it's best to add a test for verify that. 3. It's better to remove the defaultVal parameter in CapacitySchedulerConfiguration.isPreemptable: {code} public boolean isQueuePreemptable(String queue, boolean defaultVal) {code} And the default_value should be placed in CapacitySchedulerConfiguration, like other queue configuration options. I understand what you trying to do is moving some logic from queue to CapacitySchedulerConfiguration, but I still think it's better to keep the CapacitySchedulerConfiguration simply gets some values from configuration file. Thanks, Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt, YARN-2932.v2.txt, YARN-2932.v3.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14272684#comment-14272684 ] Hadoop QA commented on YARN-2932: - {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12691517/YARN-2932.v2.txt against trunk revision ef3c3a8. {color:red}-1 patch{color}. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6303//console This message is automatically generated. Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt, YARN-2932.v2.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14261719#comment-14261719 ] Wangda Tan commented on YARN-2932: -- Hi [~eepayne], Thanks for working on this patch, sorry for late response since I was on vacation in the past few weeks. Just took a look at your patch, some comments: 1) Since the QUEUE_PREEMPTION_DISABLED is an option for CS, I suggest to make it as a member of CapacitySchedulerConfiguration, like {{getUserLimitFactor}}/{{setUserLimit}}, etc. This will void some String operations. 2) Rename {{context}} in {{AbstractCSQueue}} to name like {{csContext}} since we have {{rmContext}} 3) I suggest to add a member var like preemptable to AbstractCSQueue, instead of calling: {code} + @Private + public boolean isPreemptable() { +return context.getConfiguration().isPreemptable(getQueuePath()); + } {code} The implementation of CSConfiguration.isPreemptable(..) seems too complex to me. CSConfiguration should only care about value of configuration file, such logic should put to AbstractCSQueue.setupQueueConfigs(...) 4) It's better to web UI name (preemptable) and configuration name (disable_preemption) consistent. I prefer preemptable personally. 5) {{testIsPreemptable}} should be a part of TestCapacityScheduler instead of putting it to TestProportionalCapacityPreemptionPolicy. 6) In ProportionalCapacityPreemptionPolicy.cloneQueues, preemptable field should get from Queue instead of getting from configuration. Please let me know your thoughts, Wangda Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14250788#comment-14250788 ] Hadoop QA commented on YARN-2932: - {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12687837/YARN-2932.v1.txt against trunk revision f2d150e. {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 2 new or modified test files. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. There were no new javadoc warning messages. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:red}-1 findbugs{color}. The patch appears to introduce 14 new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:red}-1 core tests{color}. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService org.apache.hadoop.yarn.server.resourcemanager.TestMoveApplication Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6135//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/6135//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-server-resourcemanager.html Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6135//console This message is automatically generated. Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne Attachments: YARN-2932.v1.txt YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14242909#comment-14242909 ] Wangda Tan commented on YARN-2932: -- bq. Actually, rather than getting the queue's 'preemption-disable' status, I think it would make more sense to get the queue's preemption status. So, something like getPreemptionStatus. It would return true or false, depending on if queue is preemptable or not. What do you think? Make sense to me. Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14241675#comment-14241675 ] Eric Payne commented on YARN-2932: -- {quote} IIRC, YARN-2056 putting the disable preemption configuration for queue code in ProportionalCapacityPreemptionPolicy instead of putting them into CapacitySchedulerConfiguration. But after read this proposal, I think we should move them to CapacitySchedulerConfiguration, and getIsPreemptionDisabled should be a method of CSQueue interface, thoughts? {quote} [~leftnoteasy], yes, I agree. These are good ideas. Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14241793#comment-14241793 ] Eric Payne commented on YARN-2932: -- Hi [~leftnoteasy]. bq. {{getIsPreemptionDisabled}} should be a method of CSQueue Actually, rather than getting the queue's 'preemption-disable' status, I think it would make more sense to get the queue's preemption status. So, something like {{getPreemptionStatus}}. It would return true or false, depending on if queue is preemptable or not. What do you think? Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2932) Add entry for preemption setting to queue status screen and startup/refresh logging
[ https://issues.apache.org/jira/browse/YARN-2932?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14238582#comment-14238582 ] Wangda Tan commented on YARN-2932: -- Thanks for raising this up [~eepayne], it is a good adding. IIRC, YARN-2056 putting the disable preemption configuration for queue code in ProportionalCapacityPreemptionPolicy instead of putting them into CapacitySchedulerConfiguration. But after read this proposal, I think we should move them to CapacitySchedulerConfiguration, and getIsPreemptionDisabled should be a method of CSQueue interface, thoughts? Add entry for preemption setting to queue status screen and startup/refresh logging --- Key: YARN-2932 URL: https://issues.apache.org/jira/browse/YARN-2932 Project: Hadoop YARN Issue Type: Bug Affects Versions: 3.0.0, 2.7.0 Reporter: Eric Payne Assignee: Eric Payne YARN-2056 enables the ability to turn preemption on or off on a per-queue level. This JIRA will provide the preemption status for each queue in the {{HOST:8088/cluster/scheduler}} UI and in the RM log during startup/queue refresh. -- This message was sent by Atlassian JIRA (v6.3.4#6332)