[
https://issues.apache.org/jira/browse/YARN-5325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15564110#comment-15564110
]
Subru Krishnan commented on YARN-5325:
--------------------------------------
Thanks [~curino] for addressing my comments. The patch looks close, have a few
minor comments:
* The {{AllocationBookkeeper}} is still being reinitialized (which is where
the heavy lifting is) for every *splitResourceRequests* invocation. Instead we
should do it as part of reinitialize of {{LocalityMulticastAMRMProxyPolicy}} as
we already short-circuit based on dirty flag there. Additionally, we only need
to update _total headroom_ as part of *notifyOfResponse*.
* Nit: we should log exceptions (at debug level?) from the
{{SubClusterResolver}}.
* We should do null check for *targetId*, could be done in
_isActiveAndEnabled_.
* We should use continues instead of multi-level nested if-else in
_splitResourceRequests_ for readability.
* Can you please add code comments for expected _numContainers_ in
{{TestLocalityMulticastAMRMProxyFederationPolicy}}.
bq. The system.outs in tests are useful for debugging the tests. I would leave
them there if you are ok with it.
* In that case, can you replace the syso with log statements.
bq. Regarding FederationPoliciesTestUtil::createResourceRequest there are few
objects to create, and this would make the code in tests quite redundant I
think.
* There seems to be only _Priority_ and _Resource_ both of which are easily
inline-able but this is only a nit.
> Stateless ARMRMProxy policies implementation
> --------------------------------------------
>
> Key: YARN-5325
> URL: https://issues.apache.org/jira/browse/YARN-5325
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager, resourcemanager
> Affects Versions: YARN-2915
> Reporter: Carlo Curino
> Assignee: Carlo Curino
> Attachments: YARN-5325-YARN-2915.05.patch,
> YARN-5325-YARN-2915.06.patch, YARN-5325-YARN-2915.07.patch,
> YARN-5325-YARN-2915.08.patch, YARN-5325-YARN-2915.09.patch,
> YARN-5325-YARN-2915.10.patch, YARN-5325.01.patch, YARN-5325.02.patch,
> YARN-5325.03.patch, YARN-5325.04.patch
>
>
> This JIRA tracks policies in the AMRMProxy that decide how to forward
> ResourceRequests, without maintaining substantial state across decissions
> (e.g., broadcast).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
