Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: yarn-issues@hadoop.apache.org
Date: Fri, 13 Dec 2013 02:19:08 +0000 (UTC)
From: "Junping Du (JIRA)" <jira@apache.org>
To: yarn-issues@hadoop.apache.org
Message-ID: <JIRA.12625971.1357271947186.29813.1386901148395@arcas>
In-Reply-To: <JIRA.12625971.1357271947186@arcas>
References: <JIRA.12625971.1357271947186@arcas>
Subject: [jira] [Commented] (YARN-312) Add updateNodeResource in
 ResourceManagerAdministrationProtocol
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/YARN-312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13847062#comment-13847062 ] 

Junping Du commented on YARN-312:
---------------------------------

Thanks [~vinodkv] for review and comments! All points make sense to me. Please see my reply.
bq. The patch isn't applying anymore. Please update.
Sure. Will update in next patch.
bq. There's a better way to implement the map. See ApplicationACLMapProto in yarn_protos.proto for example and its usage. This should avoid the length checks in AdminService. In a similar vein, the java APIs can directly deal with maps.
Thanks for your suggestion here. Yes. That seems better, will update in next patch.
bq. Didn't review the previous patches, but I think we should have a better name instead of ResourceOption. Will file a JIRA.
Yes. Please share your idea there. Thanks.
bq. The UpdateNodeResourceRequest and response objects need to be @Public too?
Yes. Nice catch. Will change it to public.
bq. Failure handling: If there is an invalid node, should we reject the change completely or partially update all the correctly defined nodes? You'e done the former. Seems fine. May be say the same in the exception message? That we are rejecting all requests?
I tried to keep it simple as getting rid of partial update. Will update the exception message.
bq. Are you not doing the CLI support for the update resources in this patch? I think we should. Here or separate patch.
Yes. This is major work for YARN-313. Make sense?
bq. Again, didn't review previous patch. So we need to fix here or elsewhere: RMNode is supposed to be a read-only interface, so setsetResourceOption() doesn't belong there. It should be an event to the node informing the change in resource.
That's a good point! Can we fix it in a separated JIRA given this patch is big enough and we may want it to be dedicated for RPC things?


> Add updateNodeResource in ResourceManagerAdministrationProtocol
> ---------------------------------------------------------------
>
>                 Key: YARN-312
>                 URL: https://issues.apache.org/jira/browse/YARN-312
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: api
>    Affects Versions: 2.2.0
>            Reporter: Junping Du
>            Assignee: Junping Du
>         Attachments: YARN-312-v1.patch, YARN-312-v2.patch, YARN-312-v3.patch, YARN-312-v4.1.patch, YARN-312-v4.patch, YARN-312-v5.1.patch, YARN-312-v5.patch, YARN-312-v6.patch, YARN-312-v7.1.patch, YARN-312-v7.1.patch, YARN-312-v7.patch, YARN-312-v8.patch
>
>
> Add fundamental RPC (ResourceManagerAdministrationProtocol) to support node's resource change. For design detail, please refer parent JIRA: YARN-291.


--
This message was sent by Atlassian JIRA
(v6.1.4#6159)