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

Bikas Saha commented on YARN-1029:
----------------------------------

I see that the patch has increased the node manager connect time in the test 
from 5s to 11s. Its not clear to me how the test earlier worked or works now.

This method used to be synchronized
{code}-  private synchronized boolean isRMActive() {{code}

Should we clear or fail to start? The data seems to be in error.
{code}+    if (elector.parentZNodeExists() && !isParentZnodeSafe(clusterId)) {
+      elector.clearParentZNode();
+    }{code}

Is it necessary to mention ZKFC here?
{code}+  private static final HAServiceProtocol.StateChangeRequestInfo req =
+      new HAServiceProtocol.StateChangeRequestInfo(
+          HAServiceProtocol.RequestSource.REQUEST_BY_ZKFC);{code}

Can we share the terminate functianality with 
RMStateStoreOperationFailedEventDispatcher in a common function?
{code}+  public static class RMEmbeddedElectorEventDispatcher implements
+      EventHandler<RMEmbeddedElectorEvent> {
+    @Override
+    public void handle(RMEmbeddedElectorEvent event) {
+      LOG.fatal("Shutting down on receiving a " +
+          RMEmbeddedElectorEvent.class.getName() + " of type " +
+          event.getType().name());
+      ExitUtil.terminate(1, event.getCause());
+    }{code}

We probably need some unified method of notifying the RM about something bad. 
One example being embedded leader election reporting an error. Else we may end 
up with a proliferation of event handlers.

The patch overall looks good to me. Would like some reviews from other 
committers, specially around the MiniYARNCluster changes. I am not that 
familiar with the mini cluster code.

> Allow embedding leader election into the RM
> -------------------------------------------
>
>                 Key: YARN-1029
>                 URL: https://issues.apache.org/jira/browse/YARN-1029
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Bikas Saha
>            Assignee: Karthik Kambatla
>         Attachments: embedded-zkfc-approach.patch, yarn-1029-0.patch, 
> yarn-1029-0.patch, yarn-1029-1.patch, yarn-1029-2.patch, yarn-1029-3.patch, 
> yarn-1029-approach.patch
>
>
> It should be possible to embed common ActiveStandyElector into the RM such 
> that ZooKeeper based leader election and notification is in-built. In 
> conjunction with a ZK state store, this configuration will be a simple 
> deployment option.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to