You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ra...@apache.org on 2012/10/02 11:19:49 UTC
svn commit: r1392802 - in /hbase/trunk/hbase-server/src:
main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java
test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
Author: ramkrishna
Date: Tue Oct 2 09:19:48 2012
New Revision: 1392802
URL: http://svn.apache.org/viewvc?rev=1392802&view=rev
Log:
HBASE-6317 Master clean start up and Partially enabled tables make region assignment inconsistent (RajeshBabu)
Modified:
hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java
hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java?rev=1392802&r1=1392801&r2=1392802&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java Tue Oct 2 09:19:48 2012
@@ -19,6 +19,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.LogFac
import org.apache.hadoop.classification.InterfaceAudience;
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;
@@ -36,7 +38,11 @@ import org.apache.hadoop.hbase.master.As
import org.apache.hadoop.hbase.master.BulkAssigner;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
+import org.apache.hadoop.hbase.master.RegionPlan;
+import org.apache.hadoop.hbase.master.RegionStates;
+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;
import org.cloudera.htrace.Trace;
@@ -50,6 +56,7 @@ public class EnableTableHandler extends
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,
@@ -60,6 +67,7 @@ public class EnableTableHandler extends
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));
@@ -111,10 +119,12 @@ public class EnableTableHandler extends
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.
@@ -123,18 +133,18 @@ public class EnableTableHandler extends
boolean done = false;
// Get the regions of this table. We're done when all listed
// tables are onlined.
- List<HRegionInfo> regionsInMeta;
- regionsInMeta = MetaReader.getTableRegions(this.ct, tableName, true);
- int countOfRegionsInTable = regionsInMeta.size();
- List<HRegionInfo> regions = regionsToAssign(regionsInMeta);
+ List<Pair<HRegionInfo, ServerName>> tableRegionsAndLocations = MetaReader
+ .getTableRegionsAndLocations(this.ct, tableName, true);
+ int countOfRegionsInTable = tableRegionsAndLocations.size();
+ List<HRegionInfo> regions = regionsToAssignWithServerName(tableRegionsAndLocations);
int regionsCount = regions.size();
if (regionsCount == 0) {
done = true;
}
LOG.info("Table '" + this.tableNameStr + "' 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;
@@ -158,19 +168,31 @@ public class EnableTableHandler extends
}
/**
- * @param regionsInMeta This datastructure is edited by this method.
- * @return The <code>regionsInMeta</code> list minus the regions that have
- * been onlined; i.e. List of regions that need onlining.
+ * @param regionsInMeta
+ * @return List of regions neither in transition nor assigned.
* @throws IOException
*/
- private List<HRegionInfo> regionsToAssign(
- final List<HRegionInfo> regionsInMeta)
- throws IOException {
- final List<HRegionInfo> onlineRegions =
- this.assignmentManager.getRegionStates()
- .getRegionsOfTable(tableName);
- regionsInMeta.removeAll(onlineRegions);
- return regionsInMeta;
+ private List<HRegionInfo> regionsToAssignWithServerName(
+ final List<Pair<HRegionInfo, ServerName>> regionsInMeta) throws IOException {
+ ServerManager serverManager = ((HMaster) this.server).getServerManager();
+ List<HRegionInfo> regions = new ArrayList<HRegionInfo>();
+ RegionStates regionStates = this.assignmentManager.getRegionStates();
+ for (Pair<HRegionInfo, ServerName> regionLocation : regionsInMeta) {
+ HRegionInfo hri = regionLocation.getFirst();
+ ServerName sn = regionLocation.getSecond();
+ if (!regionStates.isRegionInTransition(hri) && !regionStates.isRegionAssigned(hri)) {
+ if (this.retainAssignment && sn != null && serverManager.isServerOnline(sn)) {
+ this.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, sn));
+ }
+ regions.add(hri);
+ } else {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Skipping assign for the region " + hri + " during enable table "
+ + hri.getTableNameAsString() + " because its already in tranition or assigned.");
+ }
+ }
+ }
+ return regions;
}
/**
@@ -180,12 +202,14 @@ public class EnableTableHandler extends
private final List<HRegionInfo> 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<HRegionInfo> regions,
- final int countOfRegionsInTable) {
+ final int countOfRegionsInTable, boolean retainAssignment) {
super(server);
this.regions = regions;
this.countOfRegionsInTable = countOfRegionsInTable;
+ this.retainAssignment = retainAssignment;
}
@Override
@@ -193,7 +217,9 @@ public class EnableTableHandler extends
boolean roundRobinAssignment = this.server.getConfiguration().getBoolean(
"hbase.master.enabletable.roundrobin", false);
- if (!roundRobinAssignment) {
+ // In case of masterRestart always go with single assign. Going thro
+ // roundRobinAssignment will use bulkassign which may lead to double assignment.
+ if (retainAssignment || !roundRobinAssignment) {
for (HRegionInfo region : regions) {
if (assignmentManager.getRegionStates()
.isRegionInTransition(region)) {
@@ -202,7 +228,12 @@ public class EnableTableHandler extends
final HRegionInfo hri = region;
pool.execute(Trace.wrap(new Runnable() {
public void run() {
- assignmentManager.assign(hri, true);
+ if (retainAssignment) {
+ // Already plan is populated.
+ assignmentManager.assign(hri, true, false, false);
+ } else {
+ assignmentManager.assign(hri, true);
+ }
}
}));
}
Modified: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java?rev=1392802&r1=1392801&r2=1392802&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (original)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java Tue Oct 2 09:19:48 2012
@@ -54,6 +54,7 @@ import org.apache.hadoop.hbase.executor.
import org.apache.hadoop.hbase.master.RegionState.State;
import org.apache.hadoop.hbase.master.balancer.DefaultLoadBalancer;
import org.apache.hadoop.hbase.master.balancer.LoadBalancerFactory;
+import org.apache.hadoop.hbase.master.handler.EnableTableHandler;
import org.apache.hadoop.hbase.master.handler.ServerShutdownHandler;
import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
import org.apache.hadoop.hbase.protobuf.generated.ClientProtos.GetRequest;
@@ -78,6 +79,7 @@ import org.junit.BeforeClass;
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.RpcController;
import com.google.protobuf.ServiceException;
@@ -96,6 +98,8 @@ public class TestAssignmentManager {
private static final HRegionInfo REGIONINFO =
new HRegionInfo(Bytes.toBytes("t"),
HConstants.EMPTY_START_ROW, HConstants.EMPTY_START_ROW);
+ private static int assignmentCount;
+ private static boolean enabling = false;
// Mocked objects or; get redone for each test.
private Server server;
@@ -399,11 +403,9 @@ public class TestAssignmentManager {
// We need a mocked catalog tracker.
CatalogTracker ct = Mockito.mock(CatalogTracker.class);
- LoadBalancer balancer = LoadBalancerFactory.getLoadBalancer(server
- .getConfiguration());
// Create an AM.
- AssignmentManager am = new AssignmentManager(this.server,
- this.serverManager, ct, balancer, executor, null);
+ AssignmentManagerWithExtrasForTesting am = setUpMockedAssignmentManager(
+ this.server, this.serverManager);
try {
processServerShutdownHandler(ct, am, false);
} finally {
@@ -855,6 +857,42 @@ public class TestAssignmentManager {
}
/**
+ * 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;
+ List<ServerName> destServers = new ArrayList<ServerName>(1);
+ destServers.add(SERVERNAME_A);
+ Mockito.when(this.serverManager.createDestinationServersList()).thenReturn(destServers);
+ 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);
+ AssignmentManagerWithExtrasForTesting am = setUpMockedAssignmentManager(server,
+ this.serverManager);
+ try {
+ // set table in enabling state.
+ am.getZKTable().setEnablingTable(REGIONINFO.getTableNameAsString());
+ new EnableTableHandler(server, REGIONINFO.getTableName(), am.getCatalogTracker(), am, true)
+ .process();
+ assertEquals("Number of assignments should be 1.", 1, assignmentCount);
+ assertTrue("Table should be enabled.",
+ am.getZKTable().isEnabledTable(REGIONINFO.getTableNameAsString()));
+ } finally {
+ enabling = false;
+ assignmentCount = 0;
+ am.getZKTable().setEnabledTable(REGIONINFO.getTableNameAsString());
+ am.shutdown();
+ ZKAssign.deleteAllNodes(this.watcher);
+ }
+ }
+
+ /**
* Creates a new ephemeral node in the SPLITTING state for the specified region.
* Create it ephemeral in case regionserver dies mid-split.
*
@@ -928,9 +966,15 @@ public class TestAssignmentManager {
ScanResponse.Builder builder = ScanResponse.newBuilder();
builder.setMoreResults(true);
builder.addResult(ProtobufUtil.toResult(r));
- Mockito.when(ri.scan(
- (RpcController)Mockito.any(), (ScanRequest)Mockito.any())).
- thenReturn(builder.build());
+ if (enabling) {
+ Mockito.when(ri.scan((RpcController) Mockito.any(), (ScanRequest) Mockito.any()))
+ .thenReturn(builder.build()).thenReturn(builder.build()).thenReturn(builder.build())
+ .thenReturn(builder.build()).thenReturn(builder.build())
+ .thenReturn(ScanResponse.newBuilder().setMoreResults(false).build());
+ } else {
+ Mockito.when(ri.scan((RpcController) Mockito.any(), (ScanRequest) Mockito.any())).thenReturn(
+ builder.build());
+ }
// If a get, return the above result too for REGIONINFO
GetResponse.Builder getBuilder = GetResponse.newBuilder();
getBuilder.setResult(ProtobufUtil.toResult(r));
@@ -984,8 +1028,13 @@ public class TestAssignmentManager {
@Override
public void assign(HRegionInfo region, boolean setOfflineInZK, boolean forceNewPlan,
boolean hijack) {
- super.assign(region, setOfflineInZK, forceNewPlan, hijack);
- this.gate.set(true);
+ if (enabling) {
+ assignmentCount++;
+ this.regionOnline(region, SERVERNAME_A);
+ } else {
+ super.assign(region, setOfflineInZK, forceNewPlan, hijack);
+ this.gate.set(true);
+ }
}
@Override