[ https://issues.apache.org/jira/browse/YARN-2495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14181304#comment-14181304 ]
Naganarasimha G R commented on YARN-2495: ----------------------------------------- Thanks for reviewing Wangda : bq. 2) It seems NM_LABELS_FETCH_INTERVAL_MS not been used in the patch, did you forget to do that? -- Earlier was planning to make node labels script only to be dynamic and configruation based as static. Now based on your comment 4 will make it dynamic and change the configuration name too. bq. 3) Regarding ResourceTrackerProtocol, I think NodeHeartbeatRequest should only report labels when labels changed. So there're 3 possible values of node labels in NodeHeartbeatRequest ... And RegisterNodeManagerRequest should report label every time registering. -- Yes this was my plan and will be doing it in the same way. But was thinking about one sceanario labels got changed and on call to NodeLabelsProvider.getLabels() it returns the new labels but the heartbeat failed due to some reason. in that case NodeLabelsProvider will not be able to detect this and on next request to getLabels() it will return null. So we should have some mechanism such that NodeLabelsProvider are informed whether RM accepted the change in labels so that appropriate SET of labels are provided on call to getLabels (also if needed we can have RM Rejected Labels too for logging purpose) Planning to have 3 interfaces in NodeLabelsProvider * getNodeLabels() : to get the labels which can be used for registration * getNodeLabelsOnModify() : to get the labels on modification which can be used for heartbeat * rmUpdateNodeLabelsStatus(boolean success) : to indicate that next call to getNodeLabelsOnModify can be reset to null bq. 4.1 Why this class extends from CompositeService? Did you want to add more component to it? If not, AbstractService should be enough. If the purpose of the NodeLabelsFetcherService is only create a NodeLabelsProvider, and the NodeLabelsProvider will take care of periodically read configuration from yarn-site.xml.I suggest to rename NodeLabelsFetcherService to NodeLabelsProviderFactory, and not extends from any Service, because the NodeLabelsProvider should be a Service. Rename NodeLabelsProvider to NodeLabelsProviderService if your purpose is as what I mentioned. -- Your idea seems to be better, will try to do it in the way you have specified and hence NodeLabelsFetcherService will become factory or i will make it absolute. ConfigurationNodeLabelsProvider : will make it dynamic. i,e. periodically it will read the yarn-site and get the Labels. {quote} 6) More implementation suggestions: Since we need central node labels configuration, I suggest to leverage what we already have in RM admin CLI directly – user can use RM admin CLI add/remove node labels. We can disable this when we're ready to do non-central node label configuration.And there should be an option to tell if distributed node label configuration is used. If it's distributed, AdminService should disable admin change labels on nodes via RM admin CLI. I suggest to do this in a separated JIRA. {quote} -- I presume "central node labels configuration" as "Cluster Valid Node Labels" stored at RM side for validation of labels if so ok will do it in the same way as that of RM Admin CLI and for ??If it's distributed, AdminService should disable admin change labels on nodes via RM admin CLI?? will add a jira, but was wondering how to do this ? by configuration with new parameter? I was earlier under the impression as MemoryRMNodeLabelsManager => is for distributed Configuration and RMNodeLabelsManager is for Centrallized configuration. and some factory will take care of this.... Other comments will handle > Allow admin specify labels in 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_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 or using script > suggested by [~aw]) > - 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 (v6.3.4#6332)