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

ASF GitHub Bot commented on YARN-6483:
--------------------------------------

Github user juanrh commented on a diff in the pull request:

    https://github.com/apache/hadoop/pull/289#discussion_r151262546
  
    --- Diff: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/NodeReport.java
 ---
    @@ -53,15 +53,15 @@ public static NodeReport newInstance(NodeId nodeId, 
NodeState nodeState,
           String httpAddress, String rackName, Resource used, Resource 
capability,
           int numContainers, String healthReport, long lastHealthReportTime) {
         return newInstance(nodeId, nodeState, httpAddress, rackName, used,
    -        capability, numContainers, healthReport, lastHealthReportTime, 
null);
    +        capability, numContainers, healthReport, lastHealthReportTime, 
null, null);
       }
     
       @Private
       @Unstable
       public static NodeReport newInstance(NodeId nodeId, NodeState nodeState,
           String httpAddress, String rackName, Resource used, Resource 
capability,
           int numContainers, String healthReport, long lastHealthReportTime,
    -      Set<String> nodeLabels) {
    +      Set<String> nodeLabels, Integer decommissioningTimeout) {
    --- End diff --
    
    This `Integer` comes from 
[`RMNode.getDecommissioningTimeout()`](https://github.com/apache/hadoop/blob/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmnode/RMNode.java#L189)
 that was already returning an Integer before this patch, because only nodes in 
DECOMMISSIONING state have an associated decommission timeout, so `null` is 
used to express absent timeout. In this patch 
`RMNode.getDecommissioningTimeout()` is used in 
`DefaultAMSProcessor.handleNodeUpdates` to get the argument 
`decommissioningTimeout` for `BuilderUtils.newNodeReport`. If we use a `int` 
for `decommissioningTimeout` in `NodeReport.newInstance` I think we should also 
use an `int` for the same argument in `BuilderUtils.newNodeReport` for 
uniformity, which implies a conversion from `null` to `-1` in 
`DefaultAMSProcessor.handleNodeUpdates`.
    
    So I think we should either keep using `Integer decommissioningTimeout` 
everywhere, enconding absent timeout with `null`, or use `int 
decommissioningTimeout` everywhere, enconding absent timeout with a negative 
timeout, which is coherent with `message NodeReportProto` using an unsigned int 
for `decommissioning_timeout`. What do you think about these 2 alternatives?


> Add nodes transitioning to DECOMMISSIONING state to the list of updated nodes 
> returned by the Resource Manager as a response to the Application Master 
> heartbeat
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: YARN-6483
>                 URL: https://issues.apache.org/jira/browse/YARN-6483
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: resourcemanager
>    Affects Versions: 2.8.0
>            Reporter: Juan Rodríguez Hortalá
>         Attachments: YARN-6483-v1.patch
>
>
> The DECOMMISSIONING node state is currently used as part of the graceful 
> decommissioning mechanism to give time for tasks to complete in a node that 
> is scheduled for decommission, and for reducer tasks to read the shuffle 
> blocks in that node. Also, YARN effectively blacklists nodes in 
> DECOMMISSIONING state by assigning them a capacity of 0, to prevent 
> additional containers to be launched in those nodes, so no more shuffle 
> blocks are written to the node. This blacklisting is not effective for 
> applications like Spark, because a Spark executor running in a YARN container 
> will keep receiving more tasks after the corresponding node has been 
> blacklisted at the YARN level. We would like to propose a modification of the 
> YARN heartbeat mechanism so nodes transitioning to DECOMMISSIONING are added 
> to the list of updated nodes returned by the Resource Manager as a response 
> to the Application Master heartbeat. This way a Spark application master 
> would be able to blacklist a DECOMMISSIONING at the Spark level.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to