hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Naganarasimha G R (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-2729) Support script based NodeLabelsProvider Interface in Distributed Node Label Configuration Setup
Date Mon, 04 May 2015 13:25:12 GMT

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

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

Thanks for the review comments [~vinodkv],  

bq.SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them,
you have separate constants for their prefixes
Actually these are not preffixes, as per [~Wangda]'s [comment|https://issues.apache.org/jira/browse/YARN-2729?focusedCommentId=14393545&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14393545]
we had decided have whitelisting for provider : {{The option will be: yarn.node-labels.nm.provider
= "config/script/other-class-name".}} . These are modifications for it.

bq. DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration
Well As per one of the wangda's comment he had mentioned that possible values or default values
of configurations had to be kept in YARNConfiguration, hence had placed it here, if required
as per your comment can move it to AbstractNodeLabelsProvider

bq. LOG is not used anywhere
Are the logs expected when the labels are set in {{setNodeLabels}} ? i can add here but anyway
there were logs in NodeStatusUpdaterImpl on successfull and unsuccessfull attempts.

bq. BTW, assuming YARN-3565 goes in first, you will have to make some changes here.
bq. I think the format expected from the command should be more structured. Specifically as
we expect more per-label attributes in line with YARN-3565.
Well was thinking about this while working on YARN-3565, but dint modify the NodeLabelsProvider
as currently Labels(currently partitions) which needs to be sent from NM have to be one of
RM's CLUSTER NodeLabel set. So exclusiveness need not be sent from NM to RM as currently we
support specifying the exclusiveness only during adding clusterNode labels. So IMHO if there
is plan to make this interface public & stable then would be better do these changes now
itself if not it would better done after requirement for constraint labels, so that more clarity
on structure would be there? 
[~wangda] and you can share your opinion on this, based on it will do the modifications.

bq. Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry
about invalid label-set, previous-valid-labels and label validation. You should move all that
functionality into NodeLabelsProvider.
Well as per the class reponsibility i understand that NodeStatusUpdaterImpl is not supposed
to have it but as it might be expected to be public we had to ensure that
* For every heartbeat labels are sent across only if modified
* doing basic validations before sending the modified labels

These needs to be done irrespective of the label provider (system or user's) hence kept it
in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate
manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider.
Those changes were also from my previous patch, so no hard feelings in taking care of it if
req :).

bq. Can you add the documentation for setting this up too too?
Well was planning to raise jira for updating documentation on top of NodeLabels but documentation
for it is not yet completed. If required can just add some pdf here

> Support script based NodeLabelsProvider Interface in Distributed Node Label Configuration
Setup
> -----------------------------------------------------------------------------------------------
>
>                 Key: YARN-2729
>                 URL: https://issues.apache.org/jira/browse/YARN-2729
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager
>            Reporter: Naganarasimha G R
>            Assignee: Naganarasimha G R
>         Attachments: YARN-2729.20141023-1.patch, YARN-2729.20141024-1.patch, YARN-2729.20141031-1.patch,
YARN-2729.20141120-1.patch, YARN-2729.20141210-1.patch, YARN-2729.20150309-1.patch, YARN-2729.20150322-1.patch,
YARN-2729.20150401-1.patch, YARN-2729.20150402-1.patch, YARN-2729.20150404-1.patch
>
>
> Support script based NodeLabelsProvider Interface in Distributed Node Label Configuration
Setup . 



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

Mime
View raw message