[
https://issues.apache.org/jira/browse/YARN-2575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15135312#comment-15135312
]
Subru Krishnan commented on YARN-2575:
--------------------------------------
Thanks [~seanpo03] for submitting the patch. I reviewd it & please find my
comments below:
* We can remove *SchedulerUtils/AccessType* as the changes are redundant now
* In the Javadocs of *ReservationACL*, make it explicit that whoever has
created the reservations can update or delete it and administrator can modify &
list all reservations
* Rename CREATE_RESERVATIONS to SUBMIT_RESERVATIONS based on the above
comment for *ReservationACL*
* Rename _submitter_ to _reservationCreatorName_ in *ClientRMService* for
better readability
* Using a boolean array for conveying if the user has admin rights or not is
complicating the code in *ClientRMService::checkReservationACLs*, replace it
with a AtomicBoolean
* In *ClientRMService::checkReservationACLs*, first check if ACLs are enabled
or not, i.e. ACLsManager is null. If so, we can return immediately
* Nit: since *ClientRMService::checkReservationACLs* is a private method, we
can use empty as defaults and avoid the null checks to reduce the if/else checks
* In *AbstractReservationSystem*, we should instantiate ACLsManager only if
both YARN and reservations ACLs are enabled
* There is a copy-paste typo in the Javdoc of *PlanView::getReservations*
* In *CapacitySchedulerConfiguration*, _getReservationAcl/setAcl_ should be
private. Also annotate test visibility for _setReservationAcls_
* Can we maintain consistency in the order of _operator_ and
_orginalSubmitter_ in *ReservationACLsTestBase* as it's confusing now :)
* In *ReservationACLsTestBase*, can we add the scenario for admin of queue
should not be able to perform list operation on another queue
* Some copy-paste typo in both invocations and Javadoc for _update_ test
scenarios in *ReservationACLsTestBase*
* Is it possible to parameterize the ReservationACLsTest instead of separate
ones for Capacity/Fair scheduler?
> Consider creating separate ACLs for Reservation create/update/delete/list ops
> -----------------------------------------------------------------------------
>
> Key: YARN-2575
> URL: https://issues.apache.org/jira/browse/YARN-2575
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: capacityscheduler, fairscheduler, resourcemanager
> Reporter: Subru Krishnan
> Assignee: Sean Po
> Attachments: YARN-2575.v1.patch, YARN-2575.v2.1.patch,
> YARN-2575.v2.patch, YARN-2575.v3.patch, YARN-2575.v4.patch,
> YARN-2575.v5.patch, YARN-2575.v6.patch, YARN-2575.v7.patch, YARN-2575.v8.patch
>
>
> YARN-1051 introduces the ReservationSystem and in the current implementation
> anyone who can submit applications can also submit reservations. This JIRA is
> to evaluate creating separate ACLs for Reservation create/update/delete ops.
> Depends on YARN-4340
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
