[
https://issues.apache.org/jira/browse/HBASE-5209?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13207808#comment-13207808
]
[email protected] commented on HBASE-5209:
------------------------------------------------------
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > This version looks cleaner.
Thanks, just trying to get the hang of how things work. I now know the process
for future patches.
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 214
bq. > <https://reviews.apache.org/r/3892/diff/1/?file=74836#file74836line214>
bq. >
bq. > GetMaster() should be good enough.
I was trying to follow the existing naming conventions in the file, and I had
to choose between getDeadServerNames() and getServers(). I'll change this to
getMaster() as you suggest.
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 221
bq. > <https://reviews.apache.org/r/3892/diff/1/?file=74836#file74836line221>
bq. >
bq. > How about naming this method getNumberOfBackupMasters() ?
I was trying to follow the existing naming convention in the file, and I was
basing this off of getServersSize(). There is no precedent for "NumberOf" in
this file, and I do not want to introduce yet another naming convention into
this file. Please advise.
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 228
bq. > <https://reviews.apache.org/r/3892/diff/1/?file=74836#file74836line228>
bq. >
bq. > getBackupMasters() should be good enough.
Sounds good.
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 319
bq. > <https://reviews.apache.org/r/3892/diff/1/?file=74836#file74836line319>
bq. >
bq. > Please wrap long line.
bq. > Lines are 80 chars or shorter.
Will do.
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java,
line 94
bq. > <https://reviews.apache.org/r/3892/diff/1/?file=74841#file74841line94>
bq. >
bq. > How about naming this member backupMasterAddressesZNode ?
OK.
bq. On 2012-02-14 15:21:15, Ted Yu wrote:
bq. > src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java,
line 211
bq. > <https://reviews.apache.org/r/3892/diff/1/?file=74841#file74841line211>
bq. >
bq. > Would "zookeeper.znode.backup.masters" be better config name ?
bq. > Similarly, "backup-masters" for default znode name.
OK. I'll upload a new diff pending your advice on the "NumberOf" issue.
Thanks!
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3892/#review5083
-----------------------------------------------------------
On 2012-02-14 14:41:55, David Wang wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3892/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-02-14 14:41:55)
bq.
bq.
bq. Review request for hbase.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Problem:
bq. There is no method in the HBase client-facing APIs to determine which of
the masters is currently active. This can be especially useful in setups with
multiple backup masters.
bq.
bq. Solution:
bq. Augment ClusterStatus to return the currently active master and the list
of backup masters.
bq.
bq. Notes:
bq. * I uncovered a race condition in ActiveMasterManager, between when it
determines that it did not win the original race to be the active master, and
when it reads the ServerName of the active master. If the active master goes
down in that time, the read to determine the active master's ServerName will
fail ungracefully and the candidate master will abort. The solution
incorporated in this patch is to check to see if the read of the ServerName
succeeded before trying to use it.
bq. * I fixed some minor formatting issues while going through the code. I
can take these changes out if it is considered improper to commit such
non-related changes with the main changes.
bq.
bq.
bq. This addresses bug HBASE-5209.
bq. https://issues.apache.org/jira/browse/HBASE-5209
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/java/org/apache/hadoop/hbase/ClusterStatus.java b849429
bq. src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java
2f60b23
bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java 9d21903
bq. src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java f6f3f71
bq. src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java 111f76e
bq. src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
3e3d131
bq.
src/test/java/org/apache/hadoop/hbase/master/TestActiveMasterManager.java
16e4744
bq. src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java
bc98fb0
bq.
bq. Diff: https://reviews.apache.org/r/3892/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * Ran mvn -P localTests test multiple times - no new tests fail
bq. * Ran mvn -P localTests -Dtest=TestActiveMasterManager test multiple runs
- no failures
bq. * Ran mvn -P localTests -Dtest=TestMasterFailover test multiple runs - no
failures
bq. * Started active and multiple backup masters, then killed active master,
then brought it back up (will now be a backup master)
bq. * Did the following before and after killing
bq. * hbase fsck - checked output to see that active and backup masters
are reported properly
bq. * zk_dump - checked that active and backup masters are reported
properly
bq. * Started cluster with no backup masters to make sure change operates
correctly that way
bq. * Ran dev-support/test-patch.sh - no new issues fail:
bq.
bq. -1 overall.
bq.
bq. +1 @author. The patch does not contain any @author tags.
bq.
bq. +1 tests included. The patch appears to include 7 new or modified
tests.
bq.
bq. -1 javadoc. The javadoc tool appears to have generated -136 warning
messages.
bq.
bq. +1 javac. The applied patch does not increase the total number of
javac compiler warnings.
bq.
bq. +1 findbugs. The patch does not introduce any new Findbugs (version )
warnings.
bq.
bq. +1 release audit. The applied patch does not increase the total
number of release audit warnings.
bq.
bq.
bq. Thanks,
bq.
bq. David
bq.
bq.
> HConnection/HMasterInterface should allow for way to get hostname of
> currently active master in multi-master HBase setup
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-5209
> URL: https://issues.apache.org/jira/browse/HBASE-5209
> Project: HBase
> Issue Type: Improvement
> Components: master
> Affects Versions: 0.94.0, 0.90.5, 0.92.0
> Reporter: Aditya Acharya
> Assignee: David S. Wang
> Fix For: 0.94.0, 0.90.7, 0.92.1
>
> Attachments: HBASE-5209-v0.diff, HBASE-5209-v1.diff
>
>
> I have a multi-master HBase set up, and I'm trying to programmatically
> determine which of the masters is currently active. But the API does not
> allow me to do this. There is a getMaster() method in the HConnection class,
> but it returns an HMasterInterface, whose methods do not allow me to find out
> which master won the last race. The API should have a
> getActiveMasterHostname() or something to that effect.
--
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
