Naganarasimha G R commented on YARN-3964:

Few concerns what i have with this approach is :
* {{CommonNodeLabelsManager.isDistributedNodeLabelConfiguration}} is set in 
distributed mode and we avoid reading and storing to 
{{FileSystemNodeLabelsStore}}. But in your case its not done, hence LabelStore 
Editlogs goes on increasing over time as we not checking for weather the labels 
are not modified for a given node while replacing. Eventually 
FileSystemNodeLabelsStore.recover during startup(/failover) might be come slow.
* During update even if one node has Labels which is not part of CLuster Labels 
it fails to update for other nodes is that fine ?

few nits :
# RMNodeLabelsUpdater
#* IMO we can name this class as 
#* {{NodeLabelsUpdaterThread}} can it be better handled using TimerTask or even 
Better have ScheduledThreadPoolExecutor which doesnt have impact of change in 
internal clock time and on any exception in the task, Executor itself doesnt 
#* In {{NodeLabelsUpdaterThread.run}}, On exception during update i think we 
should log {{LOG.error}} instead of warn
#* can we merge {{nodeLabelsUpdated}} and {{updateNodeLabelsInternal}}, as 
nodeLabelsUpdated is used only by  updateNodeLabelsInternal
# AdminService & RMWebServices
#* {{checkAndThrowIfCentralizedNodeLabelConfNotEnabled}} => 
{{verifyCentralizedNodeLabelConfEnabled}} and moved to NodeLabelsUtils
# NodeLabelsProvider
#* The name of the interface seems to be same with the one in NM, i feel we can 
name it better as {{RMNodeLabelsProvider}}/{{NodeLabelsMappingProvider}} to 
avoid confusions

> 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.007.patch, YARN-3964.007.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

Reply via email to