[
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:
-------------------------------------
General
- The public classes should be annotated with the correct visibility and
stability annotations (probably {{@Public}} and {{@Unstable}})
{{SharingPolicy}}
- Javadoc throws clause and some parameters not populated
- In particular, the {{excludeList}} parameter could use some unpacking.
{{CapacityOverTimePolicy}}
- 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.
{{*Exception}}
- 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).
{{NoOverCommitPolicy}}
- The {{excludeList}} should probably be final, and cleared/populated with a
clone of the set on calls to {{init()}}
{{CapacitySchedulerConfiguration}}
- Missing javadoc for the new parameters.
{{TestNoOverCommitPolicy}}
- 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
{{TestCapacityOverTimePolicy}}
- 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
(v6.3.4#6332)
