Mailing-List: contact common-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-issues@hadoop.apache.org
Date: Mon, 9 Apr 2012 22:09:21 +0000 (UTC)
From: "Aaron T. Myers (Commented) (JIRA)" <jira@apache.org>
To: common-issues@hadoop.apache.org
Message-ID: 
 <1720009702.4969.1334009361117.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <2024739814.15210.1333587322121.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (HADOOP-8247) Auto-HA: add a config to enable
 auto-HA, which disables manual FC
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Aaron T. Myers commented on HADOOP-8247:
----------------------------------------

Patch looks great to me, Todd. Just a few little nits:

# Add a comment to HAAdmin#addFailoverOpts explaining why FORCEMANUAL isn't being added there.
# Nit: "String []argv = cmd.getArgs();" - put a space between "[]" and "argv"
# Not quite sure of the purpose of these changes:
{code}
-  public void monitorHealth() throws HealthCheckFailedException,
+  public void monitorHealth()
+                              throws HealthCheckFailedException,
                                      AccessControlException,
                                      IOException;
{code}
and:
{code}
-  public HAServiceStatus getServiceStatus() throws AccessControlException,
+  public HAServiceStatus getServiceStatus()
+                                            throws AccessControlException,
                                                    IOException;
{code}
# There's an errant whitespace change in TestZKFailoverControllerStress.
# Not quite sure what the point of this change is:
{code}
-          return nn.getRpcServer().getServiceStatus().getState() == state;
+          HAServiceStatus curStatus = nn.getRpcServer().getServiceStatus();
+          return curStatus.getState() == state;
{code}
# There's a few spots in this patch where you just do some unrelated cleanup of calls to transitionToActive() in the tests. Totally fine, but you might want to mention this in a JIRA comment in the future if you're not going to break them out into a separate patch.
# Some goofy javadoc comment closing:
{code}
+  /**
+   * Test that, even if automatic HA is enabled, the monitoring operations
+   * still function correctly.
+   *    */
{code}

Side note: Really good javadoc cleanup on NAMENODE_SPECIFIC_KEYS. Good on you.
                
> Auto-HA: add a config to enable auto-HA, which disables manual FC
> -----------------------------------------------------------------
>
>                 Key: HADOOP-8247
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8247
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: auto-failover, ha
>    Affects Versions: Auto Failover (HDFS-3042)
>            Reporter: Todd Lipcon
>            Assignee: Todd Lipcon
>         Attachments: hadoop-8247.txt, hadoop-8247.txt
>
>
> Currently, if automatic failover is set up and running, and the user uses the "haadmin -failover" command, he or she can end up putting the system in an inconsistent state, where the state in ZK disagrees with the actual state of the world. To fix this, we should add a config flag which is used to enable auto-HA. When this flag is set, we should disallow use of the haadmin command to initiate failovers. We should refuse to run ZKFCs when the flag is not set. Of course, this flag should be scoped by nameservice.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira