[
https://issues.apache.org/jira/browse/YARN-7757?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16349563#comment-16349563
]
Naganarasimha G R commented on YARN-7757:
-----------------------------------------
{quote}bq. Does this need to be run under some privileged mode or something
similar ? As per current impl, this script will be run as NodeManager user.
{quote}
IMO i think issues might come when system level information needs to be
collected. So presume it might require to run as user. This we need to discuss
more. Hope to track this discussion in another jira
{quote}bq. multi scripts for different types of attributes
{quote}
Needs more thought. As its single Admin doing this, he can ensure all the
scripts are clubbed together. And also passing of options would be complicated
anyway can be discussed in detail in another jira as mentioned by Weiwei.
Comments on other classes.
NodeManager:
* ln no 171: It looks like it can create only one of the provider, either for
labels or for attributes. I think we need to explicitly support for both.
NodeStatusUpdate:
* Similar to above we might need to support both the labels and attributes to
be fetched in a distributed way
AbstractNodeDescriptorsProvider
* ln no 149 : verifyConfiguredScript seems to be out of place. We need to
either move it to a util class or make it as static method and used across
other class
* ln no 85 serviceStart needs to capture that taskInterval needs to be set
before the service is started to ensure that the execcutor service properly
initialised
* ln no 87 Lets use scheduledexecutorservice instead of timer task as its not
consistent in changes to system clock. Also it takes care internally that if
earlier task is already running it avoids to spawn new one
ScriptBasedNodeAttributesProvider.java
* ln no 102 : // NODE_ATTRIBUTE:ATTRIBUTE_NAME,ATTRIBUTE_TYPE,ATTRIBUTE_VALUE.
I think we need to follow the same pattern as [prefix/]key[:type][=value] as
mentioned in the doc so that its in sync with the parsing of cli. And at the
same time helps to specify the type and prefix. But given that we were deciding
to put the prefix automatically for the distirubted labels then syntax can be
updated and captured well in the document. May be we can take this in another
jira as focus here is more towards refactoring
I observed on intresting thing here we are using Set<NodeAttribute> but i think
I have not handled equals and hash properly. IMO prefix, name type should
determine the uniqueness and value should not be included. I think i will raise
one more jira to handle this so that 7840 is not blocked
> Refactor NodeLabelsProvider to be more generic and reusable for node
> attributes providers
> -----------------------------------------------------------------------------------------
>
> Key: YARN-7757
> URL: https://issues.apache.org/jira/browse/YARN-7757
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager
> Reporter: Weiwei Yang
> Assignee: Weiwei Yang
> Priority: Blocker
> Attachments: YARN-7757-YARN-3409.001.patch,
> YARN-7757-YARN-3409.002.patch, YARN-7757-YARN-3409.003.patch,
> YARN-7757-YARN-3409.004.patch, YARN-7757-YARN-3409.005.patch,
> nodeLabelsProvider_refactor_class_hierarchy.pdf
>
>
> Propose to do refactor on {{NodeLabelsProvider}},
> {{AbstractNodeLabelsProvider}} to be more generic, so node attributes
> providers can reuse these interface/abstract classes.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
