Naganarasimha G R commented on YARN-2495:

Hi [~cwelch]
Thanks for taking time and reviewing.
bq. This can already be accomplished using the admin cli from a script or 
calling a web service from the node
IIUC the current way to accomplish running the script is through configuring 
the label script in health monitor script ? please correct me if i am wrong.
If it is so i see following problems
* It seems more like a workaround solution(or more like break in the health 
check functionality) to get the same thing acheived 
* One important point as [~aw] mentioned developer needs to take care of 
security issues which might be too much effort
* Feels like extra effort needs to be put by the developer to get it done which 
takes care of security, error handling , retries etc ... apart from the actual 
script which might be very simple
* Documentation and conveying the functionality to the users would be difficult
* What if admin wants both health script and node labels script ? 

bq. Is this all just to support putting labels into the node-managers 
configuration file and introducing them that way? Do we have a solid need for 
that? It's not needed for the dynamic script case, which is all I've seen 
discussed here from a "requirements" perspective (putting it into the config 
file / adding it to the heartbeat is implementation, I don't see a requirement 
for it as such).
As [~aw] mentioned for this, main idea is not to support through configuration 
file but run the configured script periodically and to directly update the RM 
when change in the labels. Also it can do the error handling as required if RM 
doesn't accept it.

bq. but I don't see any reason to have conditional enablement of the node 
manager side of things as well as a provider specification and I think it just 
adds unnecessary complexity and possible surprise at configuration time
Well this topic is debatable, but few points which i feel to be addressed if we 
want to support both central and distributed simultaneously :
# There will be always confusion which labels needs to be taken suppose user 
updates some labels for a node and NM label script doesnt have them then which 
do we give priority ?
# Do we need to "And" or "OR" when the labels are updated either by script /by 
admin CLI / REST web service

AFAIU [~gp.leftnoteasy] wanted to do these in stages and in the first stage to 
avoid confusion it would be better to have mutually exclusive i.e either 
central or distributed will be supported and if its distributed CLI 
modifications will be disabled (YARN-2728) and if central, labels from 
heartbeat will be discarded.

+1 for changes which you mentioned in {{NodeHeartbeatRequest}} & 
{{RegisterNodeLabelManagerResponse}} but for {{RegisterNodeManagerRequest}} yes 
it would be easy to read the interface if both are same but as explained in 
previous point if either one of them(central/distributed) then in case of 
distributed it anyway needs to send what ever it has and non distibuted case RM 
doesnt care abt it. So there was no need for it.

It would be helpful if  [~gp.leftnoteasy] & [~vinodkv] can further provide 
their views on the requirement level comments which has been raised by [~cwelch]

> Allow admin specify labels from each NM (Distributed configuration)
> -------------------------------------------------------------------
>                 Key: YARN-2495
>                 URL: https://issues.apache.org/jira/browse/YARN-2495
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>            Reporter: Wangda Tan
>            Assignee: Naganarasimha G R
>         Attachments: YARN-2495.20141023-1.patch, YARN-2495.20141024-1.patch, 
> YARN-2495.20141030-1.patch, YARN-2495.20141031-1.patch, 
> YARN-2495.20141119-1.patch, YARN-2495.20141126-1.patch, 
> YARN-2495.20141204-1.patch, YARN-2495.20141208-1.patch, 
> YARN-2495_20141022.1.patch
> Target of this JIRA is to allow admin specify labels in each NM, this covers
> - User can set labels in each NM (by setting yarn-site.xml (YARN-2923) or 
> using script suggested by [~aw] (YARN-2729) )
> - NM will send labels to RM via ResourceTracker API
> - RM will set labels in NodeLabelManager when NM register/update labels

This message was sent by Atlassian JIRA

Reply via email to