[
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)