[
https://issues.apache.org/jira/browse/YARN-5047?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15423565#comment-15423565
]
Daniel Templeton commented on YARN-5047:
----------------------------------------
Thanks for the patch, [~rchiang]! A few comments:
* The {{AbstractYarnScheduler.nodeUpdate()}} method is pretty rambly. Think
you could refactor it into a couple of more bite-sized methods?
* In this code:
{code}
Resource rs = container.getAllocatedResource();
if (rs != null) {
Resources.addTo(releasedResources, rs);
}
rs = container.getReservedResource();
if (rs != null) {
Resources.addTo(releasedResources, rs);
}
{code}
I'd rather not reuse the {{rs}} variable.
* For this comment:
{code}
// TODO: Fix possible race-condition when request comes in before
// update is propagated
{code}
is there a JIRA filed? If so, can you include it in the comment? (I realize
that came from the CS code, but it would be nice to clean it up.)
* In this code:
{code}
if (rmContext.isWorkPreservingRecoveryEnabled()
&& !rmContext.isSchedulerReadyForAllocatingContainers()) {
return;
}
if (Resources.greaterThanOrEqual(resourceCalculator, getClusterResource(),
node.getUnallocatedResource(), minimumAllocation)) {
...
}
{code}
I'd much rather you add the first if's conditions to the second if, rather than
use the if-return.
> Refactor nodeUpdate across schedulers
> -------------------------------------
>
> Key: YARN-5047
> URL: https://issues.apache.org/jira/browse/YARN-5047
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: capacityscheduler, fairscheduler, scheduler
> Affects Versions: 3.0.0-alpha1
> Reporter: Ray Chiang
> Assignee: Ray Chiang
> Attachments: YARN-5047.001.patch, YARN-5047.002.patch,
> YARN-5047.003.patch, YARN-5047.004.patch, YARN-5047.005.patch,
> YARN-5047.006.patch, YARN-5047.007.patch, YARN-5047.008.patch,
> YARN-5047.009.patch, YARN-5047.010.patch
>
>
> FairScheduler#nodeUpdate() and CapacityScheduler#nodeUpdate() have a lot of
> commonality in their code. See about refactoring the common parts into
> AbstractYARNScheduler.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
