hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Karthik Kambatla (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-1028) Add FailoverProxyProvider like capability to RMProxy
Date Mon, 16 Dec 2013 22:43:08 GMT

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

Karthik Kambatla commented on YARN-1028:

Uploaded a new patch that addresses most of [~vinodkv]'s and [~bikassaha]'s comments.

bq. RMFailoverProxyProvider doesn't need to be public? For java users, the clients are the
public interfaces, so they don't need to use these directly?
Left it public. If users want to implement their own RMFailoverProxyProvider, they should
be able to, and this should be public.

bq. Declaration of INSTANCE in ServerRMProxy and ClientRMProxy is missing types.
bq. We don't expect users to create ClientRMProxy and ServerRMProxy. Create and make constructors
bq. Now that you've created an INSTANCE, you can move createRMProxy() up into RMProxy itself
and reuse code.
bq. Why do we need this singleton like abstraction now?
Created private constructors for ClientRMProxy and ServerRMProxy, and moved the crux of createRMProxy
to RMProxy. Along with this, moved INSTANCE to RMProxy. {{createRMProxy}} is a static method,
requiring INSTANCE to be static (singleton-like). However, INSTANCE is defined in a static
block of subclasses - ClientRMProxy and ServerRMProxy - so, left it untyped.

bq. Shouldn't RMProxy.createFailoverProxyProvider actually be in RMFailoverProxyProvider itself?
Left it in RMFailoverProxyProvider as it is an interface. Can change it to abstract class
and move it there if preferred.

bq. Is it possible to consolidate the configuration properties for both HA and non-HA case?
If not, we'll have to fix documentation in yarn-default for non-ha configs like connect.max-wait.ms
that they are not used in HA mode.
bq. Handle HA related code up-front? Don't need to read other configs if HA is enabled.
Changed configuration. yarn.client.failover.* configs override other configs - described in
yarn-default.xml. However, when failover configs are not specified, they are inferred from
the non-HA case. For instance, wait for ever fails over for ever etc.

bq. instead of a config YARN_MINICLUSTER_USE_RPC, why not indicate as part of constructor?
Left it as a config. Two reasons: (1) Constructor with a bunch of ints and bools might not
be very user-friendly, and can lead to an explosion in the number of constructors. (2) It
is confusing to have some properties in the constructor and some in the config.

bq. This code seems to be duplicating stuff RMProxy/ClientRMProxy. Can we call the existing
methods in RMProxy instead?
Good catch. Fixed.

bq. Is this logic not valid in an HA scenario?
As discussed in this JIRA earlier, exceptionToPolicyMap logic can be extended to apply here,
but we will have to add a bunch of connection-related exceptions here. Adding these doesn't
give us much because FailoverOnNetworkException already handles this logic for us.

bq. Will mini yarn HA cluster work without fixed ports? Can the test cover that case also
if simple to extend.
TestMiniYARNClusterForHA tests this.

bq. Dont see the initial value of this being set to -1 anywhere.
MiniYARNCluster.getActiveRMIndex() returns -1 if it can't find an active ResourceManager.
Added this information to the javadoc comments for the method.

bq. New class does not seem to add anything?
It appears so in the patch, because I am splitting an existing class CustomNodeManager after
a method. Applying the patch will show the contents. :)

bq. ConfiguredFailoverProxyProvider. the common one is also doing proxy-caching, closing clients
on provider.close()
bq. Similarly some type information missing from RMProxy.createFailoverProxyProvider.
bq. Annotations missing for ConfiguredRMFailoverProxyProvider
bq. Nice test! It would help that after failing over we can verify that the new active is
really the one we expect to be active.

> Add FailoverProxyProvider like capability to RMProxy
> ----------------------------------------------------
>                 Key: YARN-1028
>                 URL: https://issues.apache.org/jira/browse/YARN-1028
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Bikas Saha
>            Assignee: Karthik Kambatla
>         Attachments: yarn-1028-1.patch, yarn-1028-2.patch, yarn-1028-3.patch, yarn-1028-4.patch,
yarn-1028-5.patch, yarn-1028-6.patch, yarn-1028-7.patch, yarn-1028-8.patch, yarn-1028-9.patch,
> RMProxy layer currently abstracts RM discovery and implements it by looking up service
information from configuration. Motivated by HDFS and using existing classes from Common,
we can add failover proxy providers that may provide RM discovery in extensible ways.

This message was sent by Atlassian JIRA

View raw message