[
https://issues.apache.org/jira/browse/YARN-4311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15212400#comment-15212400
]
Jason Lowe commented on YARN-4311:
----------------------------------
Thanks for updating the patch! Sorry for the delay in getting back to the
review.
The property documentation should not refer to RMNodes, as users who are not
savvy on the source code may be confused by it. Similarly the documentation
should elaborate a bit on how a node is "declared untracked".
Nit: The timer task should just grab the current time once and cache it in a
local rather than hammering on it in the loop.
Nit: Same current time comment for the refresh cases, and I think it's worth
factoring the common loop over the inactive nodes into a private utility method.
Nit: The timer task logic ends up calling isUntrackedNode unnecessarily often,
and it's the more expensive check to make. It think it would be better to
check that first, since each conditional clause requires knowing the result
anyway. For example, something like this:
{code}
long now = Time.monotonicNow();
if (isUntrackedNode(rmNode.getHostName())) {
if (rmNode.getUntrackedTimestamp == 0) {
rmNode.setUntrackedTimeStamp(now);
} else if (now - rmNode.getUntrackedTimeStamp() > nodeRemovalTimeout) {
[...node removal logic..]
}
} else {
rmNode.setUntrackedTimeStamp(0);
}
{code}
> 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-v10.patch,
> YARN-4311-v11.patch, YARN-4311-v11.patch, YARN-4311-v2.patch,
> YARN-4311-v3.patch, YARN-4311-v4.patch, YARN-4311-v5.patch,
> YARN-4311-v6.patch, YARN-4311-v7.patch, YARN-4311-v8.patch, YARN-4311-v9.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)
