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

Naganarasimha G R commented on YARN-4855:
-----------------------------------------

[~Tao Jie], thanks for patiently redoing on most of the rework. 
bq. It looks to me that verify nodes on server side(actually I did with this 
approach in the very earlier patch).
Had forgotten your initial approach thanks for pointing it out i think you 
started to differ the approach after wangda's comment and [~wangda] also has 
already reviewed your latest patch so i think even he too is fine with your 
latest approach.

bq. I think we also need to check getInactiveRMNodes, node which is 
decommissioning should be treated as known node
Yes wangda, i think it would be good to support this too and i think its been 
covered in the latest patch

Latest patch seems to be good with approach just few small nits :
# The way we are handling "--fail-on-unknown-nodes" is not correct in the CLI, 
i had just typed wrongly while testing as  "--fail-on-unkown-nodes" so it 
silently passes taking  "--fail-on-unkown-nodes" as the host name as per the 
existing code. For handling of additional options in a command we need to 
follow the approach similiar to one present in nodeCLI where we take from 
command parser {{cliParser.hasOption(NODE_SHOW_DETAILS)}} instead from the 
position, so that on wrong command it pops up an error
# if you agree on above we need to change "--fail-on-unknown-nodes" to 
"-fail-on-unknown-nodes". Also would like to see whether camel casing is better 
than this
# AdminService. ln no 850 "Replace labels on unknown nodes:" => "Failed to 
replace labels as there are unknown nodes:" would be better as we fail updating 
for all the mappings
# test failure message "Should not fail on unknown node when not verify nodes" 
on unknown node" instead "Should not fail on unknown node when 
fail-on-unkown-nodes is set false" 

> Should check if node exists when replace nodelabels
> ---------------------------------------------------
>
>                 Key: YARN-4855
>                 URL: https://issues.apache.org/jira/browse/YARN-4855
>             Project: Hadoop YARN
>          Issue Type: Improvement
>    Affects Versions: 2.6.0
>            Reporter: Tao Jie
>            Assignee: Tao Jie
>            Priority: Minor
>         Attachments: YARN-4855.001.patch, YARN-4855.002.patch, 
> YARN-4855.003.patch, YARN-4855.004.patch, YARN-4855.005.patch, 
> YARN-4855.006.patch, YARN-4855.007.patch, YARN-4855.008.patch, 
> YARN-4855.009.patch, YARN-4855.010.patch, YARN-4855.011.patch
>
>
> Today when we add nodelabels to nodes, it would succeed even if nodes are not 
> existing NodeManger in cluster without any message.
> It could be like this:
> When we use *yarn rmadmin -replaceLabelsOnNode --fail-on-unkown-nodes 
> "node1=label1"* , it would be denied if node is unknown.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to