[
https://issues.apache.org/jira/browse/YARN-6593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16081175#comment-16081175
]
Wangda Tan edited comment on YARN-6593 at 7/10/17 11:48 PM:
------------------------------------------------------------
Thanks [~kkaranasos],
I just took a closer look at the patch, some questions/comments (I
consolidated all my above questions/comments to this one, so you only need to
look at this comment and after).
1) For classes/methods are marked to be {{@public}}, it should be user-facing
APIs or wire-protocol-format. I'm not sure if we should mark following classes
to {{@private}}.
- Visitable/Visitor/PlacementConstraintTransformations
- PlacementConstraintToProtoConverter (Methods to transform a Constraint class
to protocol should not be {{@public}}, but behavior of the process should be
compatible).
And do you think we should explicitly add comment to following class to say
they are marked to {{@public}} only because of wire-format compatibility?
2) Is there any reason to put PlacementConstraints in {{hadoop-yarn-common}}
project instead of {{hadoop-yarn-api}}?
3) I'm not sure when you plan to use
{{SingleConstraintTransformer}}/{{SpecializedConstraintTransformer}}. If you
agree that they should not be user-facing, do you think we should move them to
a separate JIRA while doing implementation?
4) Can we revert changes to TestContainerLaunch?
5) In general: more javadocs need to be added for user-facing APIs, we can do
this once we have a general agreement on APIs.
= EDITs (July 10), added few more points: =
6) As I commented on YARN-6594, one of the biggest issue of old ResourceRequest
is it cannot properly handle resource update.
For example, at time T1 app request "host1" with #container=2, and at time T2
app decide to request "host1" for 2 more container, so it request "host1" with
#containers=4, however, at the same time, scheduler allocated 2 containers
already, so scheduler receive the request and update pending #containers=4.
When this happens, app can receive much more allocated containers than
originally requested.
There're two separate problems:
6.1) Partially update a placement request within the tree. To alleviate problem
above, we can partially update single or multiple placement constraints, by
doing this I think we can allocate unique-constraint-id for each contraint
internally and every time send diff to RM, end-user should not even notice that.
For example:
{code}
A placement constraint specified by user
and {
OR {
AND {
target-constraint { host IN "host1" },
cardinality-constraint { max_cardinality = 2}
},
AND {
target-constraint { host IN "host2" },
cardinality-constraint { max_cardinality = 4 }
}
}
{code}
unique-constraint-id (UCID) will be added in client and will be sent to server
for each constraint.
{code}
A placement constraint specified by user
and (ucid=1) {
OR (ucid=2) {
AND (ucid=3) {
target-constraint (ucid=4) { host IN "host1" },
cardinality-constraint (ucid=5) { max_cardinality = 2}
},
AND (ucid=6) {
target-constraint (ucid=7) { host IN "host2" },
cardinality-constraint (ucid=8) { max_cardinality = 4 }
}
}
{code}
If application updated max_cardinality to 3 for host1, only diff will be sent
to scheduler:
{code}
diff: [
{
cardinality-constraint (ucid=5) { max_cardinality = 3}
}
]
{code}
6.2) Increment/decrement #pending asks.
Only partially update constraint is not enough because it cannot avoid the
problem mentioned in 6). In addition to proposed in 6.1, do you think in the
protocol we can say: +2 for maximum-cardinality for ucid=5? When scheduler
received such requests, it will look at allocated-but-not-yet-acquired
containers to avoid allocate more containers than requested.
Requesting: [~asuresh]/[~subru]/[~chris.douglas]/[~sunilg] for more suggestions.
was (Author: leftnoteasy):
Thanks [~kkaranasos],
I just took a closer look at the patch, some questions/comments (I
consolidated all my above questions/comments to this one, so you only need to
look at this comment and after).
1) For classes/methods are marked to be {{@public}}, it should be user-facing
APIs or wire-protocol-format. I'm not sure if we should mark following classes
to {{@private}}.
- Visitable/Visitor/PlacementConstraintTransformations
- PlacementConstraintToProtoConverter (Methods to transform a Constraint class
to protocol should not be {{@public}}, but behavior of the process should be
compatible).
And do you think we should explicitly add comment to following class to say
they are marked to {{@public}} only because of wire-format compatibility?
2) Is there any reason to put PlacementConstraints in {{hadoop-yarn-common}}
project instead of {{hadoop-yarn-api}}?
3) I'm not sure when you plan to use
{{SingleConstraintTransformer}}/{{SpecializedConstraintTransformer}}. If you
agree that they should not be user-facing, do you think we should move them to
a separate JIRA while doing implementation?
4) Can we revert changes to TestContainerLaunch?
5) In general: more javadocs need to be added for user-facing APIs, we can do
this once we have a general agreement on APIs.
> [API] Introduce Placement Constraint object
> -------------------------------------------
>
> Key: YARN-6593
> URL: https://issues.apache.org/jira/browse/YARN-6593
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Konstantinos Karanasos
> Assignee: Konstantinos Karanasos
> Fix For: 3.0.0-alpha3
>
> Attachments: YARN-6593.001.patch, YARN-6593.002.patch,
> YARN-6593.003.patch, YARN-6593.004.patch
>
>
> This JIRA introduces an object for defining placement constraints.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
