hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Templeton (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-5047) Refactor nodeUpdate across schedulers
Date Tue, 16 Aug 2016 23:09:20 GMT

    [ 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: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message