This is an automated email from the ASF dual-hosted git repository.
pbacsko 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 4383726 YARN-10635. CSMapping rule can return paths with empty parts.
Contributed by Gergely Pollak.
4383726 is described below
commit 4383726d194f7310c12d928aacd2de44e0711639
Author: Peter Bacsko <pbac...@cloudera.com>
AuthorDate: Fri Feb 19 12:01:31 2021 +0100
YARN-10635. CSMapping rule can return paths with empty parts. Contributed
by Gergely Pollak.
---
.../placement/CSMappingPlacementRule.java | 33 +++++++++++--
.../placement/MappingQueuePath.java | 15 ++++++
.../placement/MappingRuleActions.java | 6 ++-
.../placement/TestCSMappingPlacementRule.java | 55 ++++++++++++++++------
4 files changed, 89 insertions(+), 20 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/CSMappingPlacementRule.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java
index 28d2cb6..821d055 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/CSMappingPlacementRule.java
@@ -194,6 +194,7 @@ public class CSMappingPlacementRule extends PlacementRule {
String secondaryGroup = null;
Iterator<String> it = groupsSet.iterator();
String primaryGroup = it.next();
+
while (it.hasNext()) {
String group = it.next();
if (this.queueManager.getQueue(group) != null) {
@@ -203,8 +204,7 @@ public class CSMappingPlacementRule extends PlacementRule {
}
if (secondaryGroup == null && LOG.isDebugEnabled()) {
- LOG.debug("User {} is not associated with any Secondary " +
- "Group. Hence it may use the 'default' queue", user);
+ LOG.debug("User {} is not associated with any Secondary group", user);
}
vctx.put("%primary_group", primaryGroup);
@@ -223,7 +223,15 @@ public class CSMappingPlacementRule extends PlacementRule {
//To place queues specifically to default, users must use root.default
if (!asc.getQueue().equals(YarnConfiguration.DEFAULT_QUEUE_NAME)) {
vctx.put("%specified", asc.getQueue());
+ } else {
+ //Adding specified as empty will prevent it to be undefined and it won't
+ //try to place the application to a queue named '%specified', queue path
+ //validation will reject the empty path or the path with empty parts,
+ //so we sill still hit the fallback action of this rule if no queue
+ //is specified
+ vctx.put("%specified", "");
}
+
vctx.put("%application", asc.getApplicationName());
vctx.put("%default", "root.default");
try {
@@ -239,6 +247,12 @@ public class CSMappingPlacementRule extends PlacementRule {
private String validateAndNormalizeQueue(
String queueName, boolean allowCreate) throws YarnException {
MappingQueuePath path = new MappingQueuePath(queueName);
+
+ if (path.hasEmptyPart()) {
+ throw new YarnException("Invalid path returned by rule: '" +
+ queueName + "'");
+ }
+
String leaf = path.getLeafName();
String parent = path.getParent();
@@ -335,14 +349,19 @@ public class CSMappingPlacementRule extends PlacementRule
{
MappingRule rule, VariableContext variables) {
MappingRuleResult result = rule.evaluate(variables);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Evaluated rule '{}' with result: '{}'", rule, result);
+ }
+
if (result.getResult() == MappingRuleResultType.PLACE) {
try {
result.updateNormalizedQueue(validateAndNormalizeQueue(
result.getQueue(), result.isCreateAllowed()));
} catch (Exception e) {
- LOG.info("Cannot place to queue '{}' returned by mapping rule. " +
- "Reason: {}", result.getQueue(), e.getMessage());
result = rule.getFallback();
+ LOG.info("Cannot place to queue '{}' returned by mapping rule. " +
+ "Reason: '{}' Fallback operation: '{}'",
+ result.getQueue(), e.getMessage(), result);
}
}
@@ -451,6 +470,12 @@ public class CSMappingPlacementRule extends PlacementRule {
}
}
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Placement final result '{}' for application '{}'",
+ (ret == null ? "null" : ret.getFullQueuePath()),
+ asc.getApplicationId());
+ }
+
return ret;
}
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/MappingQueuePath.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingQueuePath.java
index 2c01821..b1ccf70 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingQueuePath.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingQueuePath.java
@@ -70,6 +70,21 @@ public class MappingQueuePath {
}
/**
+ * Simple helper method to determine if the path contains any empty parts.
+ * @return true if there is at least one empty part of the path
+ */
+ public boolean hasEmptyPart() {
+ String[] parts = getFullPath().split("\\.");
+ for (int i = 0; i < parts.length; i++) {
+ if (parts[i].equals("")) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+ /**
* Getter for the parent part of the path.
* @return Parent path of the queue, null if there is no parent.
*/
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/MappingRuleActions.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingRuleActions.java
index 35d7276..3f19dcd 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingRuleActions.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/MappingRuleActions.java
@@ -66,13 +66,15 @@ public final class MappingRuleActions {
* This method is the main logic of the action, it will replace all the
* variables in the queuePattern with their respective values, then returns
* a placementResult with the final queue name.
+ *
* @param variables The variable context, which contains all the variables
* @return The result of the action
*/
@Override
public MappingRuleResult execute(VariableContext variables) {
- String substituted = variables.replacePathVariables(queuePattern);
- return MappingRuleResult.createPlacementResult(substituted, allowCreate);
+ String substituted = variables.replacePathVariables(queuePattern);
+ return MappingRuleResult.createPlacementResult(
+ substituted, allowCreate);
}
/**
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestCSMappingPlacementRule.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestCSMappingPlacementRule.java
index 69f56ec..f0b19c4 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestCSMappingPlacementRule.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestCSMappingPlacementRule.java
@@ -64,7 +64,7 @@ public class TestCSMappingPlacementRule {
"alice", ImmutableSet.of("p_alice", "user", "developer"),
"bob", ImmutableSet.of("p_bob", "user", "developer"),
"charlie", ImmutableSet.of("p_charlie", "user", "tester"),
- "dave", ImmutableSet.of("user", "tester"),
+ "dave", ImmutableSet.of("user"),
"emily", ImmutableSet.of("user", "tester", "developer")
);
@@ -90,6 +90,7 @@ public class TestCSMappingPlacementRule {
.withQueue("root.disambiguous.deep.disambiuser.disambi")
.withQueue("root.disambiguous.deep.group.developer")
.withManagedParentQueue("root.disambiguous.deep.dman")
+ .withDynamicParentQueue("root.dynamic")
.build();
when(queueManager.getQueue(isNull())).thenReturn(null);
@@ -151,8 +152,9 @@ public class TestCSMappingPlacementRule {
private void assertReject(String message, CSMappingPlacementRule engine,
ApplicationSubmissionContext asc, String user) {
try {
- engine.getPlacementForApp(asc, user);
- fail(message);
+ ApplicationPlacementContext apc = engine.getPlacementForApp(asc, user);
+ fail("Unexpected queue result: " + apc.getFullQueuePath() + " - " +
+ message);
} catch (YarnException e) {
//To prevent PlacementRule chaining present in PlacementManager
//when an application is rejected an exception is thrown to make sure
@@ -483,14 +485,36 @@ public class TestCSMappingPlacementRule {
}
@Test
- public void testGroupMatching() throws IOException {
+ public void testGroupTargetMatching() throws IOException {
ArrayList<MappingRule> rules = new ArrayList<>();
- rules.add(createGroupMapping("p_alice", "root.man.p_alice"));
- rules.add(createGroupMapping("developer", "root.man.developer"));
+ rules.add(
+ new MappingRule(
+ MappingRuleMatchers.createUserMatcher("alice"),
+ (new MappingRuleActions.PlaceToQueueAction(
+ "root.man.%primary_group", true))
+ .setFallbackReject()));
+
+ rules.add(
+ new MappingRule(
+ MappingRuleMatchers.createUserMatcher("bob"),
+ (new MappingRuleActions.PlaceToQueueAction(
+ "root.dynamic.%secondary_group.%user", true))
+ .setFallbackReject()));
+
+ rules.add(
+ new MappingRule(
+ MappingRuleMatchers.createUserMatcher("charlie"),
+ (new MappingRuleActions.PlaceToQueueAction(
+ "root.man.%secondary_group", true))
+ .setFallbackReject()));
- //everybody is in the user group, this should catch all
- rules.add(createGroupMapping("user", "root.man.user"));
+ rules.add(
+ new MappingRule(
+ MappingRuleMatchers.createUserMatcher("dave"),
+ (new MappingRuleActions.PlaceToQueueAction(
+ "root.dynamic.%secondary_group.%user", true))
+ .setFallbackReject()));
CSMappingPlacementRule engine = setupEngine(true, rules);
ApplicationSubmissionContext app = createApp("app");
@@ -499,12 +523,15 @@ public class TestCSMappingPlacementRule {
"Alice should be placed to root.man.p_alice based on her primary
group",
engine, app, "alice", "root.man.p_alice");
assertPlace(
- "Bob should be placed to root.man.developer based on his developer " +
- "group", engine, app, "bob", "root.man.developer");
- assertPlace(
- "Charlie should be placed to root.man.user because he is not a " +
- "developer nor in the p_alice group", engine, app, "charlie",
- "root.man.user");
+ "Bob should be placed to root.dynamic.developer.bob based on his " +
+ "secondary group, since we have a queue named 'developer', bob " +
+ "identifies as a user with secondary_group 'developer'", engine, app,
+ "bob", "root.dynamic.developer.bob");
+ assertReject("Charlie should get rejected because he neither of his" +
+ "groups have an ambiguous queue, so effectively he has no secondary " +
+ "group", engine, app, "charlie");
+ assertReject("Dave should get rejected because he has no secondary group",
+ engine, app, "dave");
}
void assertConfigTestResult(List<MappingRule> rules) {
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
