Junping Du commented on YARN-41:

[~devaraj.k], thanks for updating the patch! [~vinodkv] is on vacation, so I 
will help to review here. Just a quick glance at your patch (v3), a couple of 
comments and questions:

Looks like we are adding a new event. Given we already have decommission event, 
so this is for other cases, e.g. shutdown NM daemon through CLI. Isn't it? If 
so, we should consider the case that NM work preserving is enabled (for rolling 
upgrade), and these nodes shouldn't be unregister to RM.

   protected void serviceStop() throws Exception {
+    // the isStopped check is for avoiding multiple unregistrations.
+    if (this.registeredWithRM && !this.isStopped) {
+      unRegisterNM();
+    }
Like I said above, we only need to unregister NM from RM when NM recovery is 
disabled. We may should put a check here.

More comments will come later.

> The RM should handle the graceful shutdown of the NM.
> -----------------------------------------------------
>                 Key: YARN-41
>                 URL: https://issues.apache.org/jira/browse/YARN-41
>             Project: Hadoop YARN
>          Issue Type: New Feature
>          Components: nodemanager, resourcemanager
>            Reporter: Ravi Teja Ch N V
>            Assignee: Devaraj K
>         Attachments: MAPREDUCE-3494.1.patch, MAPREDUCE-3494.2.patch, 
> MAPREDUCE-3494.patch, YARN-41-1.patch, YARN-41-2.patch, YARN-41-3.patch, 
> YARN-41.patch
> Instead of waiting for the NM expiry, RM should remove and handle the NM, 
> which is shutdown gracefully.

This message was sent by Atlassian JIRA

Reply via email to