[
https://issues.apache.org/jira/browse/YARN-10254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101749#comment-17101749
]
Peter Bacsko commented on YARN-10254:
-------------------------------------
Thanks for the fix [~shuzirra]. Some comments:
#1
{noformat}
private ApplicationPlacementContext getContextForGroup(
String group,
QueueMapping mapping) throws IOException {
return getPlacementContext(mapping, group);
}
{noformat}
This method is so tiny, do we need this? It's called twice from
{{getPlacementForUser()}}.
#2
{noformat}
private ApplicationPlacementContext getPlacementContextWithParent(
QueueMapping mapping,
String leafQueueName) {
CSQueue parent = queueManager.getQueue(mapping.getParentQueue());
//we don't find the specified parent, so the placement rule is invalid
//for this case
if (parent == null) {
return null;
}
{noformat}
Here, if the parent {{CSQueue}} object doesn't exist, we return null. AFAIK the
application will be placed into "root.default" then. Shouldn't we throw an
exception instead?
#3
{{if ( groupQueue != null) {}}
Nit: unnecessary whitespace
#4
For readability purposes, the method {{private ApplicationPlacementContext
getPlacementContext(QueueMapping mapping, String leafQueueName) throws
IOException}} should be placed above {{getPlacementContextNoParent()}} and
{{getPlacementContextWithParent()}}.
#5
This part of the code is interesting inside {{getPlacementContextWithParent()}}:
{noformat}
if (!(parent instanceof ManagedParentQueue)) {
CSQueue queue = queueManager.getQueue(
mapping.getParentQueue() + "." + leafQueueName);
{noformat}
No matter how we define the parent in the mapping ("users.%primary_group" vs
"root.users.%primary_group"), we always rely on {{mapping.getParentQueue()}}.
But this could be either "users" or "root.users". I have a feeling that a
normalization step with {{alterMapping()}} is necessary (that step is only used
inside {{getContextForGroupParent()}}).
#6
The method name {{alterMapping()}} is not expressive enough, what about
{{normalizeMapping()}} or {{resolveMapping()}}, with a short comment like
"Creates a new mapping from the original by replacing certain values that were
not known before the original was evaluated, like username or the full path of
the parent". Just an idea.
> CapacityScheduler incorrect User Group Mapping after leaf queue change
> ----------------------------------------------------------------------
>
> Key: YARN-10254
> URL: https://issues.apache.org/jira/browse/YARN-10254
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Gergely Pollak
> Assignee: Gergely Pollak
> Priority: Major
> Attachments: YARN-10254.001.patch, YARN-10254.002.patch
>
>
> YARN-9879 and YARN-10198 introduced some major changes to user group mapping,
> and some of them unfortunately had some negative impact on the way mapping
> works.
> In some cases incorrect PlacementContexts were created, where full queue path
> was passed as leaf queue name. This affects how the yarn cli app list
> displays the queues.
> u:%user:%primary_group.%user mapping fails with an incorrect validation error
> when the %primary_group parent queue was a managed parent.
> Group based rules in certain cases are mapped to root.[primary_group] rules,
> loosing the ability to create deeper structures.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
