[
https://issues.apache.org/jira/browse/YARN-4837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15284991#comment-15284991
]
Wangda Tan commented on YARN-4837:
----------------------------------
Thanks [~vinodkv], patch generally looks good to me, minor comments:
1) Should we disable am-blacklisting by default?
2) Javadocs and method name is not matched:
{code}
/**
* RM is updating blacklist for AM containers.
* @param blacklistAdditions resources to be added to the amBlacklist
* @param blacklistRemovals resources to be added to the amBlacklist
*/
public void updatePlacesBlacklistedBySystem(
{code}
One is updating for AM containers and the other one is update by system.
3) Similarily, it's better to add a simple explanation why
updatePlacesBlackedBySystem is called when isWaitingForAMContainer == true.
{code}
public synchronized void updateBlacklist(List<String> blacklistAdditions,
List<String> blacklistRemovals) {
if (!isStopped) {
if (isWaitingForAMContainer()) {
// The request is for the AM-container, so update the corresponding
// blacklists
this.appSchedulingInfo.updatePlacesBlacklistedBySystem(
{code}
4) RMAppAttemptBlock.java:
{code}
._("Nodes skipped for AM launches:", rmBlackListedNodes);
{code}
Update it to be: "Nodes blacklisted by system?
5) Similiarily, AppAttemptInfo.java
nodesSkippedForAMLaunches -> nodesBlacklistedBySystem?
6) ResourceBlacklistRequest -> (Resource)Place(ment)BlacklistRequest?
7) It's better to add a test for shouldCountTowardsNodeBlacklisting changes --
it's important to make sure no regression happens to the behavior.
> User facing aspects of 'AM blacklisting' feature need fixing
> ------------------------------------------------------------
>
> Key: YARN-4837
> URL: https://issues.apache.org/jira/browse/YARN-4837
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Vinod Kumar Vavilapalli
> Assignee: Vinod Kumar Vavilapalli
> Priority: Critical
> Attachments: YARN-4837-20160515.txt
>
>
> Was reviewing the user-facing aspects that we are releasing as part of 2.8.0.
> Looking at the 'AM blacklisting feature', I see several things to be fixed
> before we release it in 2.8.0.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
