[ 
https://issues.apache.org/jira/browse/YARN-9841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16938291#comment-16938291
 ] 

Peter Bacsko commented on YARN-9841:
------------------------------------

Just some really minor comments:

{noformat}
70          if (expectedParentQueue != null) {
71            Assert.assertEquals(expectedParentQueue,
72                ctx != null ? ctx.getParentQueue() : inputQueue);
73          }
{noformat}

Could you add an assertion message here?


{noformat}
187         Assert.assertEquals("a", ctx != null ? ctx.getQueue() : "default");
188         Assert.assertEquals("agroup",
189             ctx != null ? ctx.getParentQueue() : "default");
190       }
{noformat}

Three things here:
1. Assertion message "a" is very short, I think it should be like "Expected 
queue".
2. Similaly, "Expected group" instead of "agroup"
3. Can {{ctx}} ever be null? I assume this test should produce the same 
behavior each time, provided the code-under-test doesn't change. So to me it 
seems more logical to make sure that {{ctx}} is not null, which  practically 
means a new assertion. Btw this applies to the piece of code above, too.

> Capacity scheduler: add support for combined %user + %primary_group mapping
> ---------------------------------------------------------------------------
>
>                 Key: YARN-9841
>                 URL: https://issues.apache.org/jira/browse/YARN-9841
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: capacity scheduler
>            Reporter: Peter Bacsko
>            Assignee: Manikandan R
>            Priority: Major
>         Attachments: YARN-9841.001.patch, YARN-9841.001.patch, 
> YARN-9841.junit.patch
>
>
> Right now in CS, using {{%primary_group}} with a parent queue is only 
> possible this way:
> {{u:%user:parentqueue.%primary_group}}
> Looking at 
> https://github.com/apache/hadoop/blob/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java,
>  we cannot do something like:
> {{u:%user:%primary_group.%user}}
> Fair Scheduler supports a nested rule where such a placement/mapping rule is 
> possible. This improvement would reduce this feature gap.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to