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

Bikas Saha commented on YARN-1028:
----------------------------------

Why do we need this singleton like abstraction now?
{code}
+  private static ClientRMProxy INSTANCE = new ClientRMProxy();
{code}

I dont think we should make YarnClientImpl HA aware. The main point of RMProxy 
was to hide this from upper layers like the client code. For the purposes of 
the log, we could add an API to the RMProxy interface that gets the rm address 
relevant to the protocol being proxied.
{code}
-  private static InetSocketAddress getRmAddress(Configuration conf) {
{code}

This code seems to be duplicating stuff RMProxy/ClientRMProxy. Can we call the 
existing methods in RMProxy instead?
{code}
+  public T getProxy() {
.....
+            return (T) YarnRPC.create(conf).getProxy(protocol, rmAddress, 
conf);
{code}

Is this logic not valid in an HA scenario?
{code}
+      Map<Class<? extends Exception>, RetryPolicy> exceptionToPolicyMap =
+          new HashMap<Class<? extends Exception>, RetryPolicy>();
+      exceptionToPolicyMap.put(ConnectException.class, retryPolicy);
+      //TO DO: after HADOOP-9576,  IOException can be changed to EOFException
+      exceptionToPolicyMap.put(IOException.class, retryPolicy);
+      return RetryPolicies.retryByException(
+          RetryPolicies.TRY_ONCE_THEN_FAIL, exceptionToPolicyMap);
{code}

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.

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

Dont see the initial value of this being set to -1 anywhere. 
{code}
+    assertFalse("RM never turned active", -1 == cluster.getActiveRMIndex());
{code}

New class does not seem to add anything?
{code}
+  private class ShortCircuitedNodeManager extends CustomNodeManager {
{code}

Can we please leave a short note on how the mini yarn cluster behaves with 
multiple RM's?

> 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-draft-cumulative.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
(v6.1.4#6159)

Reply via email to