[
https://issues.apache.org/jira/browse/YARN-8925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16678431#comment-16678431
]
Weiwei Yang edited comment on YARN-8925 at 11/7/18 3:58 PM:
------------------------------------------------------------
Hi [~Tao Yang]
Thanks for the patch. It's a nice refactor in {{NodeStatusUpdaterImpl}}, looks
pretty good. And also thanks for adding unit tests, the coverage seems good
too. Some comments,
*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.
*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.
*TestResourceTrackerService#testNodeRegistrationWithAttributes*
{code:java}
File tempDir = File.createTempFile("nattr", ".tmp");
{code}
can we put tmp dir under {{TEMP_DIR}} that to be consistent with rest of tests.
*TestNodeStatusUpdaterForAttributes*
waitTillHeartbeat/waitTillHeartbeat
can these methods be simplified with GenericTestUtils.waitFor?
Thanks
was (Author: cheersyang):
Hi [~Tao Yang]
Thanks for the patch. It's a nice refactor in {{NodeStatusUpdaterImpl}}, looks
pretty good. And also thanks for adding unit tests, the coverage seems good
too. Some comments,
*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.
*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.
*TestResourceTrackerService#testNodeRegistrationWithAttributes*
{code}
File tempDir = File.createTempFile("nattr", ".tmp");
{code}
can we put tmp dir under \{{TEMP_DIR}} that to be consistent with rest of tests.
*TestNodeStatusUpdaterForAttributes*
waitTillHeartbeat/waitTillHeartbeat
can these methods be simplified with GenericTestUtils.waitFor?
Thanks
> 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]
