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 e8457877b59..7784c75b421 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,14 +2201,6 @@ 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. *

@@ -2244,9 +2236,6 @@ 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 " + @@ -2255,9 +2244,6 @@ 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"); } @@ -2451,8 +2437,6 @@ 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(); @@ -2486,13 +2470,6 @@ 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 @@ -2501,37 +2478,21 @@ public class AssignmentManager extends ZooKeeperListener { regions.put(regionInfo, regionLocation); addToServers(regionLocation, regionInfo); } - 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); + addTheTablesInPartialState(this.disablingTables, this.enablingTables, regionInfo, + tableName); } } return offlineServers; } - private void enableTableIfNotDisabledOrDisablingOrEnabling(boolean disabled, - boolean disablingOrEnabling, String tableName) { - if (!disabled && !disablingOrEnabling - && !getZKTable().isEnabledTable(tableName)) { - setEnabledTable(tableName); - } - } - - private Boolean addTheTablesInPartialState(Set disablingTables, + private void 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; + } } /** @@ -3348,16 +3309,4 @@ 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 cc56b620ef0..03623388a48 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -626,9 +626,6 @@ 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()); @@ -654,19 +651,12 @@ 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 143c8c40baa..7de80d32258 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,7 +81,8 @@ 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. - am.getZKTable().setDeletedTable(Bytes.toString(tableName)); + // Call to undisableTable does this. TODO: Make a more formal purge table. + am.getZKTable().setEnabledTable(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 2ec6677c2cb..4068b0dc562 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,27 +327,13 @@ 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){ - LOG.info("The table " + hri.getTableNameAsString() - + " was disabled. Hence not proceeding."); - return false; - } + if (disabled) 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 76bae4a26e7..093f2890031 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 (this.cache.get(tableName) != null && !isEnabledTable(tableName)) { + if (!isEnabledTable(tableName)) { return false; } setTableState(tableName, TableState.DISABLING); @@ -267,7 +267,10 @@ public class ZKTable { } public boolean isEnabledTable(String tableName) { - return isTableState(tableName, TableState.ENABLED); + synchronized (this.cache) { + // No entry in cache means enabled table. + return !this.cache.containsKey(tableName); + } } /** @@ -282,7 +285,7 @@ public class ZKTable { public static boolean isEnabledTable(final ZooKeeperWatcher zkw, final String tableName) throws KeeperException { - return getTableState(zkw, tableName) == TableState.ENABLED; + return getTableState(zkw, tableName) == null; } public boolean isDisablingOrDisabledTable(final String tableName) { @@ -334,47 +337,23 @@ public class ZKTable { } /** - * Deletes the table in zookeeper. Fails silently if the + * Enables 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 setDeletedTable(final String tableName) + public void setEnabledTable(final String tableName) throws KeeperException { synchronized (this.cache) { if (this.cache.remove(tableName) == null) { - LOG.warn("Moving table " + tableName + " state to deleted but was " + - "already deleted"); + LOG.warn("Moving table " + tableName + " state to enabled but was " + + "already enabled"); } 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 9541b407c1a..fb109c3edd5 100644 --- a/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java +++ b/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java @@ -222,9 +222,6 @@ 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); @@ -239,9 +236,6 @@ 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 { @@ -313,9 +307,6 @@ 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 3122b159224..b7a8270cf3b 100644 --- a/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java +++ b/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java @@ -69,8 +69,6 @@ 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 d3ea94b5b19..63882e199fe 100644 --- a/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java +++ b/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java @@ -743,8 +743,6 @@ 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)); @@ -790,9 +788,6 @@ 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 ec08b1740c1..777c67d9bd4 100644 --- a/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java +++ b/src/test/java/org/apache/hadoop/hbase/master/TestMasterRestartAfterDisablingTable.java @@ -20,7 +20,6 @@ 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; @@ -97,9 +96,6 @@ 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()); @@ -112,8 +108,6 @@ 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 0a78f4039f3..3de482bfc77 100644 --- a/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java +++ b/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTable.java @@ -68,42 +68,29 @@ public class TestZKTable { ZooKeeperWatcher zkw = new ZooKeeperWatcher(TEST_UTIL.getConfiguration(), name, abortable, true); ZKTable zkt = new ZKTable(zkw); - assertFalse(zkt.isEnabledTable(name)); + assertTrue(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