[
https://issues.apache.org/jira/browse/YARN-6858?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16360222#comment-16360222
]
Weiwei Yang edited comment on YARN-6858 at 2/12/18 2:47 AM:
------------------------------------------------------------
Hi [~Naganarasimha]
Except Jenkins warnings, the patch mostly looks good to me. Some comments
{code:java}
AttributeValue#compare(AttributeValue other, AttributeExpressionOperation op)
{code}
this API compares a value with another by an operator. But for IN/NOT_IN, it is
more like a set based comparison. That says for a given attribute value, we
need to check if it is in (or not in) a set of strings. In this case, this API
doesn't seem to be appropriate.
*NodeAttributePBImpl*
It seems you have included the changes from YARN-7892, have you checked the
comments I left on that one, see this comment? Basically I think we should have
a more restrictive equals implementation to avoid confusions.
*NodeAttributesManagerImpl*
# We need an UT for #validate, but I am OK to track this in a lower priority
JIRA.
# NodeLabelUtil.checkAndThrowLabelName(attribute.getAttributePrefix()); —
This pattern doesn't seem to allow DNS format of prefixes, could you please
double check.
Other seems good to me.
Thanks for the updates.
was (Author: cheersyang):
Hi [~Naganarasimha]
Except Jenkins warnings, the patch mostly looks good to me. Some comments
{code:java}
AttributeValue#compare(AttributeValue other, AttributeExpressionOperation op)
{code}
this API compares a value with another by an operator. But for IN/NOT_IN, it is
more like a set based comparison. That says for a given attribute value, we
need to check if it is in (or not in) a set of strings. In this case, this API
doesn't seem to be appropriate.
*NodeAttributePBImpl*
It seems you have included the changes from YARN-7892, have you checked the
comments I left on that one, see this comment? Basically I think we should have
a more restrictive equals implementation to avoid confusions.
*NodeAttributesManagerImpl*
# We need an UT for #validate, but I am OK to track this in a lower priority
JIRA.
# NodeLabelUtil.checkAndThrowLabelName(attribute.getAttributePrefix()); —
This pattern doesn't seem to allow DNS format of prefixes, could you please
double check.
# You still used {{Host}} abstraction, I am assuming you have tried and it
doesn't work if we read info, for example, just from RMNode.
Other seems good to me.
Thanks for the updates.
> Attribute Manager to store and provide the attributes in RM
> -----------------------------------------------------------
>
> Key: YARN-6858
> URL: https://issues.apache.org/jira/browse/YARN-6858
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: api, capacityscheduler, client
> Reporter: Naganarasimha G R
> Assignee: Naganarasimha G R
> Priority: Major
> Attachments: YARN-6858-YARN-3409.001.patch,
> YARN-6858-YARN-3409.002.patch, YARN-6858-YARN-3409.003.patch
>
>
> Similar to CommonNodeLabelsManager we need to have a centralized manager for
> Node Attributes too.
--
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
