Index: src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (revision 1407365) +++ src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (working copy) @@ -142,9 +142,10 @@ // store all the table names in disabling state Set disablingTables = new HashSet(1); - // store all the enabling state tablenames. - Set enablingTables = new HashSet(1); - + // store all the enabling state table names and corresponding online servers' regions. + // This may be needed to avoid calling assign twice for the regions of the ENABLING table + // that could have been assigned through processRIT. + Map> enablingTables = new HashMap>(1); /** * Server to regions assignment map. * Contains the set of regions currently assigned to a given server. @@ -274,6 +275,16 @@ } /** + * Gives enabling table regions. + * + * @param tableName + * @return list of regionInfos + */ + public List getEnablingTableRegions(String tableName){ + return this.enablingTables.get(tableName); + } + + /** * Add a regionPlan for the specified region. * @param encodedName * @param plan @@ -364,7 +375,9 @@ // Recover the tables that were not fully moved to DISABLED state. // These tables are in DISABLING state when the master restarted/switched. boolean isWatcherCreated = recoverTableInDisablingState(this.disablingTables); - recoverTableInEnablingState(this.enablingTables, isWatcherCreated); + recoverTableInEnablingState(this.enablingTables.keySet(), isWatcherCreated); + this.enablingTables.clear(); + this.disablingTables.clear(); } /** @@ -509,6 +522,10 @@ String encodedRegionName = regionInfo.getEncodedName(); LOG.info("Processing region " + regionInfo.getRegionNameAsString() + " in state " + data.getEventType()); + List hris = this.enablingTables.get(regionInfo.getTableNameAsString()); + if (hris != null && !hris.isEmpty()) { + hris.remove(regionInfo); + } synchronized (regionsInTransition) { RegionState regionState = regionsInTransition.get(encodedRegionName); if (regionState != null || @@ -2306,11 +2323,12 @@ // Skip assignment for regions of tables in DISABLING state also because // during clean cluster startup no RS is alive and regions map also doesn't // have any information about the regions. See HBASE-6281. - Set disablingAndDisabledTables = new HashSet(this.disablingTables); - disablingAndDisabledTables.addAll(this.zkTable.getDisabledTables()); + Set disablingDisabledAndEnablingTables = new HashSet(this.disablingTables); + disablingDisabledAndEnablingTables.addAll(this.zkTable.getDisabledTables()); + disablingDisabledAndEnablingTables.addAll(this.enablingTables.keySet()); // Scan META for all user regions, skipping any disabled tables Map allRegions = MetaReader.fullScan(catalogTracker, - disablingAndDisabledTables, true); + disablingDisabledAndEnablingTables, true); if (allRegions == null || allRegions.isEmpty()) return; // Get all available servers @@ -2558,13 +2576,14 @@ // from ENABLED state when application calls disableTable. // It can't be in DISABLED state, because DISABLED states transitions // from DISABLING state. - if (false == checkIfRegionsBelongsToEnabling(regionInfo)) { - LOG.warn("Region " + regionInfo.getEncodedName() + - " has null regionLocation." + " But its table " + tableName + - " isn't in ENABLING state."); + boolean enabling = checkIfRegionsBelongsToEnabling(regionInfo); + addTheTablesInPartialState(regionInfo); + if (enabling) { + addToEnablingTableRegions(regionInfo); + } else { + LOG.warn("Region " + regionInfo.getEncodedName() + " has null regionLocation." + + " But its table " + tableName + " isn't in ENABLING state."); } - addTheTablesInPartialState(this.disablingTables, this.enablingTables, regionInfo, - tableName); } else if (!onlineServers.contains(regionLocation)) { // Region is located on a server that isn't online List> offlineRegions = @@ -2575,8 +2594,7 @@ } offlineRegions.add(new Pair(regionInfo, result)); disabled = checkIfRegionBelongsToDisabled(regionInfo); - disablingOrEnabling = addTheTablesInPartialState(this.disablingTables, - this.enablingTables, regionInfo, tableName); + disablingOrEnabling = addTheTablesInPartialState(regionInfo); // need to enable the table if not disabled or disabling or enabling // this will be used in rolling restarts enableTableIfNotDisabledOrDisablingOrEnabling(disabled, @@ -2597,16 +2615,18 @@ } // Region is being served and on an active server // add only if region not in disabled and enabling table - if (false == checkIfRegionBelongsToDisabled(regionInfo) - && false == checkIfRegionsBelongsToEnabling(regionInfo)) { + boolean enabling = checkIfRegionsBelongsToEnabling(regionInfo); + disabled = checkIfRegionBelongsToDisabled(regionInfo); + if (!enabling && !disabled) { synchronized (this.regions) { regions.put(regionInfo, regionLocation); addToServers(regionLocation, regionInfo); } } - disablingOrEnabling = addTheTablesInPartialState(this.disablingTables, - this.enablingTables, regionInfo, tableName); - disabled = checkIfRegionBelongsToDisabled(regionInfo); + disablingOrEnabling = addTheTablesInPartialState(regionInfo); + if (enabling) { + addToEnablingTableRegions(regionInfo); + } // need to enable the table if not disabled or disabling or enabling // this will be used in rolling restarts enableTableIfNotDisabledOrDisablingOrEnabling(disabled, @@ -2616,6 +2636,18 @@ return offlineServers; } + private void addToEnablingTableRegions(HRegionInfo regionInfo) { + String tableName = regionInfo.getTableNameAsString(); + List hris = this.enablingTables.get(tableName); + if (!hris.contains(regionInfo)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Adding region" + regionInfo.getRegionNameAsString() + + " to enabling table " + tableName + "."); + } + hris.add(regionInfo); + } + } + private void enableTableIfNotDisabledOrDisablingOrEnabling(boolean disabled, boolean disablingOrEnabling, String tableName) { if (!disabled && !disablingOrEnabling @@ -2624,14 +2656,15 @@ } } - private Boolean addTheTablesInPartialState(Set disablingTables, - Set enablingTables, HRegionInfo regionInfo, - String disablingTableName) { + private Boolean addTheTablesInPartialState(HRegionInfo regionInfo) { + String tableName = regionInfo.getTableNameAsString(); if (checkIfRegionBelongsToDisabling(regionInfo)) { - disablingTables.add(disablingTableName); + this.disablingTables.add(tableName); return true; } else if (checkIfRegionsBelongsToEnabling(regionInfo)) { - enablingTables.add(disablingTableName); + if (!this.enablingTables.containsKey(tableName)) { + this.enablingTables.put(tableName, new ArrayList()); + } return true; } return false; Index: src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java (revision 1407365) +++ src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java (working copy) @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.master.handler; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutorService; @@ -27,6 +28,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.Server; +import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.catalog.CatalogTracker; @@ -34,7 +36,11 @@ import org.apache.hadoop.hbase.executor.EventHandler; import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.BulkAssigner; +import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.RegionPlan; +import org.apache.hadoop.hbase.master.ServerManager; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.Pair; import org.apache.zookeeper.KeeperException; /** @@ -46,6 +52,7 @@ private final String tableNameStr; private final AssignmentManager assignmentManager; private final CatalogTracker ct; + private boolean retainAssignment = false; public EnableTableHandler(Server server, byte [] tableName, CatalogTracker catalogTracker, AssignmentManager assignmentManager, @@ -56,6 +63,7 @@ this.tableNameStr = Bytes.toString(tableName); this.ct = catalogTracker; this.assignmentManager = assignmentManager; + this.retainAssignment = skipTableStateCheck; // Check if table exists if (!MetaReader.tableExists(catalogTracker, this.tableNameStr)) { throw new TableNotFoundException(Bytes.toString(tableName)); @@ -99,10 +107,12 @@ LOG.error("Error trying to enable the table " + this.tableNameStr, e); } catch (KeeperException e) { LOG.error("Error trying to enable the table " + this.tableNameStr, e); + } catch (InterruptedException e) { + LOG.error("Error trying to enable the table " + this.tableNameStr, e); } } - private void handleEnableTable() throws IOException, KeeperException { + private void handleEnableTable() throws IOException, KeeperException, InterruptedException { // I could check table is disabling and if so, not enable but require // that user first finish disabling but that might be obnoxious. @@ -111,18 +121,18 @@ boolean done = false; // Get the regions of this table. We're done when all listed // tables are onlined. - List regionsInMeta; - regionsInMeta = MetaReader.getTableRegions(this.ct, tableName, true); - int countOfRegionsInTable = regionsInMeta.size(); - List regions = regionsToAssign(regionsInMeta); + List> tableRegionsAndLocations = MetaReader + .getTableRegionsAndLocations(this.ct, tableName, true); + int countOfRegionsInTable = tableRegionsAndLocations.size(); + List regions = regionsToAssignWithServerName(tableRegionsAndLocations); int regionsCount = regions.size(); if (regionsCount == 0) { done = true; } LOG.info("Table has " + countOfRegionsInTable + " regions of which " + regionsCount + " are offline."); - BulkEnabler bd = new BulkEnabler(this.server, regions, - countOfRegionsInTable); + BulkEnabler bd = new BulkEnabler(this.server, regions, countOfRegionsInTable, + this.retainAssignment); try { if (bd.bulkAssign()) { done = true; @@ -140,17 +150,34 @@ /** * @param regionsInMeta This datastructure is edited by this method. - * @return The regionsInMeta list minus the regions that have - * been onlined; i.e. List of regions that need onlining. + * @return List of regions neither in transition nor assigned. * @throws IOException */ - private List regionsToAssign( - final List regionsInMeta) - throws IOException { - final List onlineRegions = - this.assignmentManager.getRegionsOfTable(tableName); - regionsInMeta.removeAll(onlineRegions); - return regionsInMeta; + private List regionsToAssignWithServerName( + final List> regionsInMeta) throws IOException { + ServerManager serverManager = ((HMaster) this.server).getServerManager(); + List regions = new ArrayList(); + List enablingTableRegions = this.assignmentManager + .getEnablingTableRegions(this.tableNameStr); + final List onlineRegions = this.assignmentManager.getRegionsOfTable(tableName); + for (Pair regionLocation : regionsInMeta) { + HRegionInfo hri = regionLocation.getFirst(); + ServerName sn = regionLocation.getSecond(); + if (this.retainAssignment) { + // Region may be available in enablingTableRegions during master startup only. + if (enablingTableRegions != null && enablingTableRegions.contains(hri)) { + regions.add(hri); + if (sn != null && serverManager.isServerOnline(sn)) { + this.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, sn)); + } + } + } else if (onlineRegions.contains(hri)) { + continue; + } else { + regions.add(hri); + } + } + return regions; } /** @@ -160,12 +187,14 @@ private final List regions; // Count of regions in table at time this assign was launched. private final int countOfRegionsInTable; + private final boolean retainAssignment; BulkEnabler(final Server server, final List regions, - final int countOfRegionsInTable) { + final int countOfRegionsInTable,final boolean retainAssignment) { super(server); this.regions = regions; this.countOfRegionsInTable = countOfRegionsInTable; + this.retainAssignment = retainAssignment; } @Override @@ -173,7 +202,7 @@ boolean roundRobinAssignment = this.server.getConfiguration().getBoolean( "hbase.master.enabletable.roundrobin", false); - if (!roundRobinAssignment) { + if (retainAssignment || !roundRobinAssignment) { for (HRegionInfo region : regions) { if (assignmentManager.isRegionInTransition(region) != null) { continue; @@ -181,7 +210,11 @@ final HRegionInfo hri = region; pool.execute(new Runnable() { public void run() { - assignmentManager.assign(hri, true); + if (retainAssignment) { + assignmentManager.assign(hri, true, false, false); + } else { + assignmentManager.assign(hri, true); + } } }); } Index: src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (revision 1407365) +++ src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (working copy) @@ -74,6 +74,7 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import org.mockito.Mockito; +import org.mockito.internal.util.reflection.Whitebox; import com.google.protobuf.ServiceException; @@ -91,6 +92,10 @@ private static final HRegionInfo REGIONINFO = new HRegionInfo(Bytes.toBytes("t"), HConstants.EMPTY_START_ROW, HConstants.EMPTY_START_ROW); + private static final HRegionInfo REGIONINFO_2 = new HRegionInfo(Bytes.toBytes("t"), + Bytes.toBytes("a"),Bytes.toBytes( "b")); + private static int assignmentCount; + private static boolean enabling = false; // Mocked objects or; get redone for each test. private Server server; @@ -768,14 +773,27 @@ // with an encoded name by doing a Get on .META. HRegionInterface ri = Mockito.mock(HRegionInterface.class); // Get a meta row result that has region up on SERVERNAME_A for REGIONINFO + Result[] result = null; + if (enabling) { + result = new Result[2]; + result[0] = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); + result[1] = getMetaTableRowResult(REGIONINFO_2, SERVERNAME_A); + } Result r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A); Mockito.when(ri .openScanner((byte[]) Mockito.any(), (Scan) Mockito.any())). thenReturn(System.currentTimeMillis()); - // Return good result 'r' first and then return null to indicate end of scan - Mockito.when(ri.next(Mockito.anyLong(), Mockito.anyInt())).thenReturn(new Result[] { r }); - // If a get, return the above result too for REGIONINFO - Mockito.when(ri.get((byte[]) Mockito.any(), (Get) Mockito.any())). - thenReturn(r); + if (enabling) { + Mockito.when(ri.next(Mockito.anyLong(), Mockito.anyInt())).thenReturn(result, result, result, + (Result[]) null); + // If a get, return the above result too for REGIONINFO_2 + Mockito.when(ri.get((byte[]) Mockito.any(), (Get) Mockito.any())).thenReturn( + getMetaTableRowResult(REGIONINFO_2, SERVERNAME_A)); + } else { + // Return good result 'r' first and then return null to indicate end of scan + Mockito.when(ri.next(Mockito.anyLong(), Mockito.anyInt())).thenReturn(new Result[] { r }); + // If a get, return the above result too for REGIONINFO + Mockito.when(ri.get((byte[]) Mockito.any(), (Get) Mockito.any())).thenReturn(r); + } // Get a connection w/ mocked up common methods. HConnection connection = HConnectionTestingUtility. getMockedConnectionAndDecorate(HTU.getConfiguration(), ri, SERVERNAME_B, @@ -892,6 +910,53 @@ } /** + * Test verifies whether all the enabling table regions assigned only once during master startup. + * + * @throws KeeperException + * @throws IOException + * @throws Exception + */ + @Test + public void testMasterRestartWhenTableInEnabling() throws KeeperException, IOException, Exception { + enabling = true; + this.server.getConfiguration().setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, + DefaultLoadBalancer.class, LoadBalancer.class); + Map serverAndLoad = new HashMap(); + serverAndLoad.put(SERVERNAME_A, null); + Mockito.when(this.serverManager.getOnlineServers()).thenReturn(serverAndLoad); + Mockito.when(this.serverManager.isServerOnline(SERVERNAME_B)).thenReturn(false); + Mockito.when(this.serverManager.isServerOnline(SERVERNAME_A)).thenReturn(true); + HTU.getConfiguration().setInt(HConstants.MASTER_PORT, 0); + Server server = new HMaster(HTU.getConfiguration()); + Whitebox.setInternalState(server, "serverManager", this.serverManager); + assignmentCount = 0; + AssignmentManagerWithExtrasForTesting am = setUpMockedAssignmentManager(server, + this.serverManager); + am.regionOnline(new HRegionInfo("t1".getBytes(), HConstants.EMPTY_START_ROW, + HConstants.EMPTY_END_ROW), SERVERNAME_A); + am.gate.set(false); + try { + // set table in enabling state. + am.getZKTable().setEnablingTable(REGIONINFO.getTableNameAsString()); + ZKAssign.createNodeOffline(this.watcher, REGIONINFO_2, SERVERNAME_B); + + am.joinCluster(); + while (!am.getZKTable().isEnabledTable(REGIONINFO.getTableNameAsString())) { + Thread.sleep(10); + } + assertEquals("Number of assignments should be equal.", 2, assignmentCount); + assertTrue("Table should be enabled.", + am.getZKTable().isEnabledTable(REGIONINFO.getTableNameAsString())); + } finally { + enabling = false; + am.getZKTable().setEnabledTable(REGIONINFO.getTableNameAsString()); + am.shutdown(); + ZKAssign.deleteAllNodes(this.watcher); + assignmentCount = 0; + } + } + + /** * Mocked load balancer class used in the testcase to make sure that the testcase waits until * random assignment is called and the gate variable is set to true. */ @@ -960,8 +1025,13 @@ @Override public void assign(HRegionInfo region, boolean setOfflineInZK, boolean forceNewPlan, boolean hijack) { - assignInvoked = true; - super.assign(region, setOfflineInZK, forceNewPlan, hijack); + if (enabling) { + assignmentCount++; + this.regionOnline(region, SERVERNAME_A); + } else { + assignInvoked = true; + super.assign(region, setOfflineInZK, forceNewPlan, hijack); + } } @Override