hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carlo Curino (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-7010) Federation: routing REST invocations transparently to multiple RMs (part 2 - getApps)
Date Mon, 28 Aug 2017 20:59:00 GMT

    [ 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: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message