[
https://issues.apache.org/jira/browse/YARN-1447?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13837492#comment-13837492
]
Sandy Ryza commented on YARN-1447:
----------------------------------
Thanks Wangda. The proto definitions look good to me now. Just some stylistic
comments on the patch:
{code}
+abstract public class ContainerResourceDecrease {
{code}
"public" should come before "abstract". Also applies to a couple other places.
{code}
+ * Used by AM to ask NM reduce size of a specified container
{code}
Better to spell it out - Used by application masters to as NodeManagers to
reduce the size of a specified container. The same applies to a few different
places.
{code}
+ if ((getContainerId() == null) ^ (ctx.getContainerId() == null)) {
{code}
It's more traditional to use "||" which can short-circuit, than "^". Applies
to a couple other places as well.
{code}
+ ContainerResourceIncreaseRequestProto proto =
((ContainerResourceIncreaseRequestPBImpl)context).getProto();
+ ContainerResourceIncreaseRequest contextRecover = new
ContainerResourceIncreaseRequestPBImpl(proto);
{code}
Way over 80 characters.
> 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, yarn-1447.2.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)
