[jira] [Commented] (YARN-8016) Refine PlacementRule interface and add a app-name queue mapping rule as an example

["Zian Chen (JIRA)" <ji...@apache.org>]

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

Zian Chen commented on YARN-8016:
---------------------------------

Hi [~leftnoteasy] , Thank you so much for your comments, here are my thoughts after further discussion,

1) will remove readlock here.

2) initially I check rule.getQueueMappingLists() instead of rule is because we change the initialize logic inside UserGroupMappingRule and other rules to extend initialize method from PlacementRule instead of calling get() method to normalize the logic. In initialize method, rule will not become null, we only affect the mappings field of rule variable, however your suggestions also make a lot of sense, I propose we can do this,

throw Exception as long as we get mappings empty at the initialize method, which means user actually not specify any placement rules for initialize. 

 

will remove List<QueueMappingEntity> as we only want to provide general framework here, use List<QueueMappingEntity> will restrict user cannot specify dynamic placement rule mapping strategy. 

 

5) public UserGroupMappingPlacementRule(){} is the default constructor which will be hidden since we have constructor with parameters inside UserGroupMappingPlacementRule class. I could modify it to call 

public UserGroupMappingPlacementRule(boolean overrideWithQueueMappings,
List<QueueMapping> newMappings, Groups groups)
{code:java}
public UserGroupMappingPlacementRule(){}
{code}
Is not necessary.

6) {{validateParentQueue}} rename it to something like {{validateQueueMappingUnderParentQueue}} and place it under {{QueuePlacementRuleUtils}}?

 

9) Agree. There is only one thing I want to suggest here is, we should probably check if placementRuleStrs have duplicate placement rule first, if yes, we can simply fail the case as this is more like user input error. Then add UserGroupMappingPlacementRule if absent, then for loop it.

 

Agree with all the other comments. will change them according to the suggestions.

 

Thanks!

 

> Refine PlacementRule interface and add a app-name queue mapping rule as an example
> ----------------------------------------------------------------------------------
>
>                 Key: YARN-8016
>                 URL: https://issues.apache.org/jira/browse/YARN-8016
>             Project: Hadoop YARN
>          Issue Type: Task
>            Reporter: Zian Chen
>            Assignee: Zian Chen
>            Priority: Major
>         Attachments: YARN-8016.001.patch
>
>
> After YARN-3635/YARN-6689, PlacementRule becomes a common interface which can be used by scheduler and can be dynamically updated by scheduler according to configs. There're some other works. 
> - There's no way to initialize PlacementRule.
> - No example of PlacementRule except the user-group mapping one.
> This JIRA is targeted to refine PlacementRule interfaces and add another PlacementRule example.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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