[
https://issues.apache.org/jira/browse/YARN-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16679371#comment-16679371
]
Tao Yang edited comment on YARN-8925 at 11/8/18 7:23 AM:
---------------------------------------------------------
Thanks [~cheersyang] for the review and comments.
{quote}NodeLabelUtil#isNodeAttributesEquals
if leftNodeAttributes is a subset of rightNodeAttributes seems also equals.
And except for the name and value, we also need to compare prefix right?
It would be good if we have a separate UT for this method, to verify various
of cases.
{quote}
Comparison between two node attributes sets has considered set size through
this clause: {{leftNodeAttributes.size() != rightNodeAttributes.size())}} , and
considered attribute prefix in comparison inside for NodeAttributeKey
(reference code is {{nodeAttributes.stream().anyMatch(e ->
e.equals(checkNodeAttribute)}} in NodeLabelUtil#isNodeAttributeIncludes).
Separate UT for this method makes sense to me, I will add it in next patch.
{quote}HeartbeatSyncIfNeededHandler
Can we rename this to CachedNodeDescriptorHandler? As this class caches the
last value of node label/attribute and leverages the cache to reduce the
overhead.
{quote}
Agree, CachedNodeDescriptorHandler is a better name.
{quote}TestResourceTrackerService#testNodeRegistrationWithAttributes
File tempDir = File.createTempFile("nattr", ".tmp");
can we put tmp dir under TEMP_DIR that to be consistent with rest of tests.
{quote}
Make sense to me, I copied this code from testNodeHeartbeatWithNodeAttributes
and will update this method too.
{quote}TestNodeStatusUpdaterForAttributes
waitTillHeartbeat/waitTillHeartbeat
can these methods be simplified with GenericTestUtils.waitFor?
{quote}
Make sense to me.
I will upload a new patch a few hours later.
was (Author: tao yang):
Thanks [~cheersyang] for the review and comments.
{quote}NodeLabelUtil#isNodeAttributesEquals
if leftNodeAttributes is a subset of rightNodeAttributes seems also equals.
And except for the name and value, we also need to compare prefix right?
It would be good if we have a separate UT for this method, to verify various
of cases.
{quote}
Comparison between two node attributes sets has considered set size through
this clause: {{leftNodeAttributes.size() != rightNodeAttributes.size())}} , and
considered attribute name in comparison for NodeAttribute (reference code is
{{nodeAttributes.stream().anyMatch(e -> e.equals(checkNodeAttribute)}} in
NodeLabelUtil#isNodeAttributeIncludes).
Separate UT for this method makes sense to me, I will add it in next patch.
{quote}HeartbeatSyncIfNeededHandler
Can we rename this to CachedNodeDescriptorHandler? As this class caches the
last value of node label/attribute and leverages the cache to reduce the
overhead.
{quote}
Agree, CachedNodeDescriptorHandler is a better name.
{quote}TestResourceTrackerService#testNodeRegistrationWithAttributes
File tempDir = File.createTempFile("nattr", ".tmp");
can we put tmp dir under TEMP_DIR that to be consistent with rest of tests.
{quote}
Make sense to me, I copied this code from testNodeHeartbeatWithNodeAttributes
and will update this method too.
{quote}TestNodeStatusUpdaterForAttributes
waitTillHeartbeat/waitTillHeartbeat
can these methods be simplified with GenericTestUtils.waitFor?
{quote}
Make sense to me.
I will upload a new patch a few hours later.
> Updating distributed node attributes only when necessary
> --------------------------------------------------------
>
> Key: YARN-8925
> URL: https://issues.apache.org/jira/browse/YARN-8925
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Affects Versions: 3.2.1
> Reporter: Tao Yang
> Assignee: Tao Yang
> Priority: Major
> Labels: performance
> Attachments: YARN-8925.001.patch, YARN-8925.002.patch,
> YARN-8925.003.patch
>
>
> Currently if distributed node attributes exist, even though there is no
> change, updating for distributed node attributes will happen in every
> heartbeat between NM and RM. Updating process will hold
> NodeAttributesManagerImpl#writeLock and may have some influence in a large
> cluster. We have found nodes UI of a large cluster is opened slowly and most
> time it's waiting for the lock in NodeAttributesManagerImpl. I think this
> updating should be called only when necessary to enhance the performance of
> related process.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
