[
https://issues.apache.org/jira/browse/YARN-5331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15468705#comment-15468705
]
Sean Po commented on YARN-5331:
-------------------------------
Thanks [~ajsangeetha] for the patch! This looks good, except for a few very
minor comments:
* NIT: PeriodicRLESParseResourceAllocation.java+27
** Remove extra newline.
* NIT: TestPeriodicRLESparseResourceAllocation.java+18
** Add a newline.
* NIT: PeriodicRLESParseResourceAllocation.java+57
** It looks like RLESparseResourceAllocation.AddInterval would include 0, and
does not include interval.getEndTime(). Please update the javadoc.
* NIT: PeriodicRLESParseResourceAllocation.java+61
** Extra space before "specified".
* PeriodicRLESParseResourceAllocation.java+65
** Do we need to check interval.getStartTime() >= 0 and interval.getEndTime() >
interval.getStartTime()?
* TestPeriodicRLESparseResourceAllocation.java+81
** Can be static.
** Also looks a lot like ReservationSystemTestUtil::generateAllocation. Maybe
you can use that method instead, or move this one into
ReservationSystemTestUtil. That way you can re-use in
TestRLESparseResourceAllocation as well.
* TestRLESparseResourceAllocation.java+528
** Please include tests for this where tick > lastKey. Is it expected to be
ZERO_RESOURCE?
** Also include test for tick == lastKey.
Question about PeriodicRLESparseResourceAllocation::setTimePeriod:
* If the new period is smaller than the previous period, should the behavior be
to trim rleVector? It looks like there will be a bug for
PeriodicRLESparseResourceAllocation::getMaxPeriodicCapacity if rleVector isn't
trimmed, because the end condition is t <= lastKey.
** Please add a unit test for this condition.
> Extend RLESparseResourceAllocation with period for supporting recurring
> reservations in YARN ReservationSystem
> --------------------------------------------------------------------------------------------------------------
>
> Key: YARN-5331
> URL: https://issues.apache.org/jira/browse/YARN-5331
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Subru Krishnan
> Assignee: Sangeetha Abdu Jyothi
> Attachments: YARN-5331.001.patch, YARN-5331.002.patch
>
>
> YARN-5326 proposes adding native support for recurring reservations in the
> YARN ReservationSystem. This JIRA is a sub-task to add a
> PeriodicRLESparseResourceAllocation. Please refer to the design doc in the
> parent JIRA for details.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
