You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by sn...@apache.org on 2020/06/17 12:35:33 UTC
[hadoop] branch trunk updated: YARN-10281. Redundant QueuePath
usage in UserGroupMappingPlacementRule and AppNameMappingPlacementRule.
Contributed by Gergely Pollak
This is an automated email from the ASF dual-hosted git repository.
snemeth pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new 5b1a56f YARN-10281. Redundant QueuePath usage in UserGroupMappingPlacementRule and AppNameMappingPlacementRule. Contributed by Gergely Pollak
5b1a56f is described below
commit 5b1a56f9f1aec7d75b14a60d0c42192b04407356
Author: Szilard Nemeth <sn...@apache.org>
AuthorDate: Wed Jun 17 14:34:40 2020 +0200
YARN-10281. Redundant QueuePath usage in UserGroupMappingPlacementRule and AppNameMappingPlacementRule. Contributed by Gergely Pollak
---
.../placement/AppNameMappingPlacementRule.java | 11 ++--
.../resourcemanager/placement/QueueMapping.java | 24 +++++----
.../resourcemanager/placement/QueuePath.java | 61 ----------------------
.../placement/QueuePlacementRuleUtils.java | 31 ++++-------
.../placement/UserGroupMappingPlacementRule.java | 35 ++++++-------
.../capacity/CapacitySchedulerConfiguration.java | 4 +-
.../scheduler/capacity/TestQueueMappings.java | 27 ++++++++++
7 files changed, 73 insertions(+), 120 deletions(-)
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/AppNameMappingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/AppNameMappingPlacementRule.java
index cf725b6..63d98ba 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/AppNameMappingPlacementRule.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/AppNameMappingPlacementRule.java
@@ -86,8 +86,6 @@ public class AppNameMappingPlacementRule extends PlacementRule {
// check if mappings refer to valid queues
for (QueueMapping mapping : queueMappings) {
- QueuePath queuePath = mapping.getQueuePath();
-
if (isStaticQueueMapping(mapping)) {
//at this point mapping.getQueueName() return only the queue name, since
//the config parsing have been changed making QueueMapping more
@@ -98,7 +96,7 @@ public class AppNameMappingPlacementRule extends PlacementRule {
//Try getting queue by its full path name, if it exists it is a static
//leaf queue indeed, without any auto creation magic
- if (queueManager.isAmbiguous(queuePath.getFullPath())) {
+ if (queueManager.isAmbiguous(mapping.getFullPath())) {
throw new IOException(
"mapping contains ambiguous leaf queue reference " + mapping
.getFullPath());
@@ -110,8 +108,7 @@ public class AppNameMappingPlacementRule extends PlacementRule {
// then it should exist and
// be an instance of AutoCreateEnabledParentQueue
QueueMapping newMapping =
- validateAndGetAutoCreatedQueueMapping(queueManager, mapping,
- queuePath);
+ validateAndGetAutoCreatedQueueMapping(queueManager, mapping);
if (newMapping == null) {
throw new IOException(
"mapping contains invalid or non-leaf queue " + mapping
@@ -124,7 +121,7 @@ public class AppNameMappingPlacementRule extends PlacementRule {
// if its an instance of auto created leaf queue,
// then extract parent queue name and update queue mapping
QueueMapping newMapping = validateAndGetQueueMapping(
- queueManager, queue, mapping, queuePath);
+ queueManager, queue, mapping);
newMappings.add(newMapping);
}
} else {
@@ -135,7 +132,7 @@ public class AppNameMappingPlacementRule extends PlacementRule {
// parent queue exists and an instance of AutoCreateEnabledParentQueue
//
QueueMapping newMapping = validateAndGetAutoCreatedQueueMapping(
- queueManager, mapping, queuePath);
+ queueManager, mapping);
if (newMapping != null) {
newMappings.add(newMapping);
} else{
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueueMapping.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueueMapping.java
index 3fcb5fe..b142dd6 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueueMapping.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueueMapping.java
@@ -66,10 +66,20 @@ public class QueueMapping {
return this;
}
- public QueueMappingBuilder queuePath(QueuePath path) {
- this.queue = path.getLeafQueue();
- this.parentQueue = path.getParentQueue();
- return this;
+ public QueueMappingBuilder parsePathString(String queuePath) {
+ int parentQueueNameEndIndex = queuePath.lastIndexOf(DOT);
+
+ if (parentQueueNameEndIndex > -1) {
+ final String parentQueue =
+ queuePath.substring(0, parentQueueNameEndIndex).trim();
+ final String leafQueue =
+ queuePath.substring(parentQueueNameEndIndex + 1).trim();
+ return this
+ .parentQueue(parentQueue)
+ .queue(leafQueue);
+ }
+
+ return this.queue(queuePath);
}
public QueueMapping build() {
@@ -138,12 +148,6 @@ public class QueueMapping {
return fullPath;
}
- public QueuePath getQueuePath() {
- //This is to make sure the parsing is the same everywhere, but the
- //whole parsing part should be moved to QueuePathConstructor
- return QueuePlacementRuleUtils.extractQueuePath(getFullPath());
- }
-
@Override
public int hashCode() {
final int prime = 31;
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePath.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePath.java
deleted file mode 100644
index e02cf58..0000000
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePath.java
+++ /dev/null
@@ -1,61 +0,0 @@
-/**
- * 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.placement;
-
-import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.DOT;
-
-public class QueuePath {
-
- private String parentQueue;
- private String leafQueue;
- private String fullPath;
-
- public QueuePath(final String leafQueue) {
- //if the queue does not have a parent, the full path == leaf queue name
- this.leafQueue = leafQueue;
- this.fullPath = leafQueue;
- }
-
- public QueuePath(final String parentQueue, final String leafQueue) {
- this.parentQueue = parentQueue;
- this.leafQueue = leafQueue;
- this.fullPath = parentQueue + DOT + leafQueue;
- }
-
- public String getParentQueue() {
- return parentQueue;
- }
-
- public String getLeafQueue() {
- return leafQueue;
- }
-
- public boolean hasParentQueue() {
- return parentQueue != null;
- }
-
- public String getFullPath() {
- return fullPath;
- }
-
- @Override
- public String toString() {
- return fullPath;
- }
-}
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java
index 350f2b9..15c8fd8 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java
@@ -66,18 +66,19 @@ public final class QueuePlacementRuleUtils {
}
public static QueueMapping validateAndGetAutoCreatedQueueMapping(
- CapacitySchedulerQueueManager queueManager, QueueMapping mapping,
- QueuePath queuePath) throws IOException {
- if (queuePath.hasParentQueue()) {
+ CapacitySchedulerQueueManager queueManager, QueueMapping mapping)
+ throws IOException {
+ if (mapping.hasParentQueue()) {
//if parent queue is specified,
// then it should exist and be an instance of ManagedParentQueue
validateQueueMappingUnderParentQueue(queueManager.getQueue(
- queuePath.getParentQueue()), queuePath.getParentQueue(),
- queuePath.getFullPath());
+ mapping.getParentQueue()), mapping.getParentQueue(),
+ mapping.getFullPath());
return QueueMapping.QueueMappingBuilder.create()
.type(mapping.getType())
.source(mapping.getSource())
- .queuePath(queuePath)
+ .parentQueue(mapping.getParentQueue())
+ .queue(mapping.getQueue())
.build();
}
@@ -86,7 +87,7 @@ public final class QueuePlacementRuleUtils {
public static QueueMapping validateAndGetQueueMapping(
CapacitySchedulerQueueManager queueManager, CSQueue queue,
- QueueMapping mapping, QueuePath queuePath) throws IOException {
+ QueueMapping mapping) throws IOException {
if (!(queue instanceof LeafQueue)) {
throw new IOException(
"mapping contains invalid or non-leaf queue : " +
@@ -97,7 +98,7 @@ public final class QueuePlacementRuleUtils {
.getParent() instanceof ManagedParentQueue) {
QueueMapping newMapping = validateAndGetAutoCreatedQueueMapping(
- queueManager, mapping, queuePath);
+ queueManager, mapping);
if (newMapping == null) {
throw new IOException(
"mapping contains invalid or non-leaf queue " +
@@ -114,20 +115,6 @@ public final class QueuePlacementRuleUtils {
&& !mapping.getQueue().contains(SECONDARY_GROUP_MAPPING);
}
- public static QueuePath extractQueuePath(String queuePath) {
- int parentQueueNameEndIndex = queuePath.lastIndexOf(DOT);
-
- if (parentQueueNameEndIndex > -1) {
- final String parentQueue = queuePath.substring(0, parentQueueNameEndIndex)
- .trim();
- final String leafQueue = queuePath.substring(parentQueueNameEndIndex + 1)
- .trim();
- return new QueuePath(parentQueue, leafQueue);
- }
-
- return new QueuePath(queuePath);
- }
-
public static ApplicationPlacementContext getPlacementContext(
QueueMapping mapping, CapacitySchedulerQueueManager queueManager)
throws IOException {
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java
index 643fc50..0e8cb9c 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java
@@ -386,7 +386,6 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
//at this point mapping.getQueueName() return only the queue name, since
//the config parsing have been changed making QueueMapping more consistent
- QueuePath queuePath = mapping.getQueuePath();
if (isStaticQueueMapping(mapping)) {
//Try getting queue by its full path name, if it exists it is a static
//leaf queue indeed, without any auto creation magic
@@ -407,7 +406,7 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
// then it should exist and
// be an instance of AutoCreateEnabledParentQueue
QueueMapping newMapping = validateAndGetAutoCreatedQueueMapping(
- queueManager, mapping, queuePath);
+ queueManager, mapping);
if (newMapping == null) {
throw new IOException(
"mapping contains invalid or non-leaf queue " + mapping
@@ -420,7 +419,7 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
// if its an instance of auto created leaf queue,
// then extract parent queue name and update queue mapping
QueueMapping newMapping = validateAndGetQueueMapping(queueManager,
- queue, mapping, queuePath);
+ queue, mapping);
newMappings.add(newMapping);
}
} else{
@@ -431,7 +430,7 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
// parent queue exists and an instance of AutoCreateEnabledParentQueue
//
QueueMapping newMapping = validateAndGetAutoCreatedQueueMapping(
- queueManager, mapping, queuePath);
+ queueManager, mapping);
if (newMapping != null) {
newMappings.add(newMapping);
} else{
@@ -453,7 +452,7 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
private static QueueMapping validateAndGetQueueMapping(
CapacitySchedulerQueueManager queueManager, CSQueue queue,
- QueueMapping mapping, QueuePath queuePath) throws IOException {
+ QueueMapping mapping) throws IOException {
if (!(queue instanceof LeafQueue)) {
throw new IOException(
"mapping contains invalid or non-leaf queue : " +
@@ -464,7 +463,7 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
.getParent() instanceof ManagedParentQueue) {
QueueMapping newMapping = validateAndGetAutoCreatedQueueMapping(
- queueManager, mapping, queuePath);
+ queueManager, mapping);
if (newMapping == null) {
throw new IOException(
"mapping contains invalid or non-leaf queue "
@@ -480,29 +479,29 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
}
private static QueueMapping validateAndGetAutoCreatedQueueMapping(
- CapacitySchedulerQueueManager queueManager, QueueMapping mapping,
- QueuePath queuePath) throws IOException {
- if (queuePath.hasParentQueue()
- && (queuePath.getParentQueue().equals(PRIMARY_GROUP_MAPPING)
- || queuePath.getParentQueue().equals(SECONDARY_GROUP_MAPPING))) {
+ CapacitySchedulerQueueManager queueManager, QueueMapping mapping)
+ throws IOException {
+ if (mapping.hasParentQueue()
+ && (mapping.getParentQueue().equals(PRIMARY_GROUP_MAPPING)
+ || mapping.getParentQueue().equals(SECONDARY_GROUP_MAPPING))) {
// dynamic parent queue
return QueueMappingBuilder.create()
.type(mapping.getType())
.source(mapping.getSource())
- .queue(queuePath.getLeafQueue())
- .parentQueue(queuePath.getParentQueue())
+ .queue(mapping.getQueue())
+ .parentQueue(mapping.getParentQueue())
.build();
- } else if (queuePath.hasParentQueue()) {
+ } else if (mapping.hasParentQueue()) {
//if parent queue is specified,
// then it should exist and be an instance of ManagedParentQueue
QueuePlacementRuleUtils.validateQueueMappingUnderParentQueue(
- queueManager.getQueue(queuePath.getParentQueue()),
- queuePath.getParentQueue(), queuePath.getLeafQueue());
+ queueManager.getQueue(mapping.getParentQueue()),
+ mapping.getParentQueue(), mapping.getQueue());
return QueueMappingBuilder.create()
.type(mapping.getType())
.source(mapping.getSource())
- .queue(queuePath.getLeafQueue())
- .parentQueue(queuePath.getParentQueue())
+ .queue(mapping.getQueue())
+ .parentQueue(mapping.getParentQueue())
.build();
}
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java
index 7f4150f..496dd0b 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java
@@ -1060,7 +1060,7 @@ public class CapacitySchedulerConfiguration extends ReservationSchedulerConfigur
QueueMapping m = QueueMapping.QueueMappingBuilder.create()
.type(QueueMapping.MappingType.APPLICATION)
.source(mapping[0])
- .queuePath(QueuePlacementRuleUtils.extractQueuePath(mapping[1]))
+ .parsePathString(mapping[1])
.build();
mappings.add(m);
}
@@ -1136,7 +1136,7 @@ public class CapacitySchedulerConfiguration extends ReservationSchedulerConfigur
m = QueueMappingBuilder.create()
.type(mappingType)
.source(mapping[1])
- .queuePath(QueuePlacementRuleUtils.extractQueuePath(mapping[2]))
+ .parsePathString(mapping[2])
.build();
} catch (Throwable t) {
throw new IllegalArgumentException(
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestQueueMappings.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestQueueMappings.java
index 2e7009e..039b9da 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestQueueMappings.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestQueueMappings.java
@@ -100,6 +100,33 @@ public class TestQueueMappings {
.build());
}
+ @Test
+ public void testQueueMappingPathParsing() {
+ QueueMapping leafOnly = QueueMapping.QueueMappingBuilder.create()
+ .parsePathString("leaf")
+ .build();
+
+ Assert.assertEquals("leaf", leafOnly.getQueue());
+ Assert.assertEquals(null, leafOnly.getParentQueue());
+ Assert.assertEquals("leaf", leafOnly.getFullPath());
+
+ QueueMapping twoLevels = QueueMapping.QueueMappingBuilder.create()
+ .parsePathString("root.leaf")
+ .build();
+
+ Assert.assertEquals("leaf", twoLevels.getQueue());
+ Assert.assertEquals("root", twoLevels.getParentQueue());
+ Assert.assertEquals("root.leaf", twoLevels.getFullPath());
+
+ QueueMapping deep = QueueMapping.QueueMappingBuilder.create()
+ .parsePathString("root.a.b.c.d.e.leaf")
+ .build();
+
+ Assert.assertEquals("leaf", deep.getQueue());
+ Assert.assertEquals("root.a.b.c.d.e", deep.getParentQueue());
+ Assert.assertEquals("root.a.b.c.d.e.leaf", deep.getFullPath());
+ }
+
@Test (timeout = 60000)
public void testQueueMappingParsingInvalidCases() throws Exception {
// configuration parsing tests - negative test cases
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org