[
https://issues.apache.org/jira/browse/YARN-10659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17300122#comment-17300122
]
Andras Gyori commented on YARN-10659:
-------------------------------------
Thank you [~shuzirra] for the patch. The overall logic looks good to me, I like
this approach. My additions:
* In VariableContext, I think this is a redundant null check:
{code:java}
else if (conditionalVariables.containsKey(parts[i])) {
MappingRuleConditionalVariable condVariable =
conditionalVariables.get(parts[i]);
if (condVariable != null) { // already checked because map contains key
newVal = condVariable.evaluateInPath(parts, i);
}
{code}
* MappingRuleConditionalVariable#evaluateInPath should be an abstract method,
or the abstract class could be an interface instead
* evaluateInPath should be documented, I had to look up the arguments to
determine what they are
* It might be a good idea to construct the prefix outside of evaluateInPath,
and only send a string prefix to evaluateInPath. Even more so, when you need to
implement an other conditional variable (where you would need to repeat this
logic). I see, that it is less flexible, but currently there is only one
conditional variable that needs this.
> Improve CS MappingRule %secondary_group evaluation
> --------------------------------------------------
>
> Key: YARN-10659
> URL: https://issues.apache.org/jira/browse/YARN-10659
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Gergely Pollak
> Assignee: Gergely Pollak
> Priority: Major
> Attachments: YARN-10659.001.patch
>
>
> Since the leaf queue names are not unique, there are a lot of use cases where
> %secondary_group evaluation fail, or behave inconsistently.
> We should extend it's behavior, when it's under a defined parent,
> %secondary_group evaluation should only check for queue existence under that
> queue. Egy root.group.%secondary_group, should only evaluate to groups which
> exist under root.group, while the legacy %secondary_group.%user should still
> look for groups by their leaf name globally.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
