hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Douglas (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-1709) Admission Control: Reservation subsystem
Date Sat, 06 Sep 2014 00:30:29 GMT

    [ https://issues.apache.org/jira/browse/YARN-1709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14123885#comment-14123885

Chris Douglas commented on YARN-1709:

Overall, the patch lgtm. Just a few minor tweaks, then I'm +1
* very minor: Javadoc could be compressed a bit (empty lines)

* The {{ZERO_RESOURCE}} instance escapes via {{getConsumptionForUser}}
* Some lines are more than 80 characters
* The logging can use built-in substitution more efficiently. Instead of:
String errMsg =
            "The specified Reservation with ID {0} does not exist in the plan",
LOG.error("The specified Reservation with ID {} does not exist in the plan",
Some of the code already uses this construction, but a few still use {{MessageFormat}}.
* This form is harder to read:
InMemoryReservationAllocation inMemReservation = null;
if (reservation instanceof InMemoryReservationAllocation) {
  inMemReservation = (InMemoryReservationAllocation) reservation;
} else {
  // [snip] log error
  throw new RuntimeException(errMsg);
than the if (error) { throw; } construction used the other checks. Is it an improvement over
* {{addReservation}} doesn't need to hold the write lock while it checks invariants on its
* The private methods that assume locks ({{incrementAllocation}}, {{decrementAllocation}},
{{removeReservation}}, etc.) are held should probably {{assert}} that precondition (e.g.,
* {{getMinimumAllocation}} and {{getMaximumAllocation}} return mutable data that should probably
be cloned

* minor style: redundant {{this}} in get methods
* {{toString}} should use {{StringBuilder}} instead of {{StringBuffer}}

* Mismatched javadoc on {{getEarliestStartTime}}
* {{getLastEndTime}} specifies UTC. Is that enforced in the implementation?

* Can this be made immutable? It's a key in several maps

* Though some methods in {{InMemoryPlan}}, the {{ZERO_RESOURCE}} internal variable can escape
via {{getCapacityAtTime}}.

> Admission Control: Reservation subsystem
> ----------------------------------------
>                 Key: YARN-1709
>                 URL: https://issues.apache.org/jira/browse/YARN-1709
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>            Reporter: Carlo Curino
>            Assignee: Subramaniam Krishnan
>         Attachments: YARN-1709.patch, YARN-1709.patch, YARN-1709.patch
> This JIRA is about the key data structure used to track resources over time to enable
YARN-1051. The Reservation subsystem is conceptually a "plan" of how the scheduler will allocate
resources over-time.

This message was sent by Atlassian JIRA

View raw message