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-6896) Federation: routing REST invocations transparently to multiple RMs
Date Wed, 09 Aug 2017 00:57:02 GMT

    [ https://issues.apache.org/jira/browse/YARN-6896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16119263#comment-16119263

Carlo Curino commented on YARN-6896:

Thaks [~giovanni.fumarola] for the contribution. Yetus seems mostly happy, please fix the
checkstyles that can be fixed easily (one line length and the unused imports). Below few comments
on the implementation:

# {{FederationInterceptorREST}} would we ever care to set the REST retries differenly from
ROUTER_CLIENTRM_SUBMIT_RETRY ? If not reusing the conf is good, otherwise is limiting.
# {{FederationInterceptorREST.getInterceptorForSubCluster}} has slightly weird get-or-set
semantics. Can we break it down to a pure getter and pure setter, or at least rename it?
# {{FederationInterceptorREST}} line 176, you are editing the collection returned by the federationFacade.
Is this kosher? I.e., do you know that is a local copy of it? Why not using the same "blacklisting"
style you have in other methods?
# {{FederationInterceptorREST}} line 205, in the comment should we inline the behavior (beside
stating is the same-as)? if the text is not too long it might be worth it.
# {{FederationInterceptorREST.submitApplication}} I would expect if we run-out of tries for
us to remove the attempted submission from stateStore, while you never perform a removeApplicationHomeSubcluster,
so the last attempted submission would be still stored in the statestore upon running out
of retries. Is that true?
# {{FederationInterceptorREST.getApp/updateAppState/}} does this work fine in case the String
appId in input is malformed? For submitApplication you make an explicit check. 
# What is the criteria for picking the subset of methods you are implementing? Also many of
the non-implemented methods seem trivial forward-to-homesubcluster or broadcast type behaviors.

# {{TestFederationInterceptorREST }} constructing a random list of sub-clusters seems a very
common problem in Tests... can you check if we can reuse this code from other classes (and/or
move it to a util test class).
# {{TestFederationInterceptorREST.createConfiguration}} do you need 2 mocks?
# {{TestableFederationInterceptorREST.getInterceptorForSubCluster}} this completely mask the
underlying function, which is rather important to maintain the DefaultRequestInterceptorREST
map. It would be better to find another workaround that exercise the original code, and not
mock it away.

> Federation: routing REST invocations transparently to multiple RMs
> ------------------------------------------------------------------
>                 Key: YARN-6896
>                 URL: https://issues.apache.org/jira/browse/YARN-6896
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Giovanni Matteo Fumarola
>            Assignee: Giovanni Matteo Fumarola
>         Attachments: YARN-6896.proto.patch, YARN-6896.v1.patch
> This JIRA tracks the design/implementation of the layer for routing RMWebServicesProtocol
requests to the appropriate RM(s) in a federated YARN cluster.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org

View raw message