[
https://issues.apache.org/jira/browse/YARN-3955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15652791#comment-15652791
]
Jian He commented on YARN-3955:
-------------------------------
thanks Sunil, some comments for the patch:
- PriorityACLConfiguration#getPrioirityAcl typo
- I think we may not need a public API record PriorityACL as it's not supposed
to be used by external application. May use AccessType#SUBMIT_APP_PRIORITY
directly
- AccessType#SUBMIT_APP_PRIORITY, maybe call it ACCESS_PRIORITY ?
- make the log message clear that "user does not have permission to change
priority" ?
{code}
RMAuditLogger.logFailure(callerUGI.getShortUserName(),
AuditConstants.UPDATE_APP_PRIORITY,
"User doesn't have permissions to "
+ ApplicationAccessType.MODIFY_APP.toString(),
"ClientRMService", AuditConstants.UNAUTHORIZED_USER, applicationId);
throw RPCUtil.getRemoteException(new AccessControlException("User "
+ callerUGI.getShortUserName() + " cannot perform operation "
+ ApplicationAccessType.MODIFY_APP.name() + " on " + applicationId));
{code}
- I think readLock is not needed, the field itself is not changing.
{code}
try {
readLock.lock();
return priorityAcls;
} finally {
readLock.unlock()
{code}
- could you add sample configurations in capacity-scheduler.xml, and add
comments about what the syntax is for the config ?
- the priority comes from submissionContext can be an arbitrary value, it's not
guarated that "csqueue.getPriorityPrivilegedEntity(priority)" will always
return the object?
{code}
Priority priority = submissionContext.getPriority();
if (null != csqueue) {
if ((!authorizer.checkPermission(
new AccessRequest(csqueue.getPrivilegedEntity(), userUgi,
SchedulerUtils.toAccessType(QueueACL.SUBMIT_APPLICATIONS),
applicationId.toString(), appName,
Server.getRemoteAddress(), null))
&& !authorizer.checkPermission(
new AccessRequest(csqueue.getPrivilegedEntity(), userUgi,
SchedulerUtils.toAccessType(QueueACL.ADMINISTER_QUEUE),
applicationId.toString(), appName,
Server.getRemoteAddress(), null)))
|| !authorizer.checkPermission(new AccessRequest(
csqueue.getPriorityPrivilegedEntity(priority), userUgi,
{code}
- I feel, adding every possible priority at every integer for a queue into the
ConfiguredYarnAuthorizer is cumbersome. One solution in my mind is to create a
new QueueEntity which overridds PrivilegedEntity, and we add a max-priority
field in the new object. On checkPermission, we check whether the accessType is
accessPriority, if it is, check if the priority is less than the max-priority.
AccessRequest also needs to add a new priority field.
> 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
> Affects Versions: 2.7.1
> Reporter: Sunil G
> Assignee: Sunil G
> Attachments: ApplicationPriority-ACL.pdf,
> ApplicationPriority-ACLs-v2.pdf, 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]
