[
https://issues.apache.org/jira/browse/YARN-8103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16501238#comment-16501238
]
Naganarasimha G R edited comment on YARN-8103 at 6/5/18 3:53 AM:
-----------------------------------------------------------------
Thanks for the patch and apologies for the delayed response.
Major Comments :
* I agree with the approach of not storing and updating the attributes in
RMNode and instead request NodeAttributesManager to share the information. But
was also wondering whether the API(RMNodeImpl.getAllNodeAttributes) is useful
based on current scenarios ? All the callers are eventually converting it into
a set of attributes and utilizing it. So i would prefer to change the api to
just return the set of attributes applicable on a node and when needed let the
caller take care of sorting based on the prefix( which is anyway not a current
scenario)
Few other comments:
* hadoop-yarn/bin/yarn ln no 58: i think "client" was missing from earlier
which we need to add it
* NodeAttributesCLI ln no 195: i think its better to use null here instead of
"handler" for better readability
* NodeAttributesCLI ln no 88,96 : unused variables
* TestNodeAttributesCLI ln no 405: testListAttributes is encapsulating
NodesToAttributes tests too, may be it can captured as a different case
* NodeAttributesCLI ln no 355 : HashSet => HashSet<NodeAttributeKey>
* NodeAttributesCLI ln no 394 : HashSet => HashSet<String>
* NodeCLI ln no 349: formatNodeAttribute : instead of having new formatting
here why not update NodeAttributePBImpl.toString ?
* NodeCLI ln no 317 : IMO it would better to wrap each attribute in a new line
?
* TestClusterCLI : does not capture any case for listing of attributes
(basically need to capture multiline)
* BuilderUtils ln no 211 : space is required before Set
Some of the findbugs and checkstyle seems to be valid can you have a look at
them ?
Kudos NodeAttributesCLI has been handled well !
was (Author: naganarasimha):
Thanks for the patch and apologies for the delayed response.
Major Comments :
* I agree with the approach of not storing and updating the attributes in
RMNode and instead request NodeAttributesManager to share the information. But
was also wondering whether the API(RMNodeImpl.getAllNodeAttributes) is useful
based on current scenarios ? All the callers are eventually converting it into
a set of attributes and utilizing it. So i would prefer to change the api to
just return the set of attributes applicable on a node and when needed let the
caller take care of sorting based on the prefix( which is anyway not a current
scenario)
Few other comments:
* hadoop-yarn/bin/yarn ln no 58: i think "client" was missing from earlier
which we need to add it
* NodeAttributesCLI ln no 195: i think its better to use null here instead of
"handler" for better readability
* NodeAttributesCLI ln no 88,96 : unused variables
* TestNodeAttributesCLI ln no 405: testListAttributes is encapsulating
NodesToAttributes tests too, may be it can captured as a different case
Some of the findbugs and checkstyle seems to be valid can you have a look at
them ?
Kudos NodeAttributesCLI has been handled well !
> Add CLI interface to query node attributes
> -------------------------------------------
>
> Key: YARN-8103
> URL: https://issues.apache.org/jira/browse/YARN-8103
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Bibin A Chundatt
> Assignee: Bibin A Chundatt
> Priority: Major
> Attachments: YARN-8103-YARN-3409.001.patch,
> YARN-8103-YARN-3409.002.patch, YARN-8103-YARN-3409.WIP.patch
>
>
> YARN-8100 will add API interface for querying the attributes. CLI interface
> for querying node attributes for each nodes and list all attributes in
> cluster.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
