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 2022/11/07 13:13:30 UTC
[GitHub] [hadoop] p-szucs opened a new pull request, #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
p-szucs opened a new pull request, #5115:
URL: https://github.com/apache/hadoop/pull/5115
### Description of PR
Code improvements in MutableCSConfigurationProvider class:
- Created ConfUpdateAssembler class to contain logic for ConfUpdate map creation from mutation info
- Eliminated duplicated code block (9 lines) in init / formatConfigurationInStore methods
- Changed method "getConfStore" to package-private
### 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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020228775
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
+ confUpdate.put(queuesConfig, null);
+ // Unset Ordering Policy of Leaf Queue converted from
+ // Parent Queue after removeQueue
+ String queueOrderingPolicy = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + ORDERING_POLICY;
+ proposedConf.unset(queueOrderingPolicy);
+ confUpdate.put(queueOrderingPolicy, null);
+ } else {
+ confUpdate.put(queuesConfig, Joiner.on(',').join(siblingQueues));
+ }
+ for (Map.Entry<String, String> confRemove : proposedConf.getValByRegex(
+ ".*" + queueToRemove.replaceAll("\\.", "\\.") + "\\..*")
+ .entrySet()) {
+ proposedConf.unset(confRemove.getKey());
+ confUpdate.put(confRemove.getKey(), null);
+ }
+ }
+ }
+ }
+
+ private static void addQueue(
+ QueueConfigInfo addInfo, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (addInfo != null) {
+ String queuePath = addInfo.getQueue();
+ String queueName = queuePath.substring(queuePath.lastIndexOf('.') + 1);
+ if (queuePath.lastIndexOf('.') == -1) {
+ throw new IOException("Can't add invalid queue " + queuePath);
+ } else if (getSiblingQueues(queuePath, proposedConf).contains(
+ queueName)) {
+ throw new IOException("Can't add existing queue " + queuePath);
+ }
+ String parentQueue = queuePath.substring(0, queuePath.lastIndexOf('.'));
+ String[] siblings = proposedConf.getQueues(parentQueue);
+ List<String> siblingQueues = siblings == null ? new ArrayList<>() :
+ new ArrayList<>(Arrays.<String>asList(siblings));
Review Comment:
It's not needed, removed that too.
--
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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1310620431
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 13s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 42s | | trunk passed |
| +1 :green_heart: | compile | 1m 25s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 9s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 5s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 17s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 5s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 55s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 24s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 1s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 24m 26s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 3s | | the patch passed |
| +1 :green_heart: | compile | 1m 9s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 9s | | the patch passed |
| +1 :green_heart: | compile | 1m 0s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 1m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 46s | [/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-5115/7/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 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 0s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 14s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 38s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 105m 16s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 47s | | The patch does not generate ASF License warnings. |
| | | 212m 54s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux e1ab0ebf7e22 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / cad2edcafa9387977f69b726454f44356331f442 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/7/testReport/ |
| Max. process+thread count | 949 (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-5115/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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] szilard-nemeth commented on pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
szilard-nemeth commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1312535163
Thanks @p-szucs for working on this.
Code LGTM, committed to trunk.
--
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] szilard-nemeth closed pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
szilard-nemeth closed pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
URL: https://github.com/apache/hadoop/pull/5115
--
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] K0K0V0K commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
K0K0V0K commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020428979
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
Review Comment:
thanks!
--
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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020230740
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/TestConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,170 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+/**
+ * Tests {@link ConfigurationUpdateAssembler}.
+ */
+public class TestConfigurationUpdateAssembler {
+
+ private static final String A_PATH = "root.a";
+ private static final String B_PATH = "root.b";
+ private static final String C_PATH = "root.c";
+
+ private static final String CONFIG_NAME = "testConfigName";
+ private static final String A_CONFIG_PATH = CapacitySchedulerConfiguration.PREFIX + A_PATH +
+ CapacitySchedulerConfiguration.DOT + CONFIG_NAME;
+ private static final String B_CONFIG_PATH = CapacitySchedulerConfiguration.PREFIX + B_PATH +
+ CapacitySchedulerConfiguration.DOT + CONFIG_NAME;
+ private static final String C_CONFIG_PATH = CapacitySchedulerConfiguration.PREFIX + C_PATH +
+ CapacitySchedulerConfiguration.DOT + CONFIG_NAME;
+ private static final String ROOT_QUEUES_PATH = CapacitySchedulerConfiguration.PREFIX +
+ CapacitySchedulerConfiguration.ROOT + CapacitySchedulerConfiguration.DOT +
+ CapacitySchedulerConfiguration.QUEUES;
+
+ private static final String A_INIT_CONFIG_VALUE = "aInitValue";
+ private static final String A_CONFIG_VALUE = "aValue";
+ private static final String B_INIT_CONFIG_VALUE = "bInitValue";
+ private static final String B_CONFIG_VALUE = "bValue";
+ private static final String C_CONFIG_VALUE = "cValue";
+
+ private CapacitySchedulerConfiguration csConfig;
+
+ @Before
+ public void setUp() {
+ csConfig = crateInitialCSConfig();
+ }
+
+ @Test
+ public void testAddQueue() throws Exception {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, C_CONFIG_VALUE);
+ QueueConfigInfo queueConfigInfo = new QueueConfigInfo(C_PATH, updateMap);
+ updateInfo.getAddQueueInfo().add(queueConfigInfo);
+
+ Map<String, String> configurationUpdate =
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+
+ assertEquals(C_CONFIG_VALUE, configurationUpdate.get(C_CONFIG_PATH));
+ assertEquals("a,b,c", configurationUpdate.get(ROOT_QUEUES_PATH));
+ }
+
+ @Test
+ public void testAddExistingQueue() {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, A_CONFIG_VALUE);
+ QueueConfigInfo queueConfigInfo = new QueueConfigInfo(A_PATH, updateMap);
+ updateInfo.getAddQueueInfo().add(queueConfigInfo);
+
+ assertThrows(IOException.class, () -> {
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+ });
+ }
+
+ @Test
+ public void testAddInvalidQueue() {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, A_CONFIG_VALUE);
+ QueueConfigInfo queueConfigInfo = new QueueConfigInfo("invalidPath", updateMap);
+ updateInfo.getAddQueueInfo().add(queueConfigInfo);
+
+ assertThrows(IOException.class, () -> {
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+ });
+ }
+
+ @Test
+ public void testUpdateQueue() throws Exception {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, A_CONFIG_VALUE);
+ QueueConfigInfo queueAConfigInfo = new QueueConfigInfo(A_PATH, updateMap);
+ updateInfo.getUpdateQueueInfo().add(queueAConfigInfo);
+
+ Map<String, String> updateMapQueueB = new HashMap<>();
+ updateMapQueueB.put(CONFIG_NAME, B_CONFIG_VALUE);
+ QueueConfigInfo queueBConfigInfo = new QueueConfigInfo(B_PATH, updateMapQueueB);
+
+ updateInfo.getUpdateQueueInfo().add(queueBConfigInfo);
+
+ Map<String, String> configurationUpdate =
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+
+ assertEquals(A_CONFIG_VALUE, configurationUpdate.get(A_CONFIG_PATH));
+ assertEquals(B_CONFIG_VALUE, configurationUpdate.get(B_CONFIG_PATH));
+ }
+
+ @Test
+ public void testRemoveQueue() throws Exception {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ updateInfo.getRemoveQueueInfo().add(A_PATH);
+
+ Map<String, String> configurationUpdate =
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+
+ assertEquals(null, configurationUpdate.get(A_CONFIG_PATH));
Review Comment:
I agree, fixed it
--
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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1310498093
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 47s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 39m 37s | | trunk passed |
| +1 :green_heart: | compile | 1m 15s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 5s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 1s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 12s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 4s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 46s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 1s | | the patch passed |
| +1 :green_heart: | compile | 1m 5s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 5s | | the patch passed |
| +1 :green_heart: | compile | 0m 54s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 54s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 42s | [/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-5115/5/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 14 new + 0 unchanged - 0 fixed = 14 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 56s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 1s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 30s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 99m 5s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 201m 1s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux bf5ac2257347 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 2f8482501597bf9303d65cc261ac87be97851787 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/5/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-5115/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1305877086
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 50s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets 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 | 39m 57s | | trunk passed |
| +1 :green_heart: | compile | 1m 13s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 5s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 8s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 10s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 13s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 14s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 3s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 1m 2s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 2s | | the patch passed |
| +1 :green_heart: | compile | 0m 55s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 55s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 44s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 56s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 45s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 59s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 48s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 99m 3s | [/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/1/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt) | hadoop-yarn-server-resourcemanager in the patch passed. |
| -1 :x: | asflicense | 0m 46s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/1/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 201m 3s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.yarn.server.resourcemanager.webapp.TestRMWebServices |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux b4cb5f9d1268 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / c12e9b829c110d675d8895470560800b6313e5ec |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/1/testReport/ |
| Max. process+thread count | 990 (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-5115/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020228324
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
+ confUpdate.put(queuesConfig, null);
+ // Unset Ordering Policy of Leaf Queue converted from
+ // Parent Queue after removeQueue
+ String queueOrderingPolicy = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + ORDERING_POLICY;
+ proposedConf.unset(queueOrderingPolicy);
+ confUpdate.put(queueOrderingPolicy, null);
+ } else {
+ confUpdate.put(queuesConfig, Joiner.on(',').join(siblingQueues));
+ }
+ for (Map.Entry<String, String> confRemove : proposedConf.getValByRegex(
+ ".*" + queueToRemove.replaceAll("\\.", "\\.") + "\\..*")
Review Comment:
Seems to me that it doesn't add any value, so removed it :)
--
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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1308629101
:broken_heart: **-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. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 | 40m 25s | | trunk passed |
| +1 :green_heart: | compile | 1m 12s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 13s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 2s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 12s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 8s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 54s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 22s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 1s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 1m 3s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 3s | | the patch passed |
| +1 :green_heart: | compile | 0m 57s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 48s | [/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-5115/4/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 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 0m 57s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 41s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 2s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 18s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 99m 5s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 45s | | The patch does not generate ASF License warnings. |
| | | 201m 19s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 6547d975e2d1 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 04bc57facbeedf7e3894664ff98ea103a74e4793 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/4/testReport/ |
| Max. process+thread count | 976 (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-5115/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020226416
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
Review Comment:
Sure, fixed it
--
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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1307712841
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 46s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets 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 | 39m 1s | | trunk passed |
| +1 :green_heart: | compile | 1m 12s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 6s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 3s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 12s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 15s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 54s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 16s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 44s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 59s | | the patch passed |
| +1 :green_heart: | compile | 1m 0s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 0s | | the patch passed |
| +1 :green_heart: | compile | 0m 58s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 58s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 49s | [/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-5115/3/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 + 0 unchanged - 0 fixed = 3 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 3s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 45s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 4s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 32s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 99m 5s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 43s | | The patch does not generate ASF License warnings. |
| | | 199m 49s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux b92795902d53 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / bc59203a11fb19851d6215050c15818c509151ad |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/3/testReport/ |
| Max. process+thread count | 961 (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-5115/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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] K0K0V0K commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
K0K0V0K commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1019610692
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
Review Comment:
I think that would be better if first we check the `queueToRemove` contains dot or not and we create the `queueName` after that check.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
Review Comment:
I think isEmpty is better to use instead of size == 0, cause some List implementation requires O(N) to count the size, but can compute the isEmpty at O(1)
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
Review Comment:
If we use an `if (queueToRemove == null) {return;}` like early return, we can reduce the depth of this method
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
Review Comment:
Minor stuff, but we can leave this else, cause in the if we throw an exception, so the run will be interrupted.
This way we can reduce the depth of the code.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
+ confUpdate.put(queuesConfig, null);
+ // Unset Ordering Policy of Leaf Queue converted from
+ // Parent Queue after removeQueue
+ String queueOrderingPolicy = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + ORDERING_POLICY;
+ proposedConf.unset(queueOrderingPolicy);
+ confUpdate.put(queueOrderingPolicy, null);
+ } else {
+ confUpdate.put(queuesConfig, Joiner.on(',').join(siblingQueues));
+ }
+ for (Map.Entry<String, String> confRemove : proposedConf.getValByRegex(
+ ".*" + queueToRemove.replaceAll("\\.", "\\.") + "\\..*")
Review Comment:
why do we need this replace all?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/TestConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,170 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+/**
+ * Tests {@link ConfigurationUpdateAssembler}.
+ */
+public class TestConfigurationUpdateAssembler {
+
+ private static final String A_PATH = "root.a";
+ private static final String B_PATH = "root.b";
+ private static final String C_PATH = "root.c";
+
+ private static final String CONFIG_NAME = "testConfigName";
+ private static final String A_CONFIG_PATH = CapacitySchedulerConfiguration.PREFIX + A_PATH +
+ CapacitySchedulerConfiguration.DOT + CONFIG_NAME;
+ private static final String B_CONFIG_PATH = CapacitySchedulerConfiguration.PREFIX + B_PATH +
+ CapacitySchedulerConfiguration.DOT + CONFIG_NAME;
+ private static final String C_CONFIG_PATH = CapacitySchedulerConfiguration.PREFIX + C_PATH +
+ CapacitySchedulerConfiguration.DOT + CONFIG_NAME;
+ private static final String ROOT_QUEUES_PATH = CapacitySchedulerConfiguration.PREFIX +
+ CapacitySchedulerConfiguration.ROOT + CapacitySchedulerConfiguration.DOT +
+ CapacitySchedulerConfiguration.QUEUES;
+
+ private static final String A_INIT_CONFIG_VALUE = "aInitValue";
+ private static final String A_CONFIG_VALUE = "aValue";
+ private static final String B_INIT_CONFIG_VALUE = "bInitValue";
+ private static final String B_CONFIG_VALUE = "bValue";
+ private static final String C_CONFIG_VALUE = "cValue";
+
+ private CapacitySchedulerConfiguration csConfig;
+
+ @Before
+ public void setUp() {
+ csConfig = crateInitialCSConfig();
+ }
+
+ @Test
+ public void testAddQueue() throws Exception {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, C_CONFIG_VALUE);
+ QueueConfigInfo queueConfigInfo = new QueueConfigInfo(C_PATH, updateMap);
+ updateInfo.getAddQueueInfo().add(queueConfigInfo);
+
+ Map<String, String> configurationUpdate =
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+
+ assertEquals(C_CONFIG_VALUE, configurationUpdate.get(C_CONFIG_PATH));
+ assertEquals("a,b,c", configurationUpdate.get(ROOT_QUEUES_PATH));
+ }
+
+ @Test
+ public void testAddExistingQueue() {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, A_CONFIG_VALUE);
+ QueueConfigInfo queueConfigInfo = new QueueConfigInfo(A_PATH, updateMap);
+ updateInfo.getAddQueueInfo().add(queueConfigInfo);
+
+ assertThrows(IOException.class, () -> {
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+ });
+ }
+
+ @Test
+ public void testAddInvalidQueue() {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, A_CONFIG_VALUE);
+ QueueConfigInfo queueConfigInfo = new QueueConfigInfo("invalidPath", updateMap);
+ updateInfo.getAddQueueInfo().add(queueConfigInfo);
+
+ assertThrows(IOException.class, () -> {
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+ });
+ }
+
+ @Test
+ public void testUpdateQueue() throws Exception {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ Map<String, String> updateMap = new HashMap<>();
+ updateMap.put(CONFIG_NAME, A_CONFIG_VALUE);
+ QueueConfigInfo queueAConfigInfo = new QueueConfigInfo(A_PATH, updateMap);
+ updateInfo.getUpdateQueueInfo().add(queueAConfigInfo);
+
+ Map<String, String> updateMapQueueB = new HashMap<>();
+ updateMapQueueB.put(CONFIG_NAME, B_CONFIG_VALUE);
+ QueueConfigInfo queueBConfigInfo = new QueueConfigInfo(B_PATH, updateMapQueueB);
+
+ updateInfo.getUpdateQueueInfo().add(queueBConfigInfo);
+
+ Map<String, String> configurationUpdate =
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+
+ assertEquals(A_CONFIG_VALUE, configurationUpdate.get(A_CONFIG_PATH));
+ assertEquals(B_CONFIG_VALUE, configurationUpdate.get(B_CONFIG_PATH));
+ }
+
+ @Test
+ public void testRemoveQueue() throws Exception {
+ SchedConfUpdateInfo updateInfo = new SchedConfUpdateInfo();
+ updateInfo.getRemoveQueueInfo().add(A_PATH);
+
+ Map<String, String> configurationUpdate =
+ ConfigurationUpdateAssembler.constructKeyValueConfUpdate(csConfig, updateInfo);
+
+ assertEquals(null, configurationUpdate.get(A_CONFIG_PATH));
Review Comment:
I think assertNull is more meaning full
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
+ confUpdate.put(queuesConfig, null);
+ // Unset Ordering Policy of Leaf Queue converted from
+ // Parent Queue after removeQueue
+ String queueOrderingPolicy = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + ORDERING_POLICY;
+ proposedConf.unset(queueOrderingPolicy);
+ confUpdate.put(queueOrderingPolicy, null);
+ } else {
+ confUpdate.put(queuesConfig, Joiner.on(',').join(siblingQueues));
+ }
+ for (Map.Entry<String, String> confRemove : proposedConf.getValByRegex(
+ ".*" + queueToRemove.replaceAll("\\.", "\\.") + "\\..*")
+ .entrySet()) {
+ proposedConf.unset(confRemove.getKey());
+ confUpdate.put(confRemove.getKey(), null);
+ }
+ }
+ }
+ }
+
+ private static void addQueue(
+ QueueConfigInfo addInfo, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (addInfo != null) {
+ String queuePath = addInfo.getQueue();
+ String queueName = queuePath.substring(queuePath.lastIndexOf('.') + 1);
+ if (queuePath.lastIndexOf('.') == -1) {
+ throw new IOException("Can't add invalid queue " + queuePath);
+ } else if (getSiblingQueues(queuePath, proposedConf).contains(
+ queueName)) {
+ throw new IOException("Can't add existing queue " + queuePath);
+ }
+ String parentQueue = queuePath.substring(0, queuePath.lastIndexOf('.'));
+ String[] siblings = proposedConf.getQueues(parentQueue);
+ List<String> siblingQueues = siblings == null ? new ArrayList<>() :
+ new ArrayList<>(Arrays.<String>asList(siblings));
Review Comment:
Do we need this `<String>` here, or we can use just `new ArrayList<>(Arrays.asList(siblings))`
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
+ confUpdate.put(queuesConfig, null);
+ // Unset Ordering Policy of Leaf Queue converted from
+ // Parent Queue after removeQueue
+ String queueOrderingPolicy = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + ORDERING_POLICY;
+ proposedConf.unset(queueOrderingPolicy);
+ confUpdate.put(queueOrderingPolicy, null);
+ } else {
+ confUpdate.put(queuesConfig, Joiner.on(',').join(siblingQueues));
+ }
+ for (Map.Entry<String, String> confRemove : proposedConf.getValByRegex(
+ ".*" + queueToRemove.replaceAll("\\.", "\\.") + "\\..*")
+ .entrySet()) {
+ proposedConf.unset(confRemove.getKey());
+ confUpdate.put(confRemove.getKey(), null);
+ }
+ }
+ }
+ }
+
+ private static void addQueue(
+ QueueConfigInfo addInfo, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (addInfo != null) {
+ String queuePath = addInfo.getQueue();
+ String queueName = queuePath.substring(queuePath.lastIndexOf('.') + 1);
+ if (queuePath.lastIndexOf('.') == -1) {
+ throw new IOException("Can't add invalid queue " + queuePath);
+ } else if (getSiblingQueues(queuePath, proposedConf).contains(
+ queueName)) {
+ throw new IOException("Can't add existing queue " + queuePath);
+ }
+ String parentQueue = queuePath.substring(0, queuePath.lastIndexOf('.'));
+ String[] siblings = proposedConf.getQueues(parentQueue);
+ List<String> siblingQueues = siblings == null ? new ArrayList<>() :
+ new ArrayList<>(Arrays.<String>asList(siblings));
+ siblingQueues.add(queuePath.substring(queuePath.lastIndexOf('.') + 1));
+ proposedConf.setQueues(parentQueue,
+ siblingQueues.toArray(new String[0]));
+ confUpdate.put(CapacitySchedulerConfiguration.PREFIX
+ + parentQueue + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES,
+ Joiner.on(',').join(siblingQueues));
+ String keyPrefix = CapacitySchedulerConfiguration.PREFIX
+ + queuePath + CapacitySchedulerConfiguration.DOT;
+ for (Map.Entry<String, String> kv : addInfo.getParams().entrySet()) {
+ if (kv.getValue() == null) {
Review Comment:
In the updateQueue method we check the the value is empty or not. Why we dont check that here?
--
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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1311926728
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 53s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 8s | | trunk passed |
| +1 :green_heart: | compile | 1m 18s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 12s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 2s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 14s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 9s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 56s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 20s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 50s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 23m 11s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | compile | 0m 54s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 54s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 42s | [/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-5115/8/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 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 0s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 40s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 3s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 37s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 102m 17s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | | The patch does not generate ASF License warnings. |
| | | 205m 42s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 1525e7f89d59 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / ca8333e02e16d3e51986ef938253588d8569429e |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/8/testReport/ |
| Max. process+thread count | 939 (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-5115/8/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1306094812
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 1s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 | 39m 8s | | trunk passed |
| +1 :green_heart: | compile | 1m 18s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 4s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 0s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 13s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 6s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 52s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 14s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | compile | 0m 52s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 52s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 47s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 58s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 41s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 59s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 16s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 103m 4s | [/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/2/artifact/out/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt) | hadoop-yarn-server-resourcemanager in the patch passed. |
| -1 :x: | asflicense | 0m 45s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/2/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 203m 36s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.yarn.server.resourcemanager.webapp.TestRMWebServices |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux f0a9d8ec130a 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 398f88e5a5cee57e56bfaff5bfce0dc08dee9de8 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/2/testReport/ |
| Max. process+thread count | 973 (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-5115/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1311943030
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 11s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 39m 35s | | trunk passed |
| +1 :green_heart: | compile | 1m 19s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 8s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 0m 59s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 10s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 13s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 2s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 18s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 39s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 23m 5s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 1m 0s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 0s | | the patch passed |
| +1 :green_heart: | compile | 0m 54s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 54s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 42s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 59s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 43s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 6s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 7s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 102m 44s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 47s | | The patch does not generate ASF License warnings. |
| | | 205m 46s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 108b034dd35e 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 4b8d134e6ad5ab96e2c149a839ec920ddec23ac6 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/9/testReport/ |
| Max. process+thread count | 950 (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-5115/9/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020226747
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
Review Comment:
Fixed that in the add/update/remove methods
--
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 #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1310598998
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 5s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets 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 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 39m 28s | | trunk passed |
| +1 :green_heart: | compile | 1m 15s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 1m 4s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 8s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 12s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 14s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 0s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 2m 12s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 40s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 3s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | compile | 0m 57s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 43s | [/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-5115/6/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 14 new + 0 unchanged - 0 fixed = 14 total (was 0) |
| +1 :green_heart: | mvnsite | 1m 0s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 0m 42s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 1m 59s | | the patch passed |
| +1 :green_heart: | shadedclient | 21m 52s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 122m 59s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 57s | | The patch does not generate ASF License warnings. |
| | | 224m 37s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5115 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 533fa6c3dc7a 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 5f1a3f8b19a64967e36c6e6a4680adb026ee88f4 |
| Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5115/6/testReport/ |
| Max. process+thread count | 1000 (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-5115/6/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020227543
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
Review Comment:
Right, fixed that too
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
Review Comment:
Fixed
--
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] p-szucs commented on a diff in pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
p-szucs commented on code in PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#discussion_r1020230379
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/conf/ConfigurationUpdateAssembler.java:
##########
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.thirdparty.com.google.common.base.Joiner;
+import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration;
+import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
+import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.ORDERING_POLICY;
+
+public final class ConfigurationUpdateAssembler {
+
+ private ConfigurationUpdateAssembler() {
+ }
+
+ public static Map<String, String> constructKeyValueConfUpdate(
+ CapacitySchedulerConfiguration proposedConf,
+ SchedConfUpdateInfo mutationInfo) throws IOException {
+
+ Map<String, String> confUpdate = new HashMap<>();
+ for (String queueToRemove : mutationInfo.getRemoveQueueInfo()) {
+ removeQueue(queueToRemove, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo addQueueInfo : mutationInfo.getAddQueueInfo()) {
+ addQueue(addQueueInfo, proposedConf, confUpdate);
+ }
+ for (QueueConfigInfo updateQueueInfo : mutationInfo.getUpdateQueueInfo()) {
+ updateQueue(updateQueueInfo, proposedConf, confUpdate);
+ }
+ for (Map.Entry<String, String> global : mutationInfo.getGlobalParams()
+ .entrySet()) {
+ confUpdate.put(global.getKey(), global.getValue());
+ }
+ return confUpdate;
+ }
+
+ private static void removeQueue(
+ String queueToRemove, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (queueToRemove != null) {
+ String queueName = queueToRemove.substring(
+ queueToRemove.lastIndexOf('.') + 1);
+ if (queueToRemove.lastIndexOf('.') == -1) {
+ throw new IOException("Can't remove queue " + queueToRemove);
+ } else {
+ List<String> siblingQueues = getSiblingQueues(queueToRemove,
+ proposedConf);
+ if (!siblingQueues.contains(queueName)) {
+ throw new IOException("Queue " + queueToRemove + " not found");
+ }
+ siblingQueues.remove(queueName);
+ String parentQueuePath = queueToRemove.substring(0, queueToRemove
+ .lastIndexOf('.'));
+ proposedConf.setQueues(parentQueuePath, siblingQueues.toArray(
+ new String[0]));
+ String queuesConfig = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES;
+ if (siblingQueues.size() == 0) {
+ confUpdate.put(queuesConfig, null);
+ // Unset Ordering Policy of Leaf Queue converted from
+ // Parent Queue after removeQueue
+ String queueOrderingPolicy = CapacitySchedulerConfiguration.PREFIX
+ + parentQueuePath + CapacitySchedulerConfiguration.DOT
+ + ORDERING_POLICY;
+ proposedConf.unset(queueOrderingPolicy);
+ confUpdate.put(queueOrderingPolicy, null);
+ } else {
+ confUpdate.put(queuesConfig, Joiner.on(',').join(siblingQueues));
+ }
+ for (Map.Entry<String, String> confRemove : proposedConf.getValByRegex(
+ ".*" + queueToRemove.replaceAll("\\.", "\\.") + "\\..*")
+ .entrySet()) {
+ proposedConf.unset(confRemove.getKey());
+ confUpdate.put(confRemove.getKey(), null);
+ }
+ }
+ }
+ }
+
+ private static void addQueue(
+ QueueConfigInfo addInfo, CapacitySchedulerConfiguration proposedConf,
+ Map<String, String> confUpdate) throws IOException {
+ if (addInfo != null) {
+ String queuePath = addInfo.getQueue();
+ String queueName = queuePath.substring(queuePath.lastIndexOf('.') + 1);
+ if (queuePath.lastIndexOf('.') == -1) {
+ throw new IOException("Can't add invalid queue " + queuePath);
+ } else if (getSiblingQueues(queuePath, proposedConf).contains(
+ queueName)) {
+ throw new IOException("Can't add existing queue " + queuePath);
+ }
+ String parentQueue = queuePath.substring(0, queuePath.lastIndexOf('.'));
+ String[] siblings = proposedConf.getQueues(parentQueue);
+ List<String> siblingQueues = siblings == null ? new ArrayList<>() :
+ new ArrayList<>(Arrays.<String>asList(siblings));
+ siblingQueues.add(queuePath.substring(queuePath.lastIndexOf('.') + 1));
+ proposedConf.setQueues(parentQueue,
+ siblingQueues.toArray(new String[0]));
+ confUpdate.put(CapacitySchedulerConfiguration.PREFIX
+ + parentQueue + CapacitySchedulerConfiguration.DOT
+ + CapacitySchedulerConfiguration.QUEUES,
+ Joiner.on(',').join(siblingQueues));
+ String keyPrefix = CapacitySchedulerConfiguration.PREFIX
+ + queuePath + CapacitySchedulerConfiguration.DOT;
+ for (Map.Entry<String, String> kv : addInfo.getParams().entrySet()) {
+ if (kv.getValue() == null) {
Review Comment:
Didn't make sense to me to treat them differently, so I unified them with adding an empty check to the addQueue method
--
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] K0K0V0K commented on pull request #5115: YARN-10005. Code improvements in MutableCSConfigurationProvider
Posted by GitBox <gi...@apache.org>.
K0K0V0K commented on PR #5115:
URL: https://github.com/apache/hadoop/pull/5115#issuecomment-1311981779
Thanks @p-szucs this seems good to me :)
--
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