Index: src/main/java/org/apache/hadoop/hbase/master/HMaster.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/HMaster.java (revision 1345218) +++ src/main/java/org/apache/hadoop/hbase/master/HMaster.java (working copy) @@ -341,7 +341,7 @@ // We are either the active master or we were asked to shutdown if (!this.stopped) { - finishInitialization(startupStatus); + finishInitialization(startupStatus, false); loop(); } } catch (Throwable t) { @@ -459,12 +459,13 @@ *
  • Ensure assignment of root and meta regions
  • *
  • Handle either fresh cluster start or master failover
  • * + * @param masterRecovery * * @throws IOException * @throws InterruptedException * @throws KeeperException */ - private void finishInitialization(MonitoredTask status) + private void finishInitialization(MonitoredTask status, boolean masterRecovery) throws IOException, InterruptedException, KeeperException { isActiveMaster = true; @@ -478,7 +479,7 @@ status.setStatus("Initializing Master file system"); this.masterActiveTime = System.currentTimeMillis(); // TODO: Do this using Dependency Injection, using PicoContainer, Guice or Spring. - this.fileSystemManager = new MasterFileSystem(this, this, metrics); + this.fileSystemManager = new MasterFileSystem(this, this, metrics, masterRecovery); this.tableDescriptors = new FSTableDescriptors(this.fileSystemManager.getFileSystem(), @@ -487,22 +488,25 @@ // publish cluster ID status.setStatus("Publishing Cluster ID in ZooKeeper"); ClusterId.setClusterId(this.zooKeeper, fileSystemManager.getClusterId()); + if (!masterRecovery) { + this.executorService = new ExecutorService(getServerName().toString()); + } - this.executorService = new ExecutorService(getServerName().toString()); - this.serverManager = new ServerManager(this, this); status.setStatus("Initializing ZK system trackers"); initializeZKBasedSystemTrackers(); + + if (!masterRecovery) { + // initialize master side coprocessors before we start handling requests + status.setStatus("Initializing master coprocessors"); + this.cpHost = new MasterCoprocessorHost(this, this.conf); - // initialize master side coprocessors before we start handling requests - status.setStatus("Initializing master coprocessors"); - this.cpHost = new MasterCoprocessorHost(this, this.conf); + // start up all service threads. + status.setStatus("Initializing master service threads"); + startServiceThreads(); + } - // start up all service threads. - status.setStatus("Initializing master service threads"); - startServiceThreads(); - // Wait for region servers to report in. this.serverManager.waitForRegionServers(status); // Check zk for regionservers that are up but didn't register @@ -514,8 +518,9 @@ this.serverManager.recordNewServer(sn, HServerLoad.EMPTY_HSERVERLOAD); } } - - this.assignmentManager.startTimeOutMonitor(); + if (!masterRecovery) { + this.assignmentManager.startTimeOutMonitor(); + } // TODO: Should do this in background rather than block master startup status.setStatus("Splitting logs after master startup"); splitLogAfterStartup(this.fileSystemManager); @@ -543,13 +548,15 @@ status.setStatus("Fixing up missing daughters"); fixupDaughters(status); - // Start balancer and meta catalog janitor after meta and regions have - // been assigned. - status.setStatus("Starting balancer and catalog janitor"); - this.balancerChore = getAndStartBalancerChore(this); - this.catalogJanitorChore = new CatalogJanitor(this, this); - startCatalogJanitorChore(); - registerMBean(); + if (!masterRecovery) { + // Start balancer and meta catalog janitor after meta and regions have + // been assigned. + status.setStatus("Starting balancer and catalog janitor"); + this.balancerChore = getAndStartBalancerChore(this); + this.catalogJanitorChore = new CatalogJanitor(this, this); + startCatalogJanitorChore(); + registerMBean(); + } status.markComplete("Initialization successful"); LOG.info("Master has completed initialization"); @@ -560,12 +567,14 @@ // master initialization. See HBASE-5916. this.serverManager.clearDeadServersWithSameHostNameAndPortOfOnlineServer(); - if (this.cpHost != null) { - // don't let cp initialization errors kill the master - try { - this.cpHost.postStartMaster(); - } catch (IOException ioe) { - LOG.error("Coprocessor postStartMaster() hook failed", ioe); + if (!masterRecovery) { + if (this.cpHost != null) { + // don't let cp initialization errors kill the master + try { + this.cpHost.postStartMaster(); + } catch (IOException ioe) { + LOG.error("Coprocessor postStartMaster() hook failed", ioe); + } } } } @@ -1433,13 +1442,9 @@ if (!becomeActiveMaster(status)) { return Boolean.FALSE; } - initializeZKBasedSystemTrackers(); - // Update in-memory structures to reflect our earlier Root/Meta assignment. - assignRootAndMeta(status); - // process RIT if any - // TODO: Why does this not call AssignmentManager.joinCluster? Otherwise - // we are not processing dead servers if any. - assignmentManager.processDeadServersAndRegionsInTransition(); + serverShutdownHandlerEnabled = false; + initialized = false; + finishInitialization(status, true); return Boolean.TRUE; } finally { status.cleanup(); Index: src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java (revision 1345218) +++ src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java (working copy) @@ -81,7 +81,7 @@ private final MasterServices services; public MasterFileSystem(Server master, MasterServices services, - MasterMetrics metrics) + MasterMetrics metrics, boolean masterRecovery) throws IOException { this.conf = master.getConfiguration(); this.master = master; @@ -103,7 +103,7 @@ if (this.distributedLogSplitting) { this.splitLogManager = new SplitLogManager(master.getZooKeeper(), master.getConfiguration(), master, master.getServerName().toString()); - this.splitLogManager.finishInitialization(); + this.splitLogManager.finishInitialization(masterRecovery); } else { this.splitLogManager = null; } Index: src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java (revision 1345218) +++ src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java (working copy) @@ -182,9 +182,11 @@ stopper); } - public void finishInitialization() { - Threads.setDaemonThreadRunning(timeoutMonitor.getThread(), serverName + - ".splitLogManagerTimeoutMonitor"); + public void finishInitialization(boolean masterRecovery) { + if (!masterRecovery) { + Threads.setDaemonThreadRunning(timeoutMonitor.getThread(), serverName + + ".splitLogManagerTimeoutMonitor"); + } // Watcher can be null during tests with Mock'd servers. if (this.watcher != null) { this.watcher.registerListener(this); @@ -1196,4 +1198,7 @@ return statusMsg; } } + public void finishInitialization() { + finishInitialization(false); + } } Index: src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java (revision 1345218) +++ src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java (working copy) @@ -148,7 +148,7 @@ private final AssignmentManager asm; MockMasterServices(final Server server) throws IOException { - this.mfs = new MasterFileSystem(server, this, null); + this.mfs = new MasterFileSystem(server, this, null, false); this.asm = Mockito.mock(AssignmentManager.class); } Index: src/test/java/org/apache/hadoop/hbase/master/TestMasterZKSessionRecovery.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/TestMasterZKSessionRecovery.java (revision 1345218) +++ src/test/java/org/apache/hadoop/hbase/master/TestMasterZKSessionRecovery.java (working copy) @@ -21,11 +21,38 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.NavigableMap; +import java.util.TreeMap; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HColumnDescriptor; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.MediumTests; import org.apache.hadoop.hbase.MiniHBaseCluster; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.catalog.MetaEditor; +import org.apache.hadoop.hbase.catalog.MetaReader; +import org.apache.hadoop.hbase.client.HBaseAdmin; +import org.apache.hadoop.hbase.client.HTable; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.executor.EventHandler; +import org.apache.hadoop.hbase.executor.RegionTransitionData; +import org.apache.hadoop.hbase.master.TestAssignmentManager.MockedLoadBalancer; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.zookeeper.ZKAssign; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.apache.zookeeper.KeeperException; import org.junit.After; import org.junit.Before; @@ -46,6 +73,8 @@ static { Configuration conf = TEST_UTIL.getConfiguration(); conf.setLong("hbase.master.zksession.recover.timeout", 50000); + conf.setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, + MockLoadBalancer.class, LoadBalancer.class); } @Before @@ -92,5 +121,93 @@ new KeeperException.SessionExpiredException()); assertFalse(m.isStopped()); } + + /** + * Tests that the master does not call retainAssignment after recovery from + * expired zookeeper session. Without the HBASE-6046 fix master always tries + * to assign all the user regions by calling retainAssignment. + */ + @Test + public void testRegionAssignmentAfterMasterRecoveryDueToZKExpiry() throws Exception { + MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster(); + cluster.startRegionServer(); + HMaster m = cluster.getMaster(); + // now the cluster is up. So assign some regions. + HBaseAdmin admin = new HBaseAdmin(TEST_UTIL.getConfiguration()); + byte[][] SPLIT_KEYS = new byte[][] { Bytes.toBytes("a"), Bytes.toBytes("b"), + Bytes.toBytes("c"), Bytes.toBytes("d"), Bytes.toBytes("e"), Bytes.toBytes("f"), + Bytes.toBytes("g"), Bytes.toBytes("h"), Bytes.toBytes("i"), Bytes.toBytes("j") }; + + String tableName = "testRegionAssignmentAfterMasterRecoveryDueToZKExpiry"; + admin.createTable(new HTableDescriptor(tableName), SPLIT_KEYS); + ZooKeeperWatcher zooKeeperWatcher = HBaseTestingUtility.getZooKeeperWatcher(TEST_UTIL); + ZKAssign.blockUntilNoRIT(zooKeeperWatcher); + m.getZooKeeperWatcher().close(); + MockLoadBalancer.retainAssignCalled = false; + m.abort("Test recovery from zk session expired", new KeeperException.SessionExpiredException()); + assertFalse(m.isStopped()); + // The recovered master should not call retainAssignment, as it is not a + // clean startup. + assertFalse("Retain assignment should not be called", MockLoadBalancer.retainAssignCalled); + } + + public static class MockLoadBalancer extends DefaultLoadBalancer { + static boolean retainAssignCalled = false; + + @Override + public Map> retainAssignment( + Map regions, List servers) { + retainAssignCalled = true; + return super.retainAssignment(regions, servers); + } + } + + /** + * Tests whether the logs are split when master recovers from a expired + * zookeeper session and an RS goes down. + */ + @Test(timeout = 60000) + public void testLogSplittingAfterMasterRecoveryDueToZKExpiry() throws IOException, + KeeperException, InterruptedException { + MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster(); + cluster.startRegionServer(); + HMaster m = cluster.getMaster(); + // now the cluster is up. So assign some regions. + HBaseAdmin admin = new HBaseAdmin(TEST_UTIL.getConfiguration()); + byte[][] SPLIT_KEYS = new byte[][] { Bytes.toBytes("1"), Bytes.toBytes("2"), + Bytes.toBytes("3"), Bytes.toBytes("4"), Bytes.toBytes("5") }; + + String tableName = "testLogSplittingAfterMasterRecoveryDueToZKExpiry"; + HTableDescriptor htd = new HTableDescriptor(tableName); + HColumnDescriptor hcd = new HColumnDescriptor("col"); + htd.addFamily(hcd); + admin.createTable(htd, SPLIT_KEYS); + ZooKeeperWatcher zooKeeperWatcher = HBaseTestingUtility.getZooKeeperWatcher(TEST_UTIL); + ZKAssign.blockUntilNoRIT(zooKeeperWatcher); + HTable table = new HTable(TEST_UTIL.getConfiguration(), tableName); + + Put p = null; + int numberOfPuts = 0; + for (numberOfPuts = 0; numberOfPuts < 6; numberOfPuts++) { + p = new Put(Bytes.toBytes(numberOfPuts)); + p.add(Bytes.toBytes("col"), Bytes.toBytes("ql"), Bytes.toBytes("value" + numberOfPuts)); + table.put(p); + } + m.getZooKeeperWatcher().close(); + m.abort("Test recovery from zk session expired", new KeeperException.SessionExpiredException()); + assertFalse(m.isStopped()); + cluster.getRegionServer(0).abort("Aborting"); + // Without patch for HBASE-6046 this test case will always timeout + // with patch the test case should pass. + Scan scan = new Scan(); + int numberOfRows = 0; + ResultScanner scanner = table.getScanner(scan); + Result[] result = scanner.next(1); + while (result != null && result.length > 0) { + numberOfRows++; + result = scanner.next(1); + } + assertEquals("Number of rows should be equal to number of puts.", numberOfPuts, numberOfRows); + } }