[
https://issues.apache.org/jira/browse/YARN-2080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14125994#comment-14125994
]
Vinod Kumar Vavilapalli commented on YARN-2080:
-----------------------------------------------
Some comments on the patch:
- Configuration
-- admission.enable -> Rename to reservations.enable?
-- RM_SCHEDULER_ENABLE_RESERVATIONS -> RM_RESERVATIONS_ENABLE,
DEFAULT_RM_SCHEDULER_ENABLE_RESERVATIONS -> DEFAULT_RM_RESERVATIONS_ENABLE
-- reservation.planfollower.time-step ->
reservation-system.plan-follower.time-step
-- RM_PLANFOLLOWER_TIME_STEP, DEFAULT_RM_PLANFOLLOWER_TIME_STEP ->
RM_RESERVATION_SYSTEM_PLAN_FOLLOWER_TIME_STEP,
DEFAULT_RM_RESERVATION_SYSTEM_PLAN_FOLLOWER_TIME_STEP
- A meta question about configuration: It seems like if I pick up a scheduler
and enable reservations, the system-class, the plan-follower should be picked
up automatically instead of them being standalone configs. Can we do that?
Otherwise the following
-- reservation.class -> reservation-system.class?
-- RM_RESERVATION, DEFAULT_RM_RESERVATION -> RM_RESERVATION_SYSTEM_CLASS,
DEFAULT_RM_RESERVATION_SYSTEM_CLASS
-- reservation.plan.follower -> reservation-system.plan-follower
-- RM_RESERVATION_PLAN_FOLLOWER, DEFAULT_RM_RESERVATION_PLAN_FOLLOWER ->
RM_RESERVATION_SYSTEM_PLAN_FOLLOWER, DEFAULT_RM_RESERVATION_SYSTEM_PLAN_FOLLOWER
- YarnClient.submitReservation(): We don't return a queue-name anymore after
the latest YARN-1708? There are javadoc refs to the queue-name being returned.
- ClientRMService
-- If reservations are not enabled, we get a host of "Reservation is not
enabled. Please enable & try again" everytime which is not desirable. See
checkReservationSystem(). This log and a bunch of similar logs in
ReservationInputValidator may either be (1) deleted or (2) actually belong to
the audit-log (RMAuditLogger) - we don't need to double-log
-- checkReservationACLs: Today anyone who can submit applications can also
submit reservations. We may want to separate them, if you agree, I'll file a
ticket for future separation of these ACLs.
- AbstractReservationSystem
-- getPlanFollower() -> createPlanFollower()
-- create and init plan-follower should be in serviceInit()?
-- getNewReservationId(): Use ReservationId.newInstance()
- ReservationInputValidator: Deleting a request shouldn't need
validateReservationUpdateRequest->validateReservationDefinition. We only need
the ID validation.
- CapacitySchedulerConfiguration: I don't understand the semantics of configs
- average-capacity, reservable.queue, reservation-window,
reservation-enforcement-window, instantaneous-max-capacity, - yet as they are
not used in this patch. Can we drop them (and their setters/getters) here and
move them to the JIRA that actually uses them?
Tests
- TestYarnClient: You can use the newInstance methods and avoid using pb
implementations and the setters directly (for e.g {{new
ReservationDeleteRequestPBImpl()}}
- TestClientRMService:
-- ReservationRequest.setLeaseDuration() was renamed to be simply
setDuration() in YARN-1708. Seems like there are other such occurrences in the
patch.
-- Similary to TestYarnClient, use record.newInstance methods instead of
directly invoking PBImpls.
Can't understand CapacityReservationSystem yet as I have to dig into the
details of YARN-1709.
> Admission Control: Integrate Reservation subsystem with ResourceManager
> -----------------------------------------------------------------------
>
> Key: YARN-2080
> URL: https://issues.apache.org/jira/browse/YARN-2080
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Subramaniam Krishnan
> Assignee: Subramaniam Krishnan
> Attachments: YARN-2080.patch, YARN-2080.patch, YARN-2080.patch
>
>
> This JIRA tracks the integration of Reservation subsystem data structures
> introduced in YARN-1709 with the YARN RM. This is essentially end2end wiring
> of YARN-1051.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
