[
https://issues.apache.org/jira/browse/YARN-5412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16102396#comment-16102396
]
Carlo Curino commented on YARN-5412:
------------------------------------
Thanks [~giovanni.fumarola], I feel we are getting very close. Few more
comments (beside addressing the Yetus leftover complaints).
# In {{DefaultRequestInterceptorREST}} you have some comments about params that
are not explicitly forwarded as they are QueryParam. If possible try to be
consistent and mention all of them (or none and make one high-level comment at
the top of the class).
# Do you need a {{ConcurrentHashMap}} for additionalParam? Can't you create it
and make it non-modifiable ({{Collections.unmodifiableMap(additionalParam)}} if
you have to). {{ConcurrentHashMap}} seems overkill.
# I don't like the replicated inheritance for the
{{getContainer/getContainers/getAppAttemp/getAppAttemps}}. We should move those
from {{RESTRequestInterceptor}} to be part of an interface that {{WebService}}
implements, and have {{RMWebServiceProtocol}} or {{RESTRequestInterceptor}}
extend that. (Ok to postpone in separate refactoring JIRA, please open it)
# in {{RouterWebServiceUtil.genericForward (103)}} do you ever have both
{{hsr}} AND {{additionalParam}}? If so, wouldn't you add them to the
{{paramMap}}? (if not leave at least a comment that this should never happen)
# in {{RouterWebServiceUtil.invokeRMWebService (158)}} I see you are
hard-coding APPLICATION_XML, but shouldn't we forward whatever output type is
expected from the original user call? If this works already correctly, please
write a test to validate it.
# I see in the tests you are doing a bunch of spot-checks that the Router and
RM produce the same response. Equality would not work, but what about
systematically convert the response to .json and compare the .json strings?
Wouldn't they match? If so, you can have a tighter test with much less code,
but moving the lots of the assert/checks into the performCall function, or
having a "validate(r1,r2)" generic method.
> 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.2.patch, YARN-5412-YARN-2915.3.patch,
> YARN-5412-YARN-2915.4.patch, YARN-5412-YARN-2915.5.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
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
