hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sunil G (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (YARN-3955) Support for priority ACLs in CapacityScheduler
Date Thu, 05 Jan 2017 17:25:58 GMT

     [ https://issues.apache.org/jira/browse/YARN-3955?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Sunil G updated YARN-3955:
--------------------------
    Attachment: YARN-3955.0008.patch

Thanks [~leftnoteasy] for the detailed comments. I have some more doubts here.

1) Common logic of checkAccess / getDefaultPriority can be merged further: both can get approvedPriority
first.
>> priority acls are stored in ascending order. So for checkAccess, we need to see whether
ACL match or not and then submitted priority is lesser than configure priority. However in
case there are no configurations for priority ACLs or ACLs are disabled, we still need to
say access check is passed. Now for default priority, we will loop through all configured
priority acls and if any ACLs are matching, we try to get max priority all group from which
default could be taken.
 Do you mean that below methods also can be made common.

{noformat}
    if (!isACLsEnable) {
      return true;
    }
    List<PriorityACL> acls = allAcls.get(queueName);
    if (acls == null || acls.isEmpty()) {
      return true;
    }
{noformat}

There is one issue here. If approvedPriorityACL comes are null, for checkAccess it means false.
If we put above code also inside {{getPriorityPerUserACL}}, then we expect to return true
if that returns null. Since there is conflict of interest, i pulled it out. May be you could
explain a bit further if I missed some.

2) As I commented above, do changes of capacity-scheduler.xml related to the patch? I cannot
find which module uses acl_access_priority in configuration. If not, could you add correct
default value?
>> in {{CapacitySchedulerConfiguration.getAclKey(AccessType acl)}}, we try to get priority
ACL config from  acl_access_priority . And that is used to parse and then populate internal
structures. By default I kept it *, but I have given an example as below.
{noformat}
The ACL of who can submit applications with configured priority.
For e.g, [user={name} group={name} max_priority={priority} default_priority={priority}]
{noformat}

3) CapacityScheduler:
* updateApplicationPriority should hold writeLock?
* similiarily, checkAndGetApplicationPriority should hold readlock?
>> Done. Updated in patch

* checkAndGetApplicationPriority: when an app's priority set to negative, I think we should
use 0 instead of max. Thoughts?
{noformat}
      if (appPriority.compareTo(getMaxClusterLevelAppPriority()) < 0) {
        appPriority = Priority
            .newInstance(getMaxClusterLevelAppPriority().getPriority());
      }
{noformat}
This code will reset to cluster-max priority only if submitted priority is more than cluster
max. Since I used {{compareTo}}, it not looks very readable.
Now to your point, we never worry much of -ve priority as such since we use priority as integer.
Do you feel we need to make 0 as lowest priority ?

4) AppPriorityACLsMgr:
* addPrioirityACLs, should we do "replace" instead of "add" to acl groups? If it is not intentional,
could you add a test to make sure update of acls works? (like change from [1,2,3] to [1,3,4])
 >> Could I add a clear model. It may be more easy. Thoughts? Updated patch as per this.

* getPriorityPerUserACL -> getMappedPriorityAclForUGI.
>> Done.
5) As I mentioned before, remove readlock of LQ#getPriorityAcls, final should be enough.
>> One doubt here. Since priorityAcls could also be updated in reinitialize, we can’t
make it as final rt. refreshQueue’s call flow for eg.
6) YarnScheduler: why the new added method has SettableFuture in parameters? It doesn't look
very clean ...
>> I agree with you. But we are doing statestore update within scheduler. Hence we need
to pass future to see exception is thrown immediately. Hence we had to add this while doing
move to queue.


> Support for priority ACLs in CapacityScheduler
> ----------------------------------------------
>
>                 Key: YARN-3955
>                 URL: https://issues.apache.org/jira/browse/YARN-3955
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: capacityscheduler
>            Reporter: Sunil G
>            Assignee: Sunil G
>         Attachments: ApplicationPriority-ACL.pdf, ApplicationPriority-ACLs-v2.pdf, YARN-3955.0001.patch,
YARN-3955.0002.patch, YARN-3955.0003.patch, YARN-3955.0004.patch, YARN-3955.0005.patch, YARN-3955.0006.patch,
YARN-3955.0007.patch, YARN-3955.0008.patch, YARN-3955.v0.patch, YARN-3955.v1.patch, YARN-3955.wip1.patch
>
>
> Support will be added for User-level access permission to use different application-priorities.
This is to avoid situations where all users try running max priority in the cluster and thus
degrading the value of priorities.
> Access Control Lists can be set per priority level within each queue. Below is an example
configuration that can be added in capacity scheduler configuration
> file for each Queue level.
> yarn.scheduler.capacity.root.<queue_name>.<priority>.acl=user1,user2



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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


Mime
View raw message