[
https://issues.apache.org/jira/browse/YARN-1447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13835037#comment-13835037
]
Sandy Ryza commented on YARN-1447:
----------------------------------
Thanks Wangda. Some comments:
The new files are missing apache license headers
The word "Context" in ResourceChangeContext and ResourceIncreaseContext isn't
very informative. Can we instead call them ContainerResourceChange and
ContainerResourceIncrease?
(Container?)ResourceIncrease(Context?) sounds like a kind of
(Container?)ResourceChange(Context?), when in fact the former wraps the latter
with additional information. This seems a little confusing to me. Thinking
aloud, the design uses ResourceChange for four things: sending an increase
request from the AM to the RM, sending an increase allocation from the RM to
the AM, sending an increase from the AM to the NM, and sending a decrease from
the AM to the NM. I'm also worried that we might want to later add additional
information to an increase request that wouldn't make sense in the other
contexts.
I'm wondering whether we should get rid of ResourceChange entirely and add in a
ResourceIncreaseRequest proto? We would then have:
* AM->RM increase request: ContainerResourceIncreaseRequest, which includes a
ContainerId and a Resource.
* RM->AM increase allocation: ContainerResourceIncrease, which includes a
ContainerId, a Resource, and a Token
* AM->NM increase: ContainerResourceIncrease, as above.
* AM->NM decrease: ContainerResourceDecrease, which includes a ContainerId and
a Resource.
Thoughts? Sorry to change things up at this point, but as we won't be able to
modify this in the future, I think it's worth putting in the time to get it
right on the first try.
{code}
+
Assert.assertTrue(contextRecover.getExistingContainerId().equals(containerId));
{code}
Can use assertEquals here. This applies to a few other places as well.
> Common PB types define for container resource change
> ----------------------------------------------------
>
> Key: YARN-1447
> URL: https://issues.apache.org/jira/browse/YARN-1447
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: api
> Affects Versions: 2.2.0
> Reporter: Wangda Tan
> Assignee: Wangda Tan
> Attachments: yarn-1447.1.patch
>
>
> As described in YARN-1197, we need add some common PB types for container
> resource change, like ResourceChangeContext, etc. These types will be both
> used by RM/NM protocols
--
This message was sent by Atlassian JIRA
(v6.1#6144)
