[
https://issues.apache.org/jira/browse/YARN-7965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16376903#comment-16376903
]
Naganarasimha G R edited comment on YARN-7965 at 2/26/18 2:03 PM:
------------------------------------------------------------------
Thanks [~cheersyang] for the latest patch,
{quote}If user provide a non-empty prefix set, and this set contains only 1
prefix which is not a valid one, the old logic will return all attributes to
the client, this is a bug. Hence refactor the logic and I don't think the
change complicate the code.
{quote}
Agree that it was a bug no doubt about it, but it was just missing a negation
and i just mentioned that earlier approach was concise but anyway both the
approaches works fine
{quote}I don't think the read lock is needed. It was protecting the read access
to a ConcurrentHashMap, and the map is thread safe. Adding an extra lock is
redundant.
{quote}
Yes it works over a concurrent Hashmap but replace operation removes and adds
so intermittently if some one is accessing it then they will be reading a
invalid state of information. hence added that. but anyway will relook into it
in my jira
Other than that its fine, but latest patch misses the test class, can you look
into it ?
was (Author: naganarasimha):
Thanks [~cheersyang] for the latest patch,
{quote}If user provide a non-empty prefix set, and this set contains only 1
prefix which is not a valid one, the old logic will return all attributes to
the client, this is a bug. Hence refactor the logic and I don't think the
change complicate the code.
{quote}
Agree that it was a bug no doubt about it, but it was just missing a negation
and i just mentioned that earlier approach was concise but anyway both the
approaches works fine
{quote}I don't think the read lock is needed. It was protecting the read access
to a ConcurrentHashMap, and the map is thread safe. Adding an extra lock is
redundant.
{quote}
Yes it works over a concurrent Hashmap but replace operation removes and adds
so intermittently if some one is accessing it then they will be reading a
invalid state of information. hence added that. but anyway will relook into it
in my jira
Other than that its fine, As its required to unblock you, i am going ahead and
committing it.
> NodeAttributeManager add/get API is not working properly
> --------------------------------------------------------
>
> Key: YARN-7965
> URL: https://issues.apache.org/jira/browse/YARN-7965
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Weiwei Yang
> Assignee: Weiwei Yang
> Priority: Critical
> Attachments: YARN-7965-YARN-3409.001.patch,
> YARN-7965-YARN-3409.002.patch
>
>
> Fix following issues,
> # After add node attributes to the manager, could not retrieve newly added
> attributes
> # Get cluster attributes API should return empty set when given prefix has
> no match
> # When an attribute is removed from all nodes, the manager did not remove
> this mapping
> and add UT
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
