[ https://issues.apache.org/jira/browse/YARN-3955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15752985#comment-15752985 ]
Wangda Tan commented on YARN-3955: ---------------------------------- Thanks [~sunilg]. Besides UT failures, findbugs, etc. Some comments: 1) AppPriorityACLsManager - {{private class PriorityACL}} add static - Why put it to RM(Active)Context? We have checkAndGetApplicationPriority exposed by scheduler already. Is it possible to remove it from contexts? - setPrioirityACLsToStore, remove the store? It doesn't have logic of state store? And where is the logic to update state store? - Common logic of checkAccess / getDefaultPriority can be merged. - getDefaultPriority, I found PriorityAclGroup#getDefaultPriority is not accessed, so this looks like using max priority instead of default. Adding some UTs for this? 2) Changes to capacity-scheduler.xml - Related to this JIRA? 3) CapacityScheduler - checkAndGetApplicationPriority: priorityFromContext -> something like priorityRequestedByApp - not reuse "priorityRequestedByApp", and change the appPriority - this.appPriorityACLManager.getDefaultPriority can be called before calling queueManager.getDefaultPriorityForQueue. And getDefaultPriorityForQueue can be called only if previous one is null. 4) CSQueueManager: - move setPriorityAcls to setQueueAcls? 5) ClientRMService: Instead of call methods of appPriorityAclManager (I prefer to remove it, see #1). Can we call scheduler#checkAndGetApplicationPriority to achieve the same goal? 6) LeafQueue: - getPriorityAcls, readLock is not necessary. Add a final to priorityAcls should be enough. - remove unncessary import 7) Add comprehensive tests to PriorityAclConfiguration should be better. 8) RMAppManager: - Changes of the {{if (isRecovery && YarnConfiguration.isAclEnabled(conf) && scheduler instanceof CapacityScheduler)}} may not be necessary? > 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.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