You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/09/23 13:39:04 UTC
[GitHub] [hadoop] 9uapaw opened a new pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
9uapaw opened a new pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475
…xt easier to comprehend
<!--
Thanks for sending a pull request!
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
2. Make sure your PR title starts with JIRA issue id, e.g., 'HADOOP-17799. Your PR title ...'.
-->
### Description of PR
### How was this patch tested?
### For code changes:
- [ ] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] 9uapaw commented on a change in pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
9uapaw commented on a change in pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#discussion_r721071929
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
+ return null;
+ }
+
+ try {
+ writeLock.lock();
+ return queueManager.createQueue(fallbackContext);
+ } catch (YarnException | IOException e) {
+ // A null queue is expected if the placementContext is null. In order
+ // not to disrupt the control flow, if we fail to auto create a queue,
+ // we fall back to the original logic.
+ if (placementContext == null) {
+ LOG.error("Could not auto-create leaf queue " + queueName +
+ " due to : ", e);
+ return null;
}
+ handleQueueCreationError(applicationId, user, queueName, isRecovery, e);
+ } finally {
+ writeLock.unlock();
+ }
+ return null;
+ }
- if (fallbackContext.hasParentQueue()) {
- try {
- writeLock.lock();
- return queueManager.createQueue(fallbackContext);
- } catch (YarnException | IOException e) {
- // A null queue is expected if the placementContext is null. In order
- // not to disrupt the control flow, if we fail to auto create a queue,
- // we fall back to the original logic.
- if (placementContext == null) {
- LOG.error("Could not auto-create leaf queue " + queueName +
- " due to : ", e);
- return null;
- }
- if (isRecovery) {
- if (!getConfiguration().shouldAppFailFast(getConfig())) {
- LOG.error("Could not auto-create leaf queue " + queueName +
- " due to : ", e);
- this.rmContext.getDispatcher().getEventHandler().handle(
- new RMAppEvent(applicationId, RMAppEventType.KILL,
- "Application killed on recovery"
- + " as it was submitted to queue " + queueName
- + " which could not be auto-created"));
- } else{
- String queueErrorMsg =
- "Queue named " + queueName + " could not be "
- + "auto-created during application recovery.";
- LOG.error(FATAL, queueErrorMsg, e);
- throw new QueueInvalidException(queueErrorMsg);
- }
- } else{
- LOG.error("Could not auto-create leaf queue due to : ", e);
- final String message =
- "Application " + applicationId + " submission by user : "
- + user
- + " to queue : " + queueName + " failed : " + e
- .getMessage();
- this.rmContext.getDispatcher().getEventHandler().handle(
- new RMAppEvent(applicationId, RMAppEventType.APP_REJECTED,
- message));
- }
- } finally {
- writeLock.unlock();
- }
+ private void handleQueueCreationError(
+ ApplicationId applicationId, String user, String queueName,
+ boolean isRecovery, Exception e) {
+ if (isRecovery) {
+ if (!getConfiguration().shouldAppFailFast(getConfig())) {
Review comment:
Also found that shouldAppFailFast is a strange method, that could be either:
1. Defined only in the config class without a parameter
2. Defined as a static method
I have chosen the latter, as I am not sure about how each configuration is used in CS (there are 3 of them) and I would refrain from altering it as of now (it could lead to really subtle issues).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra merged pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
shuzirra merged pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#issuecomment-938032962
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 37s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 31m 42s | | trunk passed |
| +1 :green_heart: | compile | 1m 3s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 54s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 50s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 13s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 48s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 42s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 49s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 52s | | the patch passed |
| +1 :green_heart: | compile | 0m 53s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 53s | | the patch passed |
| +1 :green_heart: | compile | 0m 47s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 0m 47s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 40s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 49s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 36s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 36s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 51s | | the patch passed |
| +1 :green_heart: | shadedclient | 19m 54s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 110m 30s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 197m 36s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3475 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 6bcadd3e5a3e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / bbe11a257863250e95adce684fb3537fcd915092 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/3/testReport/ |
| Max. process+thread count | 1009 (vs. ulimit of 5500) |
| 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://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#issuecomment-932456098
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 49s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 33m 58s | | trunk passed |
| +1 :green_heart: | compile | 1m 10s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 59s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 54s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 2s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 44s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 2m 5s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 8s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 51s | | the patch passed |
| +1 :green_heart: | compile | 0m 56s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 0m 47s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 0m 47s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 40s | [/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/2/artifact/out/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt) | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 3 new + 263 unchanged - 0 fixed = 266 total (was 263) |
| +1 :green_heart: | mvnsite | 0m 51s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 38s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 34s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 54s | | the patch passed |
| +1 :green_heart: | shadedclient | 19m 41s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 95m 37s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 186m 15s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3475 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux feb8ce5200f7 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / c2ca6c7334c700905ccdf4ce9aa4138b0ddb5061 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/2/testReport/ |
| Max. process+thread count | 941 (vs. ulimit of 5500) |
| 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://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#issuecomment-925982143
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 51s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 32m 1s | | trunk passed |
| +1 :green_heart: | compile | 1m 7s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 59s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 46s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 1s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 46s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 2m 26s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 2s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 55s | | the patch passed |
| +1 :green_heart: | compile | 0m 55s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 55s | | the patch passed |
| +1 :green_heart: | compile | 0m 50s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 0m 50s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 38s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 54s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 39s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 35s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 58s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 21s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 95m 52s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 33s | | The patch does not generate ASF License warnings. |
| | | 184m 50s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3475 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 0ba6f2e4bd4a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / b423d18b22ac4037fc6ebced4576c072e31e8979 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/1/testReport/ |
| Max. process+thread count | 955 (vs. ulimit of 5500) |
| 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://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] 9uapaw commented on a change in pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
9uapaw commented on a change in pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#discussion_r721072554
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
Review comment:
Done, but omitted the error log, because in addApplication method ambiguity is checked again and the error message would be the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra merged pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
shuzirra merged pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] 9uapaw commented on a change in pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
9uapaw commented on a change in pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#discussion_r721070781
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
+ return null;
+ }
+
+ try {
+ writeLock.lock();
+ return queueManager.createQueue(fallbackContext);
Review comment:
Actually I have replaced all references of ApplicationContext to QueuePath in QueueManager. I did not find any usage that would justify an overloaded method for this, but I presume it could be handy for PlacementRules maybe?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#issuecomment-938032962
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 37s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 31m 42s | | trunk passed |
| +1 :green_heart: | compile | 1m 3s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 54s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 50s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 13s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 48s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 42s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 49s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 52s | | the patch passed |
| +1 :green_heart: | compile | 0m 53s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 53s | | the patch passed |
| +1 :green_heart: | compile | 0m 47s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 0m 47s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 40s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 49s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 36s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 36s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 51s | | the patch passed |
| +1 :green_heart: | shadedclient | 19m 54s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 110m 30s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 197m 36s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3475 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 6bcadd3e5a3e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / bbe11a257863250e95adce684fb3537fcd915092 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/3/testReport/ |
| Max. process+thread count | 1009 (vs. ulimit of 5500) |
| 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://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3475/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra commented on a change in pull request #3475: YARN-10953. Make CapacityScheduler#getOrCreateQueueFromPlacementConte…
Posted by GitBox <gi...@apache.org>.
shuzirra commented on a change in pull request #3475:
URL: https://github.com/apache/hadoop/pull/3475#discussion_r719899219
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
Review comment:
We don't need fallbackContext, we need a fallback QueuePath, we don't actually use the placement context. See other comments why.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
+ return null;
+ }
+
+ try {
+ writeLock.lock();
+ return queueManager.createQueue(fallbackContext);
Review comment:
We need to make a polymorphic version of this method (actually two, one with the common parts, which accepts two strings, and one which accepts the QueuePath object) which accepts the QueuePath object, internally this method only uses parent / leaf names, and since we want to use QueuePath object more often, it is a good opportunity, to start adding support for it.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
+ return null;
+ }
+
Review comment:
Parent queue might be ambiguous as well, we either should check here or in the createQueue method, to give more detailed report about the problem.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
Review comment:
You might want to add an ambiguity check here (queueManager.isAmbiguous), due to legacy reasons we still need to expect queues referenced with non-full path, and if the short name of the queue is ambiguous, getQueue will return null. And perhaps we can give the user a more detailed reason if this is the case.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
+ return null;
+ }
+
+ try {
+ writeLock.lock();
+ return queueManager.createQueue(fallbackContext);
+ } catch (YarnException | IOException e) {
+ // A null queue is expected if the placementContext is null. In order
+ // not to disrupt the control flow, if we fail to auto create a queue,
+ // we fall back to the original logic.
+ if (placementContext == null) {
+ LOG.error("Could not auto-create leaf queue " + queueName +
+ " due to : ", e);
+ return null;
}
+ handleQueueCreationError(applicationId, user, queueName, isRecovery, e);
+ } finally {
+ writeLock.unlock();
+ }
+ return null;
+ }
- if (fallbackContext.hasParentQueue()) {
- try {
- writeLock.lock();
- return queueManager.createQueue(fallbackContext);
- } catch (YarnException | IOException e) {
- // A null queue is expected if the placementContext is null. In order
- // not to disrupt the control flow, if we fail to auto create a queue,
- // we fall back to the original logic.
- if (placementContext == null) {
- LOG.error("Could not auto-create leaf queue " + queueName +
- " due to : ", e);
- return null;
- }
- if (isRecovery) {
- if (!getConfiguration().shouldAppFailFast(getConfig())) {
- LOG.error("Could not auto-create leaf queue " + queueName +
- " due to : ", e);
- this.rmContext.getDispatcher().getEventHandler().handle(
- new RMAppEvent(applicationId, RMAppEventType.KILL,
- "Application killed on recovery"
- + " as it was submitted to queue " + queueName
- + " which could not be auto-created"));
- } else{
- String queueErrorMsg =
- "Queue named " + queueName + " could not be "
- + "auto-created during application recovery.";
- LOG.error(FATAL, queueErrorMsg, e);
- throw new QueueInvalidException(queueErrorMsg);
- }
- } else{
- LOG.error("Could not auto-create leaf queue due to : ", e);
- final String message =
- "Application " + applicationId + " submission by user : "
- + user
- + " to queue : " + queueName + " failed : " + e
- .getMessage();
- this.rmContext.getDispatcher().getEventHandler().handle(
- new RMAppEvent(applicationId, RMAppEventType.APP_REJECTED,
- message));
- }
- } finally {
- writeLock.unlock();
- }
+ private void handleQueueCreationError(
+ ApplicationId applicationId, String user, String queueName,
+ boolean isRecovery, Exception e) {
+ if (isRecovery) {
+ if (!getConfiguration().shouldAppFailFast(getConfig())) {
+ LOG.error("Could not auto-create leaf queue " + queueName +
+ " due to : ", e);
+ this.rmContext.getDispatcher().getEventHandler().handle(
+ new RMAppEvent(applicationId, RMAppEventType.KILL,
+ "Application killed on recovery"
+ + " as it was submitted to queue " + queueName
+ + " which could not be auto-created"));
Review comment:
which did not exist and could not be auto created.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
Review comment:
We should simply create the fallbackQueuePath here, we only use the fallbackContext, to create an instance of the QueuePath object, which has the ability to be created from a full path, so instead of CSQueueUtils.extractQueuePath(queueName); we can simply create the new QueuePath here, which saves us the creation of an unused ApplicationPlacementContext.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
+ return null;
+ }
+
+ try {
+ writeLock.lock();
+ return queueManager.createQueue(fallbackContext);
+ } catch (YarnException | IOException e) {
+ // A null queue is expected if the placementContext is null. In order
+ // not to disrupt the control flow, if we fail to auto create a queue,
+ // we fall back to the original logic.
+ if (placementContext == null) {
+ LOG.error("Could not auto-create leaf queue " + queueName +
+ " due to : ", e);
+ return null;
}
+ handleQueueCreationError(applicationId, user, queueName, isRecovery, e);
+ } finally {
+ writeLock.unlock();
+ }
+ return null;
+ }
- if (fallbackContext.hasParentQueue()) {
- try {
- writeLock.lock();
- return queueManager.createQueue(fallbackContext);
- } catch (YarnException | IOException e) {
- // A null queue is expected if the placementContext is null. In order
- // not to disrupt the control flow, if we fail to auto create a queue,
- // we fall back to the original logic.
- if (placementContext == null) {
- LOG.error("Could not auto-create leaf queue " + queueName +
- " due to : ", e);
- return null;
- }
- if (isRecovery) {
- if (!getConfiguration().shouldAppFailFast(getConfig())) {
- LOG.error("Could not auto-create leaf queue " + queueName +
- " due to : ", e);
- this.rmContext.getDispatcher().getEventHandler().handle(
- new RMAppEvent(applicationId, RMAppEventType.KILL,
- "Application killed on recovery"
- + " as it was submitted to queue " + queueName
- + " which could not be auto-created"));
- } else{
- String queueErrorMsg =
- "Queue named " + queueName + " could not be "
- + "auto-created during application recovery.";
- LOG.error(FATAL, queueErrorMsg, e);
- throw new QueueInvalidException(queueErrorMsg);
- }
- } else{
- LOG.error("Could not auto-create leaf queue due to : ", e);
- final String message =
- "Application " + applicationId + " submission by user : "
- + user
- + " to queue : " + queueName + " failed : " + e
- .getMessage();
- this.rmContext.getDispatcher().getEventHandler().handle(
- new RMAppEvent(applicationId, RMAppEventType.APP_REJECTED,
- message));
- }
- } finally {
- writeLock.unlock();
- }
+ private void handleQueueCreationError(
+ ApplicationId applicationId, String user, String queueName,
+ boolean isRecovery, Exception e) {
+ if (isRecovery) {
+ if (!getConfiguration().shouldAppFailFast(getConfig())) {
Review comment:
Couldn't we save it as a member variable, it seems a bit unnecessary to have 3 method calls, and a map lookup, just for getting a simple flag. We might need to make sure that the flag is recalculated on config reload tho.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
Review comment:
Please specify why is it an invalid path (it has empty parts)
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacityScheduler.java
##########
@@ -951,73 +951,79 @@ private CSQueue getOrCreateQueueFromPlacementContext(ApplicationId
applicationId, String user, String queueName,
ApplicationPlacementContext placementContext,
boolean isRecovery) {
-
CSQueue queue = getQueue(queueName);
ApplicationPlacementContext fallbackContext = placementContext;
- if (queue == null) {
- // Even if placement rules are turned off, we still have the opportunity
- // to auto create a queue.
- if (placementContext == null) {
- fallbackContext = CSQueueUtils.extractQueuePath(queueName);
- }
+ if (queue != null) {
+ return queue;
+ }
- //we need to make sure there is no empty path parts present
- String path = fallbackContext.getFullQueuePath();
- String[] pathParts = path.split("\\.");
- for (int i = 0; i < pathParts.length; i++) {
- if ("".equals(pathParts[i])) {
- LOG.error("Application submitted to invalid path: '{}'", path);
- return null;
- }
+ // Even if placement rules are turned off, we still have the opportunity
+ // to auto create a queue.
+ if (placementContext == null) {
+ fallbackContext = CSQueueUtils.extractQueuePath(queueName);
+ }
+
+ //we need to make sure there are no empty path parts present
+ QueuePath path = new QueuePath(fallbackContext.getFullQueuePath());
+ if (path.hasEmptyPart()) {
+ LOG.error("Application submitted to invalid path: '{}'", path);
+ return null;
+ }
+
+ if (!fallbackContext.hasParentQueue()) {
Review comment:
QueuePath has this feature as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org