Skip to content

Commit f65f64e

Browse files
YARN-10254. CapacityScheduler incorrect User Group Mapping after leaf queue change. Contributed by Gergely Pollak
1 parent 910d88e commit f65f64e

File tree

3 files changed

+213
-105
lines changed

3 files changed

+213
-105
lines changed

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

Lines changed: 152 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -105,35 +105,76 @@ private ApplicationPlacementContext getPlacementForUser(String user)
105105
if (mapping.getParentQueue() != null
106106
&& mapping.getParentQueue().equals(PRIMARY_GROUP_MAPPING)
107107
&& mapping.getQueue().equals(CURRENT_USER_MAPPING)) {
108+
if (LOG.isDebugEnabled()) {
109+
LOG.debug("Creating placement context for user {} using " +
110+
"primary group current user mapping", user);
111+
}
108112
return getContextForGroupParent(user, mapping,
109113
getPrimaryGroup(user));
110114
} else if (mapping.getParentQueue() != null
111115
&& mapping.getParentQueue().equals(SECONDARY_GROUP_MAPPING)
112116
&& mapping.getQueue().equals(CURRENT_USER_MAPPING)) {
117+
if (LOG.isDebugEnabled()) {
118+
LOG.debug("Creating placement context for user {} using " +
119+
"secondary group current user mapping", user);
120+
}
113121
return getContextForGroupParent(user, mapping,
114122
getSecondaryGroup(user));
115123
} else if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) {
124+
if (LOG.isDebugEnabled()) {
125+
LOG.debug("Creating placement context for user {} using " +
126+
"current user mapping", user);
127+
}
116128
return getPlacementContext(mapping, user);
117129
} else if (mapping.getQueue().equals(PRIMARY_GROUP_MAPPING)) {
118-
return getContextForPrimaryGroup(user, mapping);
130+
if (LOG.isDebugEnabled()) {
131+
LOG.debug("Creating placement context for user {} using " +
132+
"primary group mapping", user);
133+
}
134+
return getPlacementContext(mapping, getPrimaryGroup(user));
119135
} else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) {
120-
return getContextForSecondaryGroup(user, mapping);
136+
if (LOG.isDebugEnabled()) {
137+
LOG.debug("Creating placement context for user {} using " +
138+
"secondary group mapping", user);
139+
}
140+
return getPlacementContext(mapping, getSecondaryGroup(user));
121141
} else {
142+
if (LOG.isDebugEnabled()) {
143+
LOG.debug("Creating placement context for user {} using " +
144+
"static user static mapping", user);
145+
}
122146
return getPlacementContext(mapping);
123147
}
124148
}
125149

126150
if (user.equals(mapping.getSource())) {
127151
if (mapping.getQueue().equals(PRIMARY_GROUP_MAPPING)) {
152+
if (LOG.isDebugEnabled()) {
153+
LOG.debug("Creating placement context for user {} using " +
154+
"static user primary group mapping", user);
155+
}
128156
return getPlacementContext(mapping, getPrimaryGroup(user));
129157
} else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) {
130158
String secondaryGroup = getSecondaryGroup(user);
131159
if (secondaryGroup != null) {
160+
if (LOG.isDebugEnabled()) {
161+
LOG.debug("Creating placement context for user {} using " +
162+
"static user secondary group mapping", user);
163+
}
132164
return getPlacementContext(mapping, secondaryGroup);
133165
} else {
166+
if (LOG.isDebugEnabled()) {
167+
LOG.debug("Wanted to create placement context for user {}" +
168+
" using static user secondary group mapping," +
169+
" but user has no secondary group!", user);
170+
}
134171
return null;
135172
}
136173
} else {
174+
if (LOG.isDebugEnabled()) {
175+
LOG.debug("Creating placement context for user {} using " +
176+
"static user static mapping", user);
177+
}
137178
return getPlacementContext(mapping);
138179
}
139180
}
@@ -142,8 +183,16 @@ private ApplicationPlacementContext getPlacementForUser(String user)
142183
for (String userGroups : groups.getGroups(user)) {
143184
if (userGroups.equals(mapping.getSource())) {
144185
if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) {
186+
if (LOG.isDebugEnabled()) {
187+
LOG.debug("Creating placement context for user {} using " +
188+
"static group current user mapping", user);
189+
}
145190
return getPlacementContext(mapping, user);
146191
}
192+
if (LOG.isDebugEnabled()) {
193+
LOG.debug("Creating placement context for user {} using " +
194+
"static group static mapping", user);
195+
}
147196
return getPlacementContext(mapping);
148197
}
149198
}
@@ -152,49 +201,23 @@ private ApplicationPlacementContext getPlacementForUser(String user)
152201
return null;
153202
}
154203

155-
// invoked for mappings:
156-
// u:%user:[parent].%primary_group
157-
// u:%user:%primary_group
158-
private ApplicationPlacementContext getContextForPrimaryGroup(
159-
String user,
160-
QueueMapping mapping) throws IOException {
161-
String group =
162-
CapacitySchedulerConfiguration.ROOT + "." + getPrimaryGroup(user);
163-
164-
String parent = mapping.getParentQueue();
165-
CSQueue groupQueue = queueManager.getQueue(group);
166-
167-
if (parent != null) {
168-
CSQueue parentQueue = queueManager.getQueue(parent);
169-
170-
if (parentQueue instanceof ManagedParentQueue) {
171-
return getPlacementContext(mapping, group);
172-
} else {
173-
return groupQueue == null ? null : getPlacementContext(mapping, group);
174-
}
175-
} else {
176-
return groupQueue == null ? null : getPlacementContext(mapping, group);
177-
}
178-
}
179-
180-
// invoked for mappings
181-
// u:%user:%secondary_group
182-
// u:%user:[parent].%secondary_group
183-
private ApplicationPlacementContext getContextForSecondaryGroup(
184-
String user,
185-
QueueMapping mapping) throws IOException {
186-
String secondaryGroup = getSecondaryGroup(user);
187-
188-
if (secondaryGroup != null) {
189-
CSQueue queue = this.queueManager.getQueue(secondaryGroup);
190-
if ( queue != null) {
191-
return getPlacementContext(mapping, queue.getQueuePath());
192-
} else {
193-
return null;
194-
}
195-
} else {
196-
return null;
197-
}
204+
/**
205+
* This convenience method allows to change the parent path or a leafName in
206+
* a mapping object, by creating a new one, using the builder and copying the
207+
* rest of the parameters.
208+
* @param mapping The mapping to be changed
209+
* @param parentPath The new parentPath of the mapping
210+
* @param leafName The new leafQueueName of the mapping
211+
* @return The updated NEW mapping
212+
*/
213+
private QueueMapping alterMapping(
214+
QueueMapping mapping, String parentPath, String leafName) {
215+
return QueueMappingBuilder.create()
216+
.type(mapping.getType())
217+
.source(mapping.getSource())
218+
.queue(leafName)
219+
.parentQueue(parentPath)
220+
.build();
198221
}
199222

200223
// invoked for mappings:
@@ -205,20 +228,24 @@ private ApplicationPlacementContext getContextForGroupParent(
205228
QueueMapping mapping,
206229
String group) throws IOException {
207230

208-
if (this.queueManager.getQueue(group) != null) {
231+
CSQueue groupQueue = this.queueManager.getQueue(group);
232+
if (groupQueue != null) {
209233
// replace the group string
210-
QueueMapping resolvedGroupMapping =
211-
QueueMappingBuilder.create()
212-
.type(mapping.getType())
213-
.source(mapping.getSource())
214-
.queue(user)
215-
.parentQueue(
216-
CapacitySchedulerConfiguration.ROOT + "." +
217-
group)
218-
.build();
234+
QueueMapping resolvedGroupMapping = alterMapping(
235+
mapping,
236+
groupQueue.getQueuePath(),
237+
user);
219238
validateQueueMapping(resolvedGroupMapping);
220239
return getPlacementContext(resolvedGroupMapping, user);
221240
} else {
241+
242+
if (queueManager.isAmbiguous(group)) {
243+
LOG.info("Queue mapping rule expect group queue to exist with name {}" +
244+
" but the reference is ambiguous!", group);
245+
} else {
246+
LOG.info("Queue mapping rule expect group queue to exist with name {}" +
247+
" but it does not exist!", group);
248+
}
222249
return null;
223250
}
224251
}
@@ -247,7 +274,7 @@ public ApplicationPlacementContext getPlacementForApp(
247274
} catch (IOException ioex) {
248275
String message = "Failed to submit application " + applicationId +
249276
" submitted by user " + user + " reason: " + ioex.getMessage();
250-
throw new YarnException(message);
277+
throw new YarnException(message, ioex);
251278
}
252279
}
253280
return null;
@@ -260,21 +287,79 @@ private ApplicationPlacementContext getPlacementContext(
260287

261288
private ApplicationPlacementContext getPlacementContext(QueueMapping mapping,
262289
String leafQueueName) throws IOException {
263-
264290
//leafQueue name no longer identifies a queue uniquely checking ambiguity
265291
if (!mapping.hasParentQueue() && queueManager.isAmbiguous(leafQueueName)) {
266292
throw new IOException("mapping contains ambiguous leaf queue reference " +
267293
leafQueueName);
268294
}
269295

270296
if (!StringUtils.isEmpty(mapping.getParentQueue())) {
271-
return new ApplicationPlacementContext(leafQueueName,
272-
mapping.getParentQueue());
273-
} else{
274-
return new ApplicationPlacementContext(leafQueueName);
297+
return getPlacementContextWithParent(mapping, leafQueueName);
298+
} else {
299+
return getPlacementContextNoParent(leafQueueName);
275300
}
276301
}
277302

303+
private ApplicationPlacementContext getPlacementContextWithParent(
304+
QueueMapping mapping,
305+
String leafQueueName) {
306+
CSQueue parent = queueManager.getQueue(mapping.getParentQueue());
307+
//we don't find the specified parent, so the placement rule is invalid
308+
//for this case
309+
if (parent == null) {
310+
if (queueManager.isAmbiguous(mapping.getParentQueue())) {
311+
LOG.warn("Placement rule specified a parent queue {}, but it is" +
312+
"ambiguous.", mapping.getParentQueue());
313+
} else {
314+
LOG.warn("Placement rule specified a parent queue {}, but it does" +
315+
"not exist.", mapping.getParentQueue());
316+
}
317+
return null;
318+
}
319+
320+
String parentPath = parent.getQueuePath();
321+
322+
//if we have a parent which is not a managed parent, we check if the leaf
323+
//queue exists under this parent
324+
if (!(parent instanceof ManagedParentQueue)) {
325+
CSQueue queue = queueManager.getQueue(
326+
parentPath + "." + leafQueueName);
327+
//if the queue doesn't exit we return null
328+
if (queue == null) {
329+
LOG.warn("Placement rule specified a parent queue {}, but it is" +
330+
" not a managed parent queue, and no queue exists with name {} " +
331+
"under it.", mapping.getParentQueue(), leafQueueName);
332+
return null;
333+
}
334+
}
335+
//at this point we either have a managed parent or the queue actually
336+
//exists so we have a placement context, returning it
337+
return new ApplicationPlacementContext(leafQueueName, parentPath);
338+
}
339+
340+
private ApplicationPlacementContext getPlacementContextNoParent(
341+
String leafQueueName) {
342+
//in this case we don't have a parent specified so we expect the queue to
343+
//exist, otherwise the mapping will not be valid for this case
344+
CSQueue queue = queueManager.getQueue(leafQueueName);
345+
if (queue == null) {
346+
if (queueManager.isAmbiguous(leafQueueName)) {
347+
LOG.warn("Queue {} specified in placement rule is ambiguous",
348+
leafQueueName);
349+
} else {
350+
LOG.warn("Queue {} specified in placement rule does not exist",
351+
leafQueueName);
352+
}
353+
return null;
354+
}
355+
356+
//getting parent path to make sure if the leaf name would become ambiguous
357+
//the placement context stays valid.
358+
CSQueue parent = queueManager.getQueue(leafQueueName).getParent();
359+
return new ApplicationPlacementContext(
360+
leafQueueName, parent.getQueuePath());
361+
}
362+
278363
@VisibleForTesting
279364
@Override
280365
public boolean initialize(ResourceScheduler scheduler)
@@ -449,10 +534,12 @@ private void validateQueueMapping(QueueMapping queueMapping)
449534
//as mapping.getQueue()
450535
if (leafQueue == null && queueManager.isAmbiguous(leafQueueFullName)) {
451536
throw new IOException("mapping contains ambiguous leaf queue name: "
452-
+ leafQueueFullName);
453-
} else {
454-
throw new IOException("mapping contains invalid or non-leaf queue : "
455-
+ leafQueueFullName);
537+
+ leafQueueFullName);
538+
} else if (parentQueue == null ||
539+
(!(parentQueue instanceof ManagedParentQueue))) {
540+
throw new IOException("mapping contains invalid or non-leaf queue " +
541+
" and no managed parent is found: "
542+
+ leafQueueFullName);
456543
}
457544
} else if (parentQueue == null || (!(parentQueue instanceof ParentQueue))) {
458545
throw new IOException(

0 commit comments

Comments
 (0)