[
https://issues.apache.org/jira/browse/YARN-7522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16275178#comment-16275178
]
Konstantinos Karanasos commented on YARN-7522:
----------------------------------------------
Thanks again for the patch, [~leftnoteasy]. Please find some comments below.
* Should we start the tags manager in all cases or should we have a parameter
for enabling the whole thing for the first version, given it will add some load
to the RM?
* The current getCardinality() tries to somehow impose constraints and at the
same time check cardinalities. When I do a getCardinality over a set of tags, I
am not sure it will be useful to get the max/min/sum of them. I would rather
have a method that for a set of tags returns a set of their cardinalities so
that my scheduler has the flexibility to decide what to do with them.
* I would add one more getCardinality method that gets a single string tag and
no binary operator just for simplicity.
* It might be useful to have an exists method for a tag. Like checking without
the exact cardinality whether this tag exists.
* In getCardinality, what is the else doing when the specified tags are null or
empty?
* In RMContainerImpl, the TODO for setting the allocation tags should be part
of this JIRA, right?
* We probably need to clean up the tags of all containers of an app when the
app finishes too. Will the current codepath take care of this even in cases of
app failures etc.?
* We should define a namespace for the appID, so that we can easily retrieve
it. Like {{appID:}} and then the actual ID.
Nits:
* In the PlacementConstraint(s) classes, we call the tags "allocation tags", so
I suggest to rename the corresponding classes in this JIRA to AllocationTags
instead of PlacementTags too.
* You can just have a boolean instead of the {{numValues}} in the
{{getCardinality}}.
* Why do we need the ImmutableSet in the add/removeContainer?
* In the {{removeTagsFromNode()}}, let's add some warn messages in case the
node was not found or the count is already 0. It will help to catch bugs.
> Add application tags manager implementation
> -------------------------------------------
>
> Key: YARN-7522
> URL: https://issues.apache.org/jira/browse/YARN-7522
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Wangda Tan
> Assignee: Wangda Tan
> Attachments: YARN-7522.YARN-6592.002.patch,
> YARN-7522.YARN-6592.003.patch, YARN-7522.YARN-6592.wip-001.patch
>
>
> This is different from YARN-6596, YARN-6596 is targeted to add constraint
> manager to store intra/inter application placement constraints. This JIRA is
> targeted to support storing maps between container-tags/applications and
> nodes. This will be required by affinity/anti-affinity implementation and
> cardinality.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
