Chris Douglas commented on YARN-1710:

* 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:
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
Assuming the {{ReservationRequest}} is never modified by the plan:
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);
* {{findEarliestTime(allocations.keySet())}} is called several times and should 
be memoized
** Would a {{TreeSet}} be more appropriate, given this access pattern?
* Instead of:
boolean result = false;
if (oldReservation != null) {
  result = plan.updateReservation(capReservation);
} else {
  result = plan.addReservation(capReservation);
return result;
if (oldReservation != null) {
  return plan.updateReservation(capReservation);
return plan.addReservation(capReservation);
* A comment unpacking the arithmetic for calculating {{curMaxGang}} would help 

* 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

Reply via email to