[
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)
