[
https://issues.apache.org/jira/browse/YARN-5531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15983669#comment-15983669
]
Subru Krishnan commented on YARN-5531:
--------------------------------------
Thanks [~botong] for the patch. I went through it, please find my feedback
below.
{{UnmanagedAMPoolManager}}:
* We should Initialize *threadpool* in default constructor or in
_serviceStart_ (if null).
* The order of operations in *finishApplicationMaster* should be reversed as
now client retries are not possible?
{{UnmanagedApplicationMaster}}:
* Nit: rename {{UnmanagedApplicationMaster}} -->
{{UnmanagedApplicationManager}}.
* IIUC, *requestQueuePendingCount* is redundant, we should use
*requestQueue.size*.
* *registerUAMCalled* is also redundant as we can get the same information
from *lastResponseId* or *registerRequest*.
* A code comment explaining when *userUgi* will be null/reused will be useful.
* Shouldn't *forceKillApplication* check for Am registered state?
* The *createRMProxy* is very similar to the one in
{{FederationProxyProviderUtil}}, refactor it out to {{YarnServerSecurityUtils}}
and reuse.
* Same comment for *updateAMRMToken*
{{CallbackHandlerThread}}
* There are quite a few throw _RuntimeException_ which will exit the thread.
Do we have any better options or is this fine?
{{UnmanagedAMLauncher}}
* Can we reuse code in *submitAppAndGetAppId*,
*monitorApplicationSubmission*, *monitorCurrentAppAttempt* &
*getApplicationReport* with the existing UAM launcher
[here|https://github.com/apache/hadoop/blob/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-unmanaged-am-launcher/src/main/java/org/apache/hadoop/yarn/applications/unmanagedamlauncher/UnmanagedAMLauncher.java]?
* Why do we need the _attemptId_ in both the constructor
*initializeUnmanagedAM* as they seem to invoked subsequently? Addressing this
should also cleanup the TODO in *initializeUnmanagedAM*.
* The initialization of _recordFactory_ is redundant in
*submitAppAndGetAppId*.
Tests
* {{MockResourceManagerFacade}} already exists
[here|https://github.com/apache/hadoop/blob/YARN-2915/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/MockResourceManagerFacade.java],
so we should reuse instead of adding it again.
* There are no tests for _forceKillApplication_ and re-registering.
> UnmanagedAM pool manager for federating application across clusters
> -------------------------------------------------------------------
>
> Key: YARN-5531
> URL: https://issues.apache.org/jira/browse/YARN-5531
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager, resourcemanager
> Reporter: Subru Krishnan
> Assignee: Botong Huang
> Attachments: YARN-5531-YARN-2915.v1.patch,
> YARN-5531-YARN-2915.v2.patch, YARN-5531-YARN-2915.v3.patch,
> YARN-5531-YARN-2915.v4.patch
>
>
> One of the main tenets the YARN Federation is to *transparently* scale
> applications across multiple clusters. This is achieved by running UAMs on
> behalf of the application on other clusters. This JIRA tracks the addition of
> a UnmanagedAM pool manager for federating application across clusters which
> will be used the FederationInterceptor (YARN-3666) which is part of the
> AMRMProxy pipeline introduced in YARN-2884.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
