Apache
/
hadoop
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

YARN-10281. Redundant QueuePath usage in UserGroupMappingPlacementRule and AppNameMappingPlacementRule. Contributed by Gergely Pollak

This commit is contained in:
Szilard Nemeth 2020-06-17 14:36:08 +02:00
parent c1ef247dc6
commit ec913398a9
7 changed files with 73 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/AppNameMappingPlacementRule.java
View File

@ -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{

24
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueueMapping.java
View File

@ -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;

61
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePath.java
View File

@ -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;
}
}

31
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java
View File

@ -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 {

35
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java
View File

@ -388,7 +388,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
@ -409,7 +408,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
@ -422,7 +421,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{
@ -433,7 +432,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{
@ -455,7 +454,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 : " +
@ -466,7 +465,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 "
@ -482,29 +481,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();
}

4
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
View File

@ -1055,7 +1055,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);
}
@ -1131,7 +1131,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(

27
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
View File

@ -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