You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2012/12/10 21:34:21 UTC

svn commit: r1419736 - in /hbase/branches/0.92/src: main/java/org/apache/hadoop/hbase/master/ main/java/org/apache/hadoop/hbase/master/handler/ test/java/org/apache/hadoop/hbase/master/

Author: tedyu
Date: Mon Dec 10 20:34:21 2012
New Revision: 1419736

URL: http://svn.apache.org/viewvc?rev=1419736&view=rev
Log:
HBASE-6317 Master clean start up and Partially enabled tables make region assignment inconsistent (Rajesh)


Modified:
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java
    hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java?rev=1419736&r1=1419735&r2=1419736&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Mon Dec 10 20:34:21 2012
@@ -136,8 +136,10 @@ public class AssignmentManager extends Z
 
   // store all the table names in disabling state
   Set<String> disablingTables = new HashSet<String>(1);
-  // store all the enabling state tablenames.
-  Set<String> enablingTables = new HashSet<String>(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<String, List<HRegionInfo>> enablingTables = new HashMap<String, List<HRegionInfo>>(1);
 
   /**
    * Server to regions assignment map.
@@ -257,6 +259,16 @@ public class AssignmentManager extends Z
   }
 
   /**
+   * Gives enabling table regions.
+   * 
+   * @param tableName
+   * @return list of regionInfos
+   */
+  public List<HRegionInfo> getEnablingTableRegions(String tableName){
+    return this.enablingTables.get(tableName);
+  }
+
+  /**
    * Add a regionPlan for the specified region.
    * @param encodedName 
    * @param plan 
@@ -348,7 +360,9 @@ public class AssignmentManager extends Z
     // 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();
   }
 
   /**
@@ -485,6 +499,10 @@ public class AssignmentManager extends Z
     String encodedRegionName = regionInfo.getEncodedName();
     LOG.info("Processing region " + regionInfo.getRegionNameAsString() +
       " in state " + data.getEventType());
+    List<HRegionInfo> hris = this.enablingTables.get(regionInfo.getTableNameAsString());
+    if (hris != null && !hris.isEmpty()) {
+      hris.remove(regionInfo);
+    }
     synchronized (regionsInTransition) {
       RegionState regionState = regionsInTransition.get(encodedRegionName);
       if (regionState != null ||
@@ -2181,11 +2199,12 @@ public class AssignmentManager extends Z
     // Skip assignment for regions of tables in DISABLING state 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<String> disablingAndDisabledTables = new HashSet<String>(this.disablingTables);
-    disablingAndDisabledTables.addAll(this.zkTable.getDisabledTables());
+    Set<String> disablingDisabledAndEnablingTables = new HashSet<String>(this.disablingTables);
+    disablingDisabledAndEnablingTables.addAll(this.zkTable.getDisabledTables());
+    disablingDisabledAndEnablingTables.addAll(this.enablingTables.keySet());
     // Scan META for all user regions, skipping any disabled tables
     Map<HRegionInfo, ServerName> allRegions = MetaReader.fullScan(catalogTracker,
-        disablingAndDisabledTables, true);
+        disablingDisabledAndEnablingTables, true);
     if (allRegions == null || allRegions.isEmpty()) return;
 
     // Determine what type of assignment to do on startup
@@ -2415,12 +2434,14 @@ public class AssignmentManager extends Z
         // 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(regionInfo, tableName);
       } else if (!onlineServers.contains(regionLocation)) {
         // Region is located on a server that isn't online
         List<Pair<HRegionInfo, Result>> offlineRegions =
@@ -2430,7 +2451,7 @@ public class AssignmentManager extends Z
           offlineServers.put(regionLocation, offlineRegions);
         }
         offlineRegions.add(new Pair<HRegionInfo,Result>(regionInfo, result));
-        addTheTablesInPartialState(regionInfo, tableName);
+        addTheTablesInPartialState(regionInfo);
       } else {
         // If region is in offline and split state check the ZKNode
         if (regionInfo.isOffline() && regionInfo.isSplit()) {
@@ -2448,27 +2469,45 @@ public class AssignmentManager extends Z
         }
         // 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);
+        if (!enabling && !checkIfRegionBelongsToDisabled(regionInfo)) {
           synchronized (this.regions) {
             regions.put(regionInfo, regionLocation);
             addToServers(regionLocation, regionInfo);            
           }
         }
-        addTheTablesInPartialState(regionInfo, tableName);
+        addTheTablesInPartialState(regionInfo);
+        if (enabling) {
+          addToEnablingTableRegions(regionInfo);
+        }
       }
     }
     return offlineServers;
   }
 
-  private void addTheTablesInPartialState(HRegionInfo regionInfo, String disablingTableName) {
+  private void addTheTablesInPartialState(HRegionInfo regionInfo) {
+    String tableName = regionInfo.getTableNameAsString();
     if (checkIfRegionBelongsToDisabling(regionInfo)) {
-      this.disablingTables.add(disablingTableName);
+      this.disablingTables.add(tableName);
     } else if (checkIfRegionsBelongsToEnabling(regionInfo)) {
-      this.enablingTables.add(disablingTableName);
+      if (!this.enablingTables.containsKey(tableName)) {
+        this.enablingTables.put(tableName, new ArrayList<HRegionInfo>());
+      }
     }
   }
 
+  private void addToEnablingTableRegions(HRegionInfo regionInfo) {
+    String tableName = regionInfo.getTableNameAsString();
+    List<HRegionInfo> 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);
+    }
+  }
+  
   /**
    * Recover the tables that were not fully moved to DISABLED state. These
    * tables are in DISABLING state when the master restarted/switched.

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java?rev=1419736&r1=1419735&r2=1419736&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/handler/EnableTableHandler.java Mon Dec 10 20:34:21 2012
@@ -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.Log;
 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.catalog.M
 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 @@ 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,
@@ -56,6 +63,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));
@@ -99,10 +107,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.
 
@@ -111,10 +121,10 @@ 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;
@@ -140,17 +150,34 @@ 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.
+   * @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.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>();
+    List<HRegionInfo> enablingTableRegions = this.assignmentManager
+        .getEnablingTableRegions(this.tableNameStr);
+    final List<HRegionInfo> onlineRegions = this.assignmentManager.getRegionsOfTable(tableName);
+    for (Pair<HRegionInfo, ServerName> 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;
   }
 
   /**

Modified: hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java?rev=1419736&r1=1419735&r2=1419736&view=diff
==============================================================================
--- hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java (original)
+++ hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java Mon Dec 10 20:34:21 2012
@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -40,6 +41,7 @@ import org.apache.hadoop.hbase.Server;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.ZooKeeperConnectionException;
 import org.apache.hadoop.hbase.catalog.CatalogTracker;
+import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.HConnection;
 import org.apache.hadoop.hbase.client.HConnectionManager;
 import org.apache.hadoop.hbase.client.HConnectionTestingUtility;
@@ -63,6 +65,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockito.Mockito;
+import org.mockito.internal.util.reflection.Whitebox;
 
 
 /**
@@ -78,6 +81,10 @@ public class TestAssignmentManager {
   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;  
   private static final Abortable ABORTABLE = new Abortable() {
     boolean aborted = false;
     @Override
@@ -204,6 +211,53 @@ 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;
+    this.server.getConfiguration().setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS,
+        DefaultLoadBalancer.class, LoadBalancer.class);
+    Map<ServerName, HServerLoad> serverAndLoad = new HashMap<ServerName, HServerLoad>();
+    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;
+    }
+  }
+
+  /**
    * @param sn ServerName to use making startcode and server in meta
    * @param hri Region to serialize into HRegionInfo
    * @return A mocked up Result that fakes a Get on a row in the
@@ -267,20 +321,33 @@ public class TestAssignmentManager {
     // rebuild its list of user regions and it will also get the HRI that goes
     // with an encoded name by doing a Get on .META.
     HRegionInterface ri = Mockito.mock(HRegionInterface.class);
+    Result[] result = null;
+    if (enabling) {
+      result = new Result[2];
+      result[0] = getMetaTableRowResult(REGIONINFO, SERVERNAME_A);
+      result[1] = getMetaTableRowResult(REGIONINFO_2, SERVERNAME_A);
+    }
     // Get a meta row result that has region up on SERVERNAME_A for REGIONINFO
     Result r = getMetaTableRowResult(REGIONINFO, SERVERNAME_A);
-
     final long scannerid = 123L;
     Mockito.when(ri.openScanner((byte[]) Mockito.any(), (Scan) Mockito.any()))
         .thenReturn(scannerid);
-    // Make it so a verifiable answer comes back when next is called. Return
-    // the verifiable answer and then a null so we stop scanning. Our
-    // verifiable answer is something that looks like a row in META with
-    // a server and startcode that is that of the above defined servername.
-    Mockito.when(ri.next(Mockito.anyLong(), Mockito.anyInt()))
-        .thenReturn(new Result[] { r }, (Result[]) null)
-        .thenReturn(new Result[] { r }, (Result[]) null)
-        .thenReturn(new Result[] { r }, (Result[]) null);
+    if (enabling) {
+      Mockito.when(ri.next(Mockito.anyLong(), Mockito.anyInt()))
+          .thenReturn(result, (Result[]) null).thenReturn(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), (Result[]) null);
+    } else {
+      // Make it so a verifiable answer comes back when next is called. Return
+      // the verifiable answer and then a null so we stop scanning. Our
+      // verifiable answer is something that looks like a row in META with
+      // a server and startcode that is that of the above defined servername.
+      Mockito.when(ri.next(Mockito.anyLong(), Mockito.anyInt()))
+          .thenReturn(new Result[] { r }, (Result[]) null)
+          .thenReturn(new Result[] { r }, (Result[]) null)
+          .thenReturn(new Result[] { r }, (Result[]) null);
+    }
 
     // Associate a spied-upon HConnection with UTIL.getConfiguration. Need
     // to shove this in here first so it gets picked up all over; e.g. by
@@ -387,8 +454,13 @@ public class TestAssignmentManager {
     @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