[
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: [email protected]
For additional commands, e-mail: [email protected]
