[jira] [Commented] (YARN-9206) RMServerUtils does not count SHUTDOWN as an accepted state

[Sunil Govindan (JIRA)]

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

Sunil Govindan commented on YARN-9206:
--------------------------------------

Thanks [~kshukla]

This logic seems better as it can avoid such complex loops, but as you 
mentioned it comes with cost of multiple if and couple of boolean.

Could we improve to something like this to save few more if's. Ideally if we 
can 
{code:java}
public static List<RMNode> queryRMNodesV2(RMContext context,
    EnumSet<NodeState> acceptedStates) {
  // nodes contains nodes that are NEW, RUNNING, UNHEALTHY or DECOMMISSIONING.
  List<RMNode> inactiveNodes = new ArrayList<>();
  List<RMNode> activeNodes = new ArrayList<>();
  ArrayList<RMNode> results = new ArrayList<RMNode>();
  for (NodeState nodeState : acceptedStates) {
    if (inactiveNodes.isEmpty() && nodeState.isInactiveState()) {
      inactiveNodes.addAll(context.getInactiveRMNodes().values());
    }
    if (activeNodes.isEmpty() && nodeState.isActiveState()) {
      activeNodes.addAll(context.getRMNodes().values());
    }
    if (!inactiveNodes.isEmpty() && !activeNodes.isEmpty()) {
      break;
    }
  }

  for (RMNode rmNode : inactiveNodes) {
    if ((rmNode != null) && acceptedStates.contains(rmNode.getState())) {
      results.add(rmNode);
    }
  }

  for (RMNode rmNode : activeNodes) {
    if (acceptedStates.contains(rmNode.getState())) {
      results.add(rmNode);
    }
  }

  return results;
}{code}

> RMServerUtils does not count SHUTDOWN as an accepted state
> ----------------------------------------------------------
>
>                 Key: YARN-9206
>                 URL: https://issues.apache.org/jira/browse/YARN-9206
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 3.0.3
>            Reporter: Kuhu Shukla
>            Assignee: Kuhu Shukla
>            Priority: Major
>         Attachments: YARN-9206.001.patch, YARN-9206.002.patch, 
> YARN-9206.003.patch
>
>
> {code}
> if (acceptedStates.contains(NodeState.DECOMMISSIONED) ||
>         acceptedStates.contains(NodeState.LOST) ||
>         acceptedStates.contains(NodeState.REBOOTED)) {
>       for (RMNode rmNode : context.getInactiveRMNodes().values()) {
>         if ((rmNode != null) && acceptedStates.contains(rmNode.getState())) {
>           results.add(rmNode);
>         }
>       }
>     }
>     return results;
>   }
> {code}
> This should include SHUTDOWN state as they are inactive too. This method is 
> used for node reports and such so might be useful to account for them as well.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org