[ https://issues.apache.org/jira/browse/YARN-9278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16790501#comment-16790501 ]
Wilfred Spiegelenburg commented on YARN-9278: --------------------------------------------- I have a couple of comments on the patch. * I think we need a default that turns this functionality off by default. An administrator should actively turn this on. A default of -1 or 0 is better and we should skip over the calculation for that value. * Your code does not wrap at the end of the list. That is why I changed it to a do while loop. as an example: I have a batch size of 100 and I have 350 nodes. I could start at node 300 and still want to check those 100 nodes and want to check nodes 300-350 and 0-49. We should never double check a node, never go past the start. I updated the example code with a wrapping end calculation. * We need to cater for a setup that has the batch value set and a node list, for whatever reason or point in time, is smaller than the batch size. Your code does not handle this. We should just process all nodes at that point and not stop at the end of the list. * The text for the property is not clear at all. (See below) * Please look at adding a test for this change. Text for the property remarks: {code} The max trial nodes num to identify containers for one starved container. Defaults to 0. {code} It does not explain what it does and why it is there. It is too cryptic. We should explain the following: * what is it used for (use a partial list of nodes to check for preemptable containers in a large cluster) * when should it be used, or when it takes effect. * what is the impact: AM container impact, > Shuffle nodes when selecting to be preempted nodes > -------------------------------------------------- > > Key: YARN-9278 > URL: https://issues.apache.org/jira/browse/YARN-9278 > Project: Hadoop YARN > Issue Type: Sub-task > Components: fairscheduler > Reporter: Zhaohui Xin > Assignee: Zhaohui Xin > Priority: Major > Attachments: YARN-9278.001.patch > > > We should *shuffle* the nodes to avoid some nodes being preempted frequently. > Also, we should *limit* the num of nodes to make preemption more efficient. > Just like this, > {code:java} > // we should not iterate all nodes, that will be very slow > long maxTryNodeNum = > context.getPreemptionConfig().getToBePreemptedNodeMaxNumOnce(); > if (potentialNodes.size() > maxTryNodeNum){ > Collections.shuffle(potentialNodes); > List<FSSchedulerNode> newPotentialNodes = new ArrayList<FSSchedulerNode>(); > for (int i = 0; i < maxTryNodeNum; i++){ > newPotentialNodes.add(potentialNodes.get(i)); > } > potentialNodes = newPotentialNodes; > {code} > -- 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