bharathv commented on a change in pull request #2769:
URL: https://github.com/apache/hbase/pull/2769#discussion_r599293171
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
##########
@@ -176,7 +176,7 @@ private void testSanity(final String testName) throws
Exception {
@Test
public void testRegionAssignmentAfterMasterRecoveryDueToZKExpiry() throws
Exception {
MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
- cluster.startRegionServer();
+ cluster.startRegionServerAndWait(2000);
Review comment:
@saintstack Its the other way around. After this patch, "online" atomic
is not set to true at the right place. It is being set in
handleReportForDutyResponse() which doesn't mean the RS is online. It should
either be set after the first RS report or _ waitForServerOnline_ should check
the active master's online list of masters.
I was suggesting the latter to avoid extra code refactoring. So yes, we
should pass the active master object to it _or_ we could just switch to using
HBaseCluster#waitForRegionServerToStart() that relies on the onlineServers from
active master. Latter approach might be easier to refactor IIUC.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
