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-5412) Create a proxy chain for ResourceManager REST API in the Router
Date Mon, 10 Jul 2017 23:37:01 GMT

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

Carlo Curino commented on YARN-5412:

Giovanni, thanks for the contribution. Please find below a partial review (patch is large
going through it slowly, and posting comments 
so you can parallelize addressing them).

# {{DefaultRequestInterceptorREST.init}} does not propagate correctly to downstream interceptors...
you might want to have a "localInit" method that is implemented to locally initialize the
components, while the {{AbstractRESTRequestInterceptor}} takes care of propagating to downstream,
by invoking the outward {{init}}

# In the next version of patch, please remove the changes you already pushed in YARN-6792
# {{DefaultRequestInterceptorREST}} there is lots of repeting code doing the "check status"
stuff, which could be solved with a single method that receives in input a string-constant
and the .class of the expected outcome.
# {{RESTRequestInterceptor}} there are a few (uncommented) methods, which I am not sure belong
here. Shall we move them to RMWebServiceProtocol? Help me understand why e.g., getContainer
should be here.

# {{yarn-default.xml}}  the comment is not very clear about how the list of classes make up
the pipeline.
# {{AbstractRESTRequestInterceptor}} javadoc errors "can can"
# {{(RMWSConsts.STATES}} is a confusing name, something like CONTAINER_STATES? 
# {{RESTRequestInterceptor}} the javadoc of {{setNextInterceptor}} is confusing. It says the
last interceptor does not receive this call, while it will likely receive it with a null value,

# {{RMWebAppUtil.isStaticUser}} is invoked a lot in {{RMWebServices}}, and every time it performs
a read from conf. Can we cache the value of staticUser, so this method gets much lighter to
# {{DefaultRequestInterceptorREST}} Can we try to use generics to remove lots of the almost-code-repetition
we currently have?

# can we scope-down to tests the import of nodemanager project?
# why does {{RESTRequestInterceptor.init}} has a "user" param? Javadoc is incomplete, and
it is not used by {{DefaultRequestInterceptorREST}} so hard to understand.

[...] more to come (I will re-post the entire list adding to it, so numbering remains consistent...
we can -strike-out- elements as you do them

> Create a proxy chain for ResourceManager REST API in the Router
> ---------------------------------------------------------------
>                 Key: YARN-5412
>                 URL: https://issues.apache.org/jira/browse/YARN-5412
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager, resourcemanager
>            Reporter: Subru Krishnan
>            Assignee: Giovanni Matteo Fumarola
>         Attachments: YARN-5412-YARN-2915.1.patch, YARN-5412-YARN-2915.proto.patch
> As detailed in the proposal in the umbrella JIRA, we are introducing a new component
that routes client request to appropriate ResourceManager(s). This JIRA tracks the creation
of a proxy for ResourceManager REST API in the Router. This provides a placeholder for:
> 1) throttling mis-behaving clients (YARN-1546)
> 3) mask the access to multiple RMs (YARN-3659)
> We are planning to follow the interceptor pattern like we did in YARN-2884 to generalize
the approach and have only dynamically coupling for Federation.

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