[
https://issues.apache.org/jira/browse/YARN-7965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16376445#comment-16376445
]
Weiwei Yang edited comment on YARN-7965 at 2/26/18 6:46 AM:
------------------------------------------------------------
Thanks [[email protected]], [~sunilg]. Please help to review v2
patch.
bq. Ln no 187: change name from node to rmAttribute
Done
bq. ln no 318: why did we remove locking over here ?
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.
bq. I think its a simple check of prefix.isEmpty() should have been
!prefix.isEmpty() in my earlier code. Can we just have it in that way, it seems
to be concise ?
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.
bq. License and checkstyle issues
Fixed.
[~sunilg] regarding the logs, [[email protected]] mentioned that he
is working on a patch with more test cases and improvements, I assume that will
help. This patch is for unblock rest of my patches so it only has minimal
changes.
Hope that makes sense.
Thanks
was (Author: cheersyang):
Thanks [[email protected]], [~sunilg]. Please help to review v2
patch.
bq. Ln no 187: change name from node to rmAttribute
Done
bq. ln no 318: why did we remove locking over here ?
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.
bq. I think its a simple check of prefix.isEmpty() should have been
!prefix.isEmpty() in my earlier code. Can we just have it in that way, it seems
to be concise ?
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 it complicate
the code.
bq. License and checkstyle issues
Fixed.
[~sunilg] regarding the logs, [[email protected]] mentioned that he
is working on a patch with more test cases and improvements, I assume that will
help. This patch is for unblock rest of my patches so it only has minimal
changes.
Hope that makes sense.
Thanks
> 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: Major
> 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]
