hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Wangda Tan (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3964) Support NodeLabelsProvider at Resource Manager side
Date Mon, 21 Sep 2015 21:14:06 GMT

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

Wangda Tan commented on YARN-3964:
----------------------------------

[~dian.fu].

Some comments:
1) I suggest to make this to be an explicit node label configuration type: {{yarn.node-labels.configuration-type}}.
Currently it has "centralized/distributed", I think you may add a "delegated-centralized"
(or other better name).
Other configurations in your patch look fine to me.

2) Some comments of organization of Updater/Provider:
- Updater is a subclass of AbstractService, but no need to be abstract. I'm not sure what's
the purpose of adding an AbstractNodeLabelsUpdater. Provider will be initialized by Updater,
and Updater will call Provider's method periodically and notify RMNodeLabelsManager.
- Provider is an interface, minor comments to your patch:
** Why need a Configuration in getNodeLabels method?
** Returns Set<NodeLabel> instead of Set<String>

3) There're some methods / comments include "Fetcher", could you replace them to "Provider"?

4) Instead of adding a new checkAndThrowIfNodeLabelsFetcherConfigured, I suggest to reuse
the checkAndThrowIfDistributedNodeLabelConfEnabled: You can rename it to something like checkAndThrowIfNodeLabelCannotBeUpdatedManually,
which will check {{yarn.node-labels.configuration-type}}, we only allow manually update labels
when type=centralized configured.

5) You can add a method to get RMNodeLabelsUpdater from RMContext, and remove it from ResourceTrackerService
constructor.

6) Add a test of RMNodeLabelsUpdater? It seems can only update labels-on-node once for every
node.

7) I think we need to make sure labels will be updated *synchronizedly* when a node is registering,
this can avoid a node's labels initialized after it registered a while.

8) If you agree with #7, I think wait/notify implementation of Updater could be removed, you
can use synchronized lock instead. Code using wait/notify has bad readability and will likely
introduce bugs.

Thanks,
Wangda

> Support NodeLabelsProvider at Resource Manager side
> ---------------------------------------------------
>
>                 Key: YARN-3964
>                 URL: https://issues.apache.org/jira/browse/YARN-3964
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Dian Fu
>            Assignee: Dian Fu
>         Attachments: YARN-3964 design doc.pdf, YARN-3964.002.patch, YARN-3964.003.patch,
YARN-3964.004.patch, YARN-3964.005.patch, YARN-3964.006.patch, YARN-3964.1.patch
>
>
> Currently, CLI/REST API is provided in Resource Manager to allow users to specify labels
for nodes. For labels which may change over time, users will have to start a cron job to update
the labels. This has the following limitations:
> - The cron job needs to be run in the YARN admin user.
> - This makes it a little complicate to maintain as users will have to make sure this
service/daemon is alive.
> Adding a Node Labels Provider in Resource Manager will provide user more flexibility.



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

Mime
View raw message