Chris Douglas commented on YARN-2475:

* The Clock can be initialized in the constructor, declared private and final
* The exception refers to an InventorySizeAdjusmentPolicy
* nit: redundant parenthesis in the main loop, exceeds 80 char
* {{curSessions}} cannot be null; prefer {{!isEmpty()}} to {{size() > 0}}
** Is this check even necessary? {{sort}} and the following loop should be noops
* A brief comment about the natural order of {{ReservationAllocations}} would 
help readability of this loop. It's in the class doc, but something inline 
would be helpful
* An internal {{Resource(0,0)}} could be reused, instead of creating it in the 
* Could the inner loop be more readable? The embedded function calls in the 
{{Resource}} arithmetic are hard to read (pseudo):
ArrayList<> curSessions = new ArrayList<>(plan.getResourcesAtTime(t));
for (Iterator<> i = curSessions.iterator(); i.hasNext() && excessCap > 0;) {
  InMemoryReservationAllocation a = (InMemoryReservationAllocation) i.next();
  excessCap -= a.getResourcesAtTime(t);
* Why is the enforcement window tied to {{CapacitySchedulerConfiguration}}?

* Tests should not call {{Thread.sleep}}; instead update the mock
* Passing in a mocked {{Clock}} to the cstr rather than assigning it in the 
test is cleaner
* Instead of {{assertTrue(cond != null)}} use {{assertNotNull(cond)}} (same for 
positive null check)
* The test should not catch and discard {{PlanningException}}

> ReservationSystem: replan upon capacity reduction
> -------------------------------------------------
>                 Key: YARN-2475
>                 URL: https://issues.apache.org/jira/browse/YARN-2475
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>            Reporter: Carlo Curino
>            Assignee: Carlo Curino
>         Attachments: YARN-2475.patch
> In the context of YARN-1051, if capacity of the cluster drops significantly 
> upon machine failures we need to trigger a reorganization of the planned 
> reservations. As reservations are "absolute" it is possible that they will 
> not all fit, and some need to be rejected a-posteriori.  

This message was sent by Atlassian JIRA

Reply via email to