[
https://issues.apache.org/jira/browse/YARN-3955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15802330#comment-15802330
]
Wangda Tan commented on YARN-3955:
----------------------------------
1)
bq. There is one issue here. If approvedPriorityACL comes are null, for
checkAccess it means false.
Ok gotcha, my bad, we cannot merge the two.
2)
Got it, not related to your patch. The previous design of "acl-key" is bad, it
will be hard to find which code path uses it...
And in addition, I didn't see test case that parses raw priority acls (string)
to List of PriorityACLGroup. Could you point me if there's any test cases exist?
Few renaming suggestions:
- PriorityACLConfiguration -> AppPriorityACLConfigurationParser (I was trying
to find where's the parser code, and since we're adding queue priority
YARN-5864, so it will be better to add an App- to distinguish that)
- Similiarily, PriorityAclConfig -> AppPriorityACLOwnerType (or any better
name?)
- PriorityACLGroup -> AppPriorityACLGroup
- Do you think is it better to rename acl_access_priority to
acl_app_max_priority?
3)
bq. 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.
Yeah, since we're using Priority in different ways, sometimes lower is more
important and sometimes higher is more important. Could you use ">" to do the
comparision?
bq. checkAndGetApplicationPriority: when an app's priority set to negative, I
think we should use 0 instead of max. Thoughts?
Negative value looks fine, since app can set lower priority if needed.
4)
bq. Could I add a clear model. It may be more easy. Thoughts? Updated patch as
per this.
Not quite sure what did you mean. From my understanding, existing logic read
acls from configs while refreshQueues, and what we need to do is to replace all
ACLs instead of append to previous acl list, correct?
bq. 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.
Since the returned list can be modified by another thread, so the readLock
cannot provide enough protection. The better way might be readLock + copyList.
bq. 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.
Make sense.
> 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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
