[
https://issues.apache.org/jira/browse/YARN-103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13467208#comment-13467208
]
Siddharth Seth commented on YARN-103:
-------------------------------------
This is looking good with the synchronization fixes and error handling. The
MR-3902 branch has some similar changes in RMContainerRequester. I'll try
plugging this in there.
A few comments.
- {{makeContainerRequest}} was called {{addContainerRequest}} in
RMContainerRequestor. Why rename it. To me, addContainerRequest makes more
sense since the AMRMClient maintains an aggregate of requests.
- The javadoc for removeContainerRequest is a little misleading. "This will not
remove previous requests after allocate has been called for those requests".
removeContainerRequests just decrements the specific request from the main
request table - and sends out whatever has changed in the next allocate call.
- getClusterAvailableResources / getClusterNodeCount - Instead of documenting
that these should be called after an 'allocate' call - should this be enforced,
maybe via an exception ? (Independent of this jira, does it make sense to
include additional information in the Register call - i.e. numClusterNodes,
avaialbleResources / other parts of the allocate payload)
- Similarly, instead of documenting that allocate cannot be called concurrently
- this can be enforced.
- In the unit test, would prefer avoiding MiniMRCluster where possible. This
one should be possible using the already existing {{MockRM}} or by mocking the
RM. (branch MR-3902:TestRMContainerRequestor already does this, and tests a
failure scenario for this patch)
Minor Stuff
- Avoid RecordFactory usage. Replaced by Records.newRecord() or BuilderUtils.*
- The modification to ask in case of a failure could be a simple {{ask.addAll}}
instead of the exists check.
> Add a yarn AM - RM client module
> --------------------------------
>
> Key: YARN-103
> URL: https://issues.apache.org/jira/browse/YARN-103
> Project: Hadoop YARN
> Issue Type: Improvement
> Reporter: Bikas Saha
> Assignee: Bikas Saha
> Attachments: YARN-103.1.patch, YARN-103.2.patch, YARN-103.3.patch
>
>
> Add a basic client wrapper library to the AM RM protocol in order to prevent
> proliferation of code being duplicated everywhere. Provide helper functions
> to perform reverse mapping of container requests to RM allocation resource
> request table format.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
