[jira] [Commented] (YARN-4311) Removing nodes from include and exclude lists will not remove them from decommissioned nodes list

[Daniel Templeton (JIRA)]

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

Daniel Templeton commented on YARN-4311:
----------------------------------------

Thanks, [~kshukla].

Can we simplify this:

{code}
        if (rmNode != null) {
          if (acceptedStates.contains(rmNode.getState())) {
            results.add(rmNode);
          }
        }
{code}

to this:

{code}
        if ((rmNode != null) && acceptedStates.contains(rmNode.getState())) {
          results.add(rmNode);
        }
{code}

In general, I think it's preferable to split a line somewhere other than on a 
"." if convenient.  There are a handful of other indentation and formatting 
issues, but this is the only one that bugs me enough to mention:

{code}
    if(finalState == NodeState.SHUTDOWN && rmNode.context.getNodesListManager().
        isUntrackedNode(rmNode.hostName)) {
{code}

should be:

{code}
    if (finalState == NodeState.SHUTDOWN &&
        rmNode.context.getNodesListManager().isUntrackedNode(rmNode.hostName)) {
{code}

Note the space after the "if". :)

Otherwise, it looks good.  Unit tests look reasonable.  Thanks!

> Removing nodes from include and exclude lists will not remove them from 
> decommissioned nodes list
> -------------------------------------------------------------------------------------------------
>
>                 Key: YARN-4311
>                 URL: https://issues.apache.org/jira/browse/YARN-4311
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 2.6.1
>            Reporter: Kuhu Shukla
>            Assignee: Kuhu Shukla
>         Attachments: YARN-4311-v1.patch, YARN-4311-v2.patch, 
> YARN-4311-v3.patch, YARN-4311-v4.patch, YARN-4311-v5.patch
>
>
> In order to fully forget about a node, removing the node from include and 
> exclude list is not sufficient. The RM lists it under Decomm-ed nodes. The 
> tricky part that [~jlowe] pointed out was the case when include lists are not 
> used, in that case we don't want the nodes to fall off if they are not active.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)