From 2ec60a022e49fe2f28f699f5afa77fe6cff2d7eb Mon Sep 17 00:00:00 2001 From: Zhihong Yu Date: Wed, 14 Mar 2012 20:29:54 +0000 Subject: [PATCH] HBASE-5206 port HBASE-5155 to TRUNK (Ashutosh Jindal) git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1300711 13f79535-47bb-0310-9956-ffa450edef68 --- .../hbase/master/AssignmentManager.java | 61 +++++++++++++++++-- .../apache/hadoop/hbase/master/HMaster.java | 10 +++ .../master/handler/DeleteTableHandler.java | 3 +- .../master/handler/ServerShutdownHandler.java | 16 ++++- .../hadoop/hbase/zookeeper/ZKTable.java | 41 ++++++++++--- .../apache/hadoop/hbase/client/TestAdmin.java | 9 +++ .../hadoop/hbase/master/TestMaster.java | 2 + .../hbase/master/TestMasterFailover.java | 5 ++ .../TestMasterRestartAfterDisablingTable.java | 6 ++ .../hadoop/hbase/zookeeper/TestZKTable.java | 15 ++++- 10 files changed, 149 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 7784c75b421..e8457877b59 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -1586,7 +1586,7 @@ public class AssignmentManager extends ZooKeeperListener { if(versionOfOfflineNode != -1){ if (isDisabledorDisablingRegionInRIT(region)) { return; - } + } else setEnabledTable(region); } } @@ -2201,6 +2201,14 @@ public class AssignmentManager extends ZooKeeperListener { LOG.info("Bulk assigning done"); } + private void setEnabledTable(HRegionInfo hri) { + String tableName = hri.getTableNameAsString(); + boolean isTableEnabled = this.zkTable.isEnabledTable(tableName); + if (!isTableEnabled) { + setEnabledTable(tableName); + } + } + /** * Assigns all user regions, if any exist. Used during cluster startup. *

@@ -2236,6 +2244,9 @@ public class AssignmentManager extends ZooKeeperListener { } else { // assign regions in round-robin fashion assignUserRegions(new ArrayList(allRegions.keySet()), servers); + for (HRegionInfo hri : allRegions.keySet()) { + setEnabledTable(hri); + } return; } LOG.info("Bulk assigning " + allRegions.size() + " region(s) across " + @@ -2244,6 +2255,9 @@ public class AssignmentManager extends ZooKeeperListener { // Use fixed count thread pool assigning. BulkAssigner ba = new StartupBulkAssigner(this.master, bulkPlan, this); ba.bulkAssign(); + for (HRegionInfo hri : allRegions.keySet()) { + setEnabledTable(hri); + } LOG.info("Bulk assigning done"); } @@ -2437,6 +2451,8 @@ public class AssignmentManager extends ZooKeeperListener { new TreeMap>>(); // Iterate regions in META for (Result result : results) { + boolean disabled = false; + boolean disablingOrEnabling = false; Pair region = MetaReader.parseCatalogResult(result); if (region == null) continue; HRegionInfo regionInfo = region.getFirst(); @@ -2470,6 +2486,13 @@ public class AssignmentManager extends ZooKeeperListener { offlineServers.put(regionLocation, offlineRegions); } offlineRegions.add(new Pair(regionInfo, result)); + disabled = checkIfRegionBelongsToDisabled(regionInfo); + disablingOrEnabling = addTheTablesInPartialState(this.disablingTables, + this.enablingTables, regionInfo, tableName); + // need to enable the table if not disabled or disabling or enabling + // this will be used in rolling restarts + enableTableIfNotDisabledOrDisablingOrEnabling(disabled, + disablingOrEnabling, tableName); } else { // Region is being served and on an active server // add only if region not in disabled and enabling table @@ -2478,21 +2501,37 @@ public class AssignmentManager extends ZooKeeperListener { regions.put(regionInfo, regionLocation); addToServers(regionLocation, regionInfo); } - addTheTablesInPartialState(this.disablingTables, this.enablingTables, regionInfo, - tableName); + disablingOrEnabling = addTheTablesInPartialState(this.disablingTables, + this.enablingTables, regionInfo, tableName); + disabled = checkIfRegionBelongsToDisabled(regionInfo); + // need to enable the table if not disabled or disabling or enabling + // this will be used in rolling restarts + enableTableIfNotDisabledOrDisablingOrEnabling(disabled, + disablingOrEnabling, tableName); } } return offlineServers; } - private void addTheTablesInPartialState(Set disablingTables, + private void enableTableIfNotDisabledOrDisablingOrEnabling(boolean disabled, + boolean disablingOrEnabling, String tableName) { + if (!disabled && !disablingOrEnabling + && !getZKTable().isEnabledTable(tableName)) { + setEnabledTable(tableName); + } + } + + private Boolean addTheTablesInPartialState(Set disablingTables, Set enablingTables, HRegionInfo regionInfo, String disablingTableName) { if (checkIfRegionBelongsToDisabling(regionInfo)) { disablingTables.add(disablingTableName); + return true; } else if (checkIfRegionsBelongsToEnabling(regionInfo)) { enablingTables.add(disablingTableName); - } + return true; + } + return false; } /** @@ -3309,4 +3348,16 @@ public class AssignmentManager extends ZooKeeperListener { this.threadPoolExecutorService.shutdown(); } } + + protected void setEnabledTable(String tableName) { + try { + this.zkTable.setEnabledTable(tableName); + } catch (KeeperException e) { + // here we can abort as it is the start up flow + String errorMsg = "Unable to ensure that the table " + tableName + + " is enabled because of a ZooKeeper issue:" + e; + LOG.error(errorMsg); + this.master.abort(errorMsg, e); + } + } } diff --git a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 03623388a48..cc56b620ef0 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -626,6 +626,9 @@ Server { this.assignmentManager.regionOnline(HRegionInfo.ROOT_REGIONINFO, this.catalogTracker.getRootLocation()); } + // Enable the ROOT table if on process fail over the RS containing ROOT + // was active. + enableCatalogTables(Bytes.toString(HConstants.ROOT_TABLE_NAME)); LOG.info("-ROOT- assigned=" + assigned + ", rit=" + rit + ", location=" + catalogTracker.getRootLocation()); @@ -651,12 +654,19 @@ Server { this.assignmentManager.regionOnline(HRegionInfo.FIRST_META_REGIONINFO, this.catalogTracker.getMetaLocation()); } + enableCatalogTables(Bytes.toString(HConstants.META_TABLE_NAME)); LOG.info(".META. assigned=" + assigned + ", rit=" + rit + ", location=" + catalogTracker.getMetaLocation()); status.setStatus("META and ROOT assigned."); return assigned; } + private void enableCatalogTables(String catalogTableName) { + if (!this.assignmentManager.getZKTable().isEnabledTable(catalogTableName)) { + this.assignmentManager.setEnabledTable(catalogTableName); + } + } + void fixupDaughters(final MonitoredTask status) throws IOException { final Map offlineSplitParents = new HashMap(); diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java index 7de80d32258..143c8c40baa 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java @@ -81,8 +81,7 @@ public class DeleteTableHandler extends TableEventHandler { this.masterServices.getTableDescriptors().remove(Bytes.toString(tableName)); // If entry for this table in zk, and up in AssignmentManager, remove it. - // Call to undisableTable does this. TODO: Make a more formal purge table. - am.getZKTable().setEnabledTable(Bytes.toString(tableName)); + am.getZKTable().setDeletedTable(Bytes.toString(tableName)); } @Override diff --git a/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java b/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java index 4068b0dc562..2ec6677c2cb 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java +++ b/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java @@ -327,13 +327,27 @@ public class ServerShutdownHandler extends EventHandler { public static boolean processDeadRegion(HRegionInfo hri, Result result, AssignmentManager assignmentManager, CatalogTracker catalogTracker) throws IOException { + boolean tablePresent = assignmentManager.getZKTable().isTablePresent( + hri.getTableNameAsString()); + if (!tablePresent) { + LOG.info("The table " + hri.getTableNameAsString() + + " was deleted. Hence not proceeding."); + return false; + } // If table is not disabled but the region is offlined, boolean disabled = assignmentManager.getZKTable().isDisabledTable( hri.getTableNameAsString()); - if (disabled) return false; + if (disabled){ + LOG.info("The table " + hri.getTableNameAsString() + + " was disabled. Hence not proceeding."); + return false; + } if (hri.isOffline() && hri.isSplit()) { LOG.debug("Offlined and split region " + hri.getRegionNameAsString() + "; checking daughter presence"); + if (MetaReader.getRegion(catalogTracker, hri.getRegionName()) == null) { + return false; + } fixupDaughters(result, assignmentManager, catalogTracker); return false; } diff --git a/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTable.java b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTable.java index 093f2890031..76bae4a26e7 100644 --- a/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTable.java +++ b/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTable.java @@ -218,7 +218,7 @@ public class ZKTable { public boolean checkEnabledAndSetDisablingTable(final String tableName) throws KeeperException { synchronized (this.cache) { - if (!isEnabledTable(tableName)) { + if (this.cache.get(tableName) != null && !isEnabledTable(tableName)) { return false; } setTableState(tableName, TableState.DISABLING); @@ -267,10 +267,7 @@ public class ZKTable { } public boolean isEnabledTable(String tableName) { - synchronized (this.cache) { - // No entry in cache means enabled table. - return !this.cache.containsKey(tableName); - } + return isTableState(tableName, TableState.ENABLED); } /** @@ -285,7 +282,7 @@ public class ZKTable { public static boolean isEnabledTable(final ZooKeeperWatcher zkw, final String tableName) throws KeeperException { - return getTableState(zkw, tableName) == null; + return getTableState(zkw, tableName) == TableState.ENABLED; } public boolean isDisablingOrDisabledTable(final String tableName) { @@ -337,23 +334,47 @@ public class ZKTable { } /** - * Enables the table in zookeeper. Fails silently if the + * Deletes the table in zookeeper. Fails silently if the * table is not currently disabled in zookeeper. Sets no watches. * @param tableName * @throws KeeperException unexpected zookeeper exception */ - public void setEnabledTable(final String tableName) + public void setDeletedTable(final String tableName) throws KeeperException { synchronized (this.cache) { if (this.cache.remove(tableName) == null) { - LOG.warn("Moving table " + tableName + " state to enabled but was " + - "already enabled"); + LOG.warn("Moving table " + tableName + " state to deleted but was " + + "already deleted"); } ZKUtil.deleteNodeFailSilent(this.watcher, ZKUtil.joinZNode(this.watcher.tableZNode, tableName)); } } + + /** + * Sets the ENABLED state in the cache and deletes the zookeeper node. Fails + * silently if the node is not in enabled in zookeeper + * + * @param tableName + * @throws KeeperException + */ + public void setEnabledTable(final String tableName) throws KeeperException { + setTableState(tableName, TableState.ENABLED); + } + /** + * check if table is present . + * + * @param tableName + * @return True if the table is present + */ + public boolean isTablePresent(final String tableName) { + synchronized (this.cache) { + TableState state = this.cache.get(tableName); + return !(state == null); + } + } + /** * Gets a list of all the tables set as disabled in zookeeper. * @return Set of disabled tables, empty Set if none diff --git a/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java b/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java index fb109c3edd5..9541b407c1a 100644 --- a/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java +++ b/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java @@ -222,6 +222,9 @@ public class TestAdmin { ht.get(get); this.admin.disableTable(table); + assertTrue("Table must be disabled.", TEST_UTIL.getHBaseCluster() + .getMaster().getAssignmentManager().getZKTable().isDisabledTable( + Bytes.toString(table))); // Test that table is disabled get = new Get(row); @@ -236,6 +239,9 @@ public class TestAdmin { } assertTrue(ok); this.admin.enableTable(table); + assertTrue("Table must be enabled.", TEST_UTIL.getHBaseCluster() + .getMaster().getAssignmentManager().getZKTable().isEnabledTable( + Bytes.toString(table))); // Test that table is enabled try { @@ -307,6 +313,9 @@ public class TestAdmin { HConstants.CATALOG_FAMILY).close(); tables = this.admin.listTables(); assertEquals(numTables + 1, tables.length); + assertTrue("Table must be enabled.", TEST_UTIL.getHBaseCluster() + .getMaster().getAssignmentManager().getZKTable().isEnabledTable( + "testCreateTable")); } @Test diff --git a/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java b/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java index b7a8270cf3b..3122b159224 100644 --- a/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java +++ b/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java @@ -69,6 +69,8 @@ public class TestMaster { HMaster m = cluster.getMaster(); HTable ht = TEST_UTIL.createTable(TABLENAME, FAMILYNAME); + assertTrue(m.assignmentManager.getZKTable().isEnabledTable + (Bytes.toString(TABLENAME))); TEST_UTIL.loadTable(ht, FAMILYNAME); ht.close(); diff --git a/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java b/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java index 63882e199fe..d3ea94b5b19 100644 --- a/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java +++ b/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java @@ -743,6 +743,8 @@ public class TestMasterFailover { master.assignRegion(hri); } + assertTrue(" Table must be enabled.", master.getAssignmentManager() + .getZKTable().isEnabledTable("enabledTable")); // we also need regions assigned out on the dead server List enabledAndOnDeadRegions = new ArrayList(); enabledAndOnDeadRegions.add(enabledRegions.remove(0)); @@ -788,6 +790,9 @@ public class TestMasterFailover { ZKTable zktable = new ZKTable(zkw); zktable.setDisabledTable(Bytes.toString(disabledTable)); + assertTrue(" The enabled table should be identified on master fail over.", + zktable.isEnabledTable("enabledTable")); + /* * ZK = CLOSING */ diff --git a/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java b/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java index 777c67d9bd4..ec08b1740c1 100644 --- a/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java +++ b/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.master; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.util.List; @@ -96,6 +97,9 @@ public class TestMasterRestartAfterDisablingTable { cluster.hbaseCluster.waitOnMaster(activeMaster); cluster.waitForActiveAndReadyMaster(); + assertTrue("The table should not be in enabled state", cluster.getMaster() + .getAssignmentManager().getZKTable().isDisablingOrDisabledTable( + "tableRestart")); log("Enabling table\n"); // Need a new Admin, the previous one is on the old master HBaseAdmin admin = new HBaseAdmin(TEST_UTIL.getConfiguration()); @@ -108,6 +112,8 @@ public class TestMasterRestartAfterDisablingTable { assertEquals( "The assigned regions were not onlined after master switch except for the catalog tables.", 6, regions.size()); + assertTrue("The table should be in enabled state", cluster.getMaster() + .getAssignmentManager().getZKTable().isEnabledTable("tableRestart")); ht.close(); TEST_UTIL.shutdownMiniCluster(); } diff --git a/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java b/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java index 3de482bfc77..0a78f4039f3 100644 --- a/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java +++ b/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java @@ -68,29 +68,42 @@ public class TestZKTable { ZooKeeperWatcher zkw = new ZooKeeperWatcher(TEST_UTIL.getConfiguration(), name, abortable, true); ZKTable zkt = new ZKTable(zkw); - assertTrue(zkt.isEnabledTable(name)); + assertFalse(zkt.isEnabledTable(name)); assertFalse(zkt.isDisablingTable(name)); assertFalse(zkt.isDisabledTable(name)); assertFalse(zkt.isEnablingTable(name)); assertFalse(zkt.isDisablingOrDisabledTable(name)); assertFalse(zkt.isDisabledOrEnablingTable(name)); + assertFalse(zkt.isTablePresent(name)); zkt.setDisablingTable(name); assertTrue(zkt.isDisablingTable(name)); assertTrue(zkt.isDisablingOrDisabledTable(name)); assertFalse(zkt.getDisabledTables().contains(name)); + assertTrue(zkt.isTablePresent(name)); zkt.setDisabledTable(name); assertTrue(zkt.isDisabledTable(name)); assertTrue(zkt.isDisablingOrDisabledTable(name)); assertFalse(zkt.isDisablingTable(name)); assertTrue(zkt.getDisabledTables().contains(name)); + assertTrue(zkt.isTablePresent(name)); zkt.setEnablingTable(name); assertTrue(zkt.isEnablingTable(name)); assertTrue(zkt.isDisabledOrEnablingTable(name)); assertFalse(zkt.isDisabledTable(name)); assertFalse(zkt.getDisabledTables().contains(name)); + assertTrue(zkt.isTablePresent(name)); zkt.setEnabledTable(name); assertTrue(zkt.isEnabledTable(name)); assertFalse(zkt.isEnablingTable(name)); + assertTrue(zkt.isTablePresent(name)); + zkt.setDeletedTable(name); + assertFalse(zkt.isEnabledTable(name)); + assertFalse(zkt.isDisablingTable(name)); + assertFalse(zkt.isDisabledTable(name)); + assertFalse(zkt.isEnablingTable(name)); + assertFalse(zkt.isDisablingOrDisabledTable(name)); + assertFalse(zkt.isDisabledOrEnablingTable(name)); + assertFalse(zkt.isTablePresent(name)); } @org.junit.Rule