[
https://issues.apache.org/jira/browse/YARN-5385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15706987#comment-15706987
]
Carlo Curino commented on YARN-5385:
------------------------------------
[~seanpo03] thanks for working on this.
I like the general direction, few issues below:
# in {{ReservationSchedulerConfiguration}} you implemented
{{getPriorityReservationAgent(String queue)}} as a fix return of
{{DEFAULT_PRIORITY_RESERVATION_AGENT_NAME}}. Why? I see either a lookup in the
config flies with fallback on this, or not even having the get-by-queue method.
As is it seems a bit redundant.
# minor: you use in many comments "accomodate for", which is a bit unusual
english I think.
# how likely is that we need to "make room" for another reservation? If it is
rather likely we might want to lower the verbosity of the logs in
{{PriorityReservationAgent}}
# {{SimplePriorityReservationAgent}} to where it is used in
{{PriorityReservationAgent.addYieldedReservations}}. The cost is the same, but
a different sub-class might not return the reservation in priority order.
Alternatively, you should very prominently document this in the API.
# in {{PriorityReservationAgent.addYieldedReservations}} you simply log the
once-accepted, now-rejected reservations. Is this the best we can do? Can we
retain the list (with ReservationDefinition), and make it accessible from the
outside and/or try to come up with a call-back mechanism of sort? Maybe upon
the delete of a reservation, we could attempt to re-add some of the rejected
ones? etc..
# One concern is that if the current reservation does not fit even after
removing the lower prioirty reservations, you might change the set of accepted
reservations (e.g., because the order you submit the is no more the FIFO in
which they were accepted but a priority order, which might lead to less
reservations to fit). I should probably follow the suggestion above, and have a
switch in addYieldedReservations to retain the order of acceptance (which you
should also make sure is updated when the submission of a higher priority item
scramble this set). Try to define precisely what is the invariant enforced
(beside "low-priority reservation never prevent a higher priority reservation
to be accepted").
# I don't understand the concurrency model. How are you protecting from
concurrent/clashing invocations of the priority-enabled agents?
# Using something like tombstones or logically excluding lower-priority
reservations from the used resources might allow you to deal with concurrency
as well as some of the other issues above.
# ReservationComparator should like deal with null inputs, and comparing long
could be done with a simple difference instead of the method you have.
# what happens if you have reservation priority turned on, but the
{{ReservationDefinition}} does not carry its value? (e.g., in {{RMWebService},
but also in general}
# I haven't look at tests carefully, but I like the fact that you have plenty
scenarios. In general, I like to have methods that checks invariants: e.g.,
that no lower-priority reservation is accepted when a higher priority could fit
in the same space, or that no reservation is rejected if there is room to
accepted etc... This also allows you to have large randomized tests, and simply
check the invariants for those (I find those very useful to catch funky corner
cases).
> Add a PriorityAgent in ReservationSystem
> -----------------------------------------
>
> Key: YARN-5385
> URL: https://issues.apache.org/jira/browse/YARN-5385
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: capacity scheduler, fairscheduler, resourcemanager
> Reporter: Sean Po
> Assignee: Sean Po
> Labels: oct16-hard
> Attachments: YARN-5385.v002.patch, YARN-5385.v003.patch,
> YARN-5385.v004.patch, YARN-5385.v1.patch
>
>
> YARN-5211 proposes adding support for generalized priorities for reservations
> in the YARN ReservationSystem. This JIRA is a sub-task to track the addition
> of a priority agent to accomplish it. Please refer to the design doc in the
> parent JIRA for details.
--
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
