[
https://issues.apache.org/jira/browse/YARN-1710?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14129038#comment-14129038
]
Chris Douglas commented on YARN-1710:
-------------------------------------
{{GreedyReservationAgent}}
* Consider {{@link}} for {{ReservationRequest}} in class javadoc
* An inline comment could replace the {{adjustContract()}} method
* Most of the javadoc on private methods can be cut
* {{currentReservationStage}} does not need to be declared outside the loop
* {{allocations}} cannot be null
* An internal {{Resource(0, 0)}} could be reused
* {{li}} should be part of the loop ({{for}} not {{while}}). Its initialization
is unreadable; please use temp vars.
* Generally, embedded calls are difficult to read:
{code}
if (findEarliestTime(allocations.keySet()) > earliestStart) {
allocations.put(new ReservationInterval(earliestStart,
findEarliestTime(allocations.keySet())), ReservationRequest
.newInstance(Resource.newInstance(0, 0), 0));
// consider to add trailing zeros at the end for simmetry
}
{code}
Assuming the {{ReservationRequest}} is never modified by the plan:
{code}
private final ZERO_RSRC =
ReservationRequest.newInstance(Resource.newInstance(0, 0), 0);
// ...
long allocStart = findEarliestTime(allocations.keySet());
if (allocStart > earliestStart) {
ReservationInterval preAlloc =
new ReservationInterval(earliestStart, allocStart);
allocations.put(preAlloc, ZERO_RSRC);
}
{code}
* {{findEarliestTime(allocations.keySet())}} is called several times and should
be memoized
** Would a {{TreeSet}} be more appropriate, given this access pattern?
* Instead of:
{code}
boolean result = false;
if (oldReservation != null) {
result = plan.updateReservation(capReservation);
} else {
result = plan.addReservation(capReservation);
}
return result;
{code}
Consider:
{code}
if (oldReservation != null) {
return plan.updateReservation(capReservation);
}
return plan.addReservation(capReservation);
{code}
* A comment unpacking the arithmetic for calculating {{curMaxGang}} would help
readability
{{TestGreedyReservationAgent}}
* Instead of fixing the seed, consider setting and logging it for each run.
* {{testStress}} is brittle, as it verifies only the timeout; {{testBig}} and
{{testSmall}} don't verify anything. Both tests are useful, but probably not as
part of the build. Dropping the annotation and adding a {{main()}} that calls
each fo them would be one alternative.
> Admission Control: agents to allocate reservation
> -------------------------------------------------
>
> Key: YARN-1710
> URL: https://issues.apache.org/jira/browse/YARN-1710
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Carlo Curino
> Assignee: Carlo Curino
> Attachments: YARN-1710.1.patch, YARN-1710.patch
>
>
> This JIRA tracks the algorithms used to allocate a user ReservationRequest
> coming in from the new reservation API (YARN-1708), in the inventory
> subsystem (YARN-1709) maintaining the current plan for the cluster. The focus
> of this "agents" is to quickly find a solution for the set of contraints
> provided by the user, and the physical constraints of the plan.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
