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-1711) CapacityOverTimePolicy: a policy to enforce quotas over time for YARN-1709
Date Sun, 14 Sep 2014 21:40:34 GMT

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

Chris Douglas commented on YARN-1711:

- The public classes should be annotated with the correct visibility and stability annotations
(probably {{@Public}} and {{@Unstable}})

- Javadoc throws clause and some parameters not populated
- In particular, the {{excludeList}} parameter could use some unpacking.

- Just performing the cast, and throwing {{ClassCastException}} implicitly is equally clear
- nit: spacing/concat: {{plan.getTotalCapacity() + ")by " + "accepting reservation: "}}
- Just as an observation, no change requested: the assumption that the sharing policy holds
the lock on the plan is probably OK, but since both are interfaces there may be a missing
abstraction that associates compatible sets of interlocking components.
- I can't think of a more appropriate solution to handling aggregates of {{Resource}}. Anything
more "correct" doesn't really justify the complexity, certainly not before we get some more
experience with planning. Since enabling this is optional, enforcement with the {{IntegralResource}}
is a pragmatic tradeoff.

- s/MismatchingUserException/MismatchedUserException/
- Are the subclasses of {{PlanningException}} for a caller to distinguish the cause for the
rejected request, so it can refine it? If that's the case, should they contain diagnostic
information as a payload e.g., requested vs actual user? If the intent is to extract it, then
some more easily parsed format for the message might be appropriate (e.g., JSON).

- The {{excludeList}} should probably be final, and cleared/populated with a clone of the
set on calls to {{init()}}

- Missing javadoc for the new parameters.

- Consider using {{@Test(expected = SomeException.class)}} instead of {{Assert::fail()}} and
try/catch for {{testSingleFail()}}
- Consider specifying the expected cause/subtype instead of generic {{PlanningException}}
- {{testMultiTenantFail}} only veries that a {{PlanningException}} is thrown, not that it
fails as expected

- Most of the tests don't verify that the failure occurs when and how its parameters specify,
but only check that a {{PlanningException}} is thrown.

> CapacityOverTimePolicy: a policy to enforce quotas over time for YARN-1709
> --------------------------------------------------------------------------
>                 Key: YARN-1711
>                 URL: https://issues.apache.org/jira/browse/YARN-1711
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Carlo Curino
>            Assignee: Carlo Curino
>              Labels: reservations
>         Attachments: YARN-1711.1.patch, YARN-1711.2.patch, YARN-1711.3.patch, YARN-1711.patch
> This JIRA tracks the development of a policy that enforces user quotas (a time-extension
of the notion of capacity) in the inventory subsystem discussed in YARN-1709.

This message was sent by Atlassian JIRA

View raw message