[
https://issues.apache.org/jira/browse/YARN-7863?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16584798#comment-16584798
]
Weiwei Yang commented on YARN-7863:
-----------------------------------
Hi [~sunilg]
Thanks for the update, but I am afraid this needs some more polish.
*PlacementConstraintParser.java*
line 417: getConstraintNameWithNameSpace() defaults the namespace (when it is
missing) to "rm.yarn.io", why we have such assumptions in the parser? If some
other days we add more namespace types, we will need to change the code here
too.
line 397- 400: this confuses me.
{code:java}
target = PlacementConstraints.PlacementTargets.nodeAttribute(attributeName,
constraintEntities.toArray(new String[constraintEntities.size()]));
{code}
If I specify javaVersion=1.6, this seems to be setup as
{code:java}
nodeAttribute("javaVersion", "javaVersion=1.6")
{code}
this is incorrect. This API should be
{code:java}
// Single value
nodeAttribute("javaVersion", "1.6")
// Multiple values
nodeAttribute("javaVersion", "1.6", "1.7", "1.8")
{code}
You can take a look the original form in line 76 of
\{{TestPlacementConstraintManagerService#}}. That said, the attributeValues
should not have OPERATOR, it's just values. We will someday add a "Operator"
field in \{{SingleConstraint}} when we fully support various of operations,
such as GT, LT etc. Lets not change the meaning of the API. Note, if we correct
this, the logic in \{{PlacementConstraintsUtils}} will also need to be updated.
*PlacementConstraint.java*
We don't need to add \{{constraintKey}} in \{{AbstractConstraint}}, this is the
base class for constraints, it could be \{{SignleConstraint}}, also could be
\{{CompositeConstraint}}. You'll find this field is meaningless if it is
\{{CompositeConstraint}}.
For node-attribute expr, if allocation tags is not given, then we don't need
this info in {{SchedulingRequest, Set<String> allocationTags }}argument can be
null or an empty set.
*SingleConstraintAppPlacementAllocator.java*
I don't think we need to get node-attributes in
SingleConstraintAppPlacementAllocator. For logging purpose, we can just print
PC, that will print everything we need. Because 1) if the PC is a composite
one, multiple node-attributes specified, which one will be used? 2) PC could be
affinity or anti-affinity, just memorize the attributes is not helpful.
Some minor logging issues, there are some logs need to be removed or change to
DEBUG level,
* LocalityAppPlacementAllocator.java: line 369
* NodeAttributeManagerImpl.java: line 205, 222
* RegularContainerAllocator.java: line 144, 148, 155
Hopefully this makes sense.
Thanks
> Modify placement constraints to support node attributes
> -------------------------------------------------------
>
> Key: YARN-7863
> URL: https://issues.apache.org/jira/browse/YARN-7863
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Sunil Govindan
> Assignee: Sunil Govindan
> Priority: Major
> Attachments: YARN-7863-YARN-3409.002.patch,
> YARN-7863-YARN-3409.003.patch, YARN-7863-YARN-3409.004.patch,
> YARN-7863-YARN-3409.005.patch, YARN-7863-YARN-3409.006.patch,
> YARN-7863.v0.patch
>
>
> This Jira will track to *Modify existing placement constraints to support
> node attributes.*
--
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
