[
https://issues.apache.org/jira/browse/YARN-7010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16144338#comment-16144338
]
Carlo Curino commented on YARN-7010:
------------------------------------
Thanks [~giovanni.fumarola] for the updated version, patch generally looks
good, but I have few more comments:
# Please leave the APP_NAME to its current value, as this will also be used in
non-federated settings.
# double check the "@VisibleForTesting" tags as I think some might be misplaced
# Is {{routerMetrics.succeededMultipleAppsRetrieved(stopTime - startTime);}}
supposed to be in the retrieval loop? This will measure separately for each
retrieval what happens. Is this what you want? Or measure latency once for the
overall {{getApps}} function call?
# {{mergeAppsInfo}} I think you could combined all the loops if you use one or
two support HashMap<Appid,AppInfo>... you can merge everything you find on your
way (single scan O(n)), while you are searching for the AM. When you find the
AM you can have a deeper merge (or a swap) so that all the non-portioned
information are reflected correctly in the return. If at the end you didn't
find an AM you can filter out (based on partial or not). This would likely make
this faster (you do basically 3 full scans of the results, and you can bring it
down to 1 or 2) and a bit easier to read.
# {{MockDefaultRequestInterceptorREST}} where you do
{{appInfo.setAMHostHttpAddress("I am the AM");}} use a fake but correctly
structured http address, in case someone later on adds a validation for it, so
they don't have to fix your code here.
# Same as above for {{TestRouterWebServiceUtil}}
# {{testMerge4DifferentApps}} should check that the parameters (at least a few)
are preserved
# Consider to refactor {{testMergeAppsFinished}} and {{testMergeAppsRunning}},
you have lots of repeated code which could be factored out.
> Federation: routing REST invocations transparently to multiple RMs (part 2 -
> getApps)
> -------------------------------------------------------------------------------------
>
> Key: YARN-7010
> URL: https://issues.apache.org/jira/browse/YARN-7010
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Giovanni Matteo Fumarola
> Assignee: Giovanni Matteo Fumarola
> Attachments: YARN-7010.v0.patch, YARN-7010.v1.patch,
> YARN-7010.v2.patch, YARN-7010.v3.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
