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

Junping Du commented on YARN-3225:
----------------------------------

Thanks [~devaraj.k] for delivering the patch which is the first one in graceful 
decommission effort!
A couple of comments:
In RefreshNodesRequestPBImpl.java, 
{code}
   @Override
+  public long getTimeout() {
+    return getProto().getTimeout();
+  }
+
+  @Override
+  public void setTimeout(long timeout) {
+    builder.setTimeout(timeout);
+  }
{code}
The setTimeout() has problem because we didn't set viaProto to false, so if we 
getTimeout() afterwards then it will return the old value from old proto. 
Suggest to add a method of maybeInitBuilder() just like other PBImpls, also add 
a unit test to verify the PBImpl works as expected.

In NodeState.java,
{code}
DECOMMISSION_IN_PROGRESS
{code}
[~jlowe] suggested in umbrella JIRA that it is better to be DECOMMISSIONING. I 
had the same feeling so reflect the name in latest proposal. Do you think we 
should incorporate that comments here?

In RMAdminCLI.java,
{code}
+          .put("-refreshNodes", new UsageInfo("[-g [timeout in ms]]",
               "Refresh the hosts information at the ResourceManager."))
{code}
I think we should add more info to description message - "Refresh the hosts 
information at the ResourceManager." to explain -g option doing. Isn't it? 
Also, per my suggestion above, it is better to specify seconds in timeout. MS 
is more precisely, but get more chance for wrong (manually) operation.

Also, it is better to change the patch name to be consist with JIRA number 
(YARN-3225).

> New parameter or CLI for decommissioning node gracefully in RMAdmin CLI
> -----------------------------------------------------------------------
>
>                 Key: YARN-3225
>                 URL: https://issues.apache.org/jira/browse/YARN-3225
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Junping Du
>            Assignee: Devaraj K
>         Attachments: YARN-914.patch
>
>
> New CLI (or existing CLI with parameters) should put each node on 
> decommission list to decommissioning status and track timeout to terminate 
> the nodes that haven't get finished.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to