Return-Path: 
 <commits-return-19942-apmail-hbase-commits-archive=hbase.apache.org@hbase.apache.org>
X-Original-To: apmail-hbase-commits-archive@www.apache.org
Delivered-To: apmail-hbase-commits-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 3630D1131D
	for <apmail-hbase-commits-archive@www.apache.org>;
 Wed, 17 Sep 2014 18:21:05 +0000 (UTC)
Received: (qmail 28698 invoked by uid 500); 17 Sep 2014 18:21:05 -0000
Delivered-To: apmail-hbase-commits-archive@hbase.apache.org
Received: (qmail 28653 invoked by uid 500); 17 Sep 2014 18:21:05 -0000
Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@hbase.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@hbase.apache.org>
List-Post: <mailto:commits@hbase.apache.org>
List-Id: <commits.hbase.apache.org>
Reply-To: dev@hbase.apache.org
Delivered-To: mailing list commits@hbase.apache.org
Received: (qmail 28644 invoked by uid 99); 17 Sep 2014 18:21:05 -0000
Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org)
 (140.211.11.114)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 17 Sep 2014 18:21:05 +0000
Received: by tyr.zones.apache.org (Postfix, from userid 65534)
	id BE267A18FB1; Wed, 17 Sep 2014 18:21:04 +0000 (UTC)
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: jxiang@apache.org
To: commits@hbase.apache.org
Message-Id: <36bae92cb0c0460b89cd5c99e9eedb54@git.apache.org>
X-Mailer: ASF-Git Admin Mailer
Subject: git commit: HBASE-11991 Region states may be out of sync
Date: Wed, 17 Sep 2014 18:21:04 +0000 (UTC)

Repository: hbase
Updated Branches:
  refs/heads/branch-1 c4111ed97 -> 88e7da321


HBASE-11991 Region states may be out of sync


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/88e7da32
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/88e7da32
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/88e7da32

Branch: refs/heads/branch-1
Commit: 88e7da321c08f6bbc281f9245bdd892c3d4e20b3
Parents: c4111ed
Author: Jimmy Xiang <jxiang@cloudera.com>
Authored: Tue Sep 16 10:20:17 2014 -0700
Committer: Jimmy Xiang <jxiang@cloudera.com>
Committed: Wed Sep 17 11:20:36 2014 -0700

----------------------------------------------------------------------
 .../hadoop/hbase/master/RegionStates.java       | 80 +++++++++++++-------
 1 file changed, 52 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/88e7da32/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
index ee93c0b..9ef4cdf 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
@@ -67,30 +67,35 @@ public class RegionStates {
   /**
    * Regions currently in transition.
    */
-  final HashMap<String, RegionState> regionsInTransition;
+  final HashMap<String, RegionState> regionsInTransition =
+    new HashMap<String, RegionState>();
 
   /**
    * Region encoded name to state map.
    * All the regions should be in this map.
    */
-  private final Map<String, RegionState> regionStates;
+  private final Map<String, RegionState> regionStates =
+    new HashMap<String, RegionState>();
 
   /**
    * Server to regions assignment map.
    * Contains the set of regions currently assigned to a given server.
    */
-  private final Map<ServerName, Set<HRegionInfo>> serverHoldings;
+  private final Map<ServerName, Set<HRegionInfo>> serverHoldings =
+    new HashMap<ServerName, Set<HRegionInfo>>();
 
   /**
    * Maintains the mapping from the default region to the replica regions.
    */
-  private final Map<HRegionInfo, Set<HRegionInfo>> defaultReplicaToOtherReplicas;
+  private final Map<HRegionInfo, Set<HRegionInfo>> defaultReplicaToOtherReplicas =
+    new HashMap<HRegionInfo, Set<HRegionInfo>>();
 
   /**
    * Region to server assignment map.
    * Contains the server a given region is currently assigned to.
    */
-  private final TreeMap<HRegionInfo, ServerName> regionAssignments;
+  private final TreeMap<HRegionInfo, ServerName> regionAssignments =
+    new TreeMap<HRegionInfo, ServerName>();
 
   /**
    * Encoded region name to server assignment map for re-assignment
@@ -102,14 +107,28 @@ public class RegionStates {
    * is offline while the info in lastAssignments is cleared when
    * the region is closed or the server is dead and processed.
    */
-  private final HashMap<String, ServerName> lastAssignments;
+  private final HashMap<String, ServerName> lastAssignments =
+    new HashMap<String, ServerName>();
+
+  /**
+   * Encoded region name to server assignment map for the
+   * purpose to clean up serverHoldings when a region is online
+   * on a new server. When the region is offline from the previous
+   * server, we cleaned up regionAssignments so that it has the
+   * latest assignment map. But we didn't clean up serverHoldings
+   * to match the meta. We need this map to find out the old server
+   * whose serverHoldings needs cleanup, given a moved region.
+   */
+  private final HashMap<String, ServerName> oldAssignments =
+    new HashMap<String, ServerName>();
 
   /**
    * Map a host port pair string to the latest start code
    * of a region server which is known to be dead. It is dead
    * to us, but server manager may not know it yet.
    */
-  private final HashMap<String, Long> deadServers;
+  private final HashMap<String, Long> deadServers =
+    new HashMap<String, Long>();
 
   /**
    * Map a dead servers to the time when log split is done.
@@ -118,7 +137,8 @@ public class RegionStates {
    * on a configured time. By default, we assume a dead
    * server should be done with log splitting in two hours.
    */
-  private final HashMap<ServerName, Long> processedServers;
+  private final HashMap<ServerName, Long> processedServers =
+    new HashMap<ServerName, Long>();
   private long lastProcessedServerCleanTime;
 
   private final TableStateManager tableStateManager;
@@ -132,14 +152,6 @@ public class RegionStates {
 
   RegionStates(final Server master, final TableStateManager tableStateManager,
       final ServerManager serverManager, final RegionStateStore regionStateStore) {
-    regionStates = new HashMap<String, RegionState>();
-    regionsInTransition = new HashMap<String, RegionState>();
-    serverHoldings = new HashMap<ServerName, Set<HRegionInfo>>();
-    defaultReplicaToOtherReplicas = new HashMap<HRegionInfo, Set<HRegionInfo>>();
-    regionAssignments = new TreeMap<HRegionInfo, ServerName>();
-    lastAssignments = new HashMap<String, ServerName>();
-    processedServers = new HashMap<ServerName, Long>();
-    deadServers = new HashMap<String, Long>();
     this.tableStateManager = tableStateManager;
     this.regionStateStore = regionStateStore;
     this.serverManager = serverManager;
@@ -334,6 +346,9 @@ public class RegionStates {
       }
       if (lastHost != null && newState != State.SPLIT) {
         addToServerHoldings(lastHost, hri);
+        if (newState != State.OPEN) {
+          oldAssignments.put(encodedName, lastHost);
+        }
       }
     }
     return regionState;
@@ -391,24 +406,28 @@ public class RegionStates {
    */
   public void regionOnline(final HRegionInfo hri,
       final ServerName serverName, long openSeqNum) {
+    String encodedName = hri.getEncodedName();
     if (!serverManager.isServerOnline(serverName)) {
       // This is possible if the region server dies before master gets a
       // chance to handle ZK event in time. At this time, if the dead server
       // is already processed by SSH, we should ignore this event.
       // If not processed yet, ignore and let SSH deal with it.
-      LOG.warn("Ignored, " + hri.getEncodedName()
+      LOG.warn("Ignored, " + encodedName
         + " was opened on a dead server: " + serverName);
       return;
     }
     updateRegionState(hri, State.OPEN, serverName, openSeqNum);
 
     synchronized (this) {
-      regionsInTransition.remove(hri.getEncodedName());
+      regionsInTransition.remove(encodedName);
       ServerName oldServerName = regionAssignments.put(hri, serverName);
       if (!serverName.equals(oldServerName)) {
         LOG.info("Onlined " + hri.getShortNameToLog() + " on " + serverName);
         addToServerHoldings(serverName, hri);
         addToReplicaMapping(hri);
+        if (oldServerName == null) {
+          oldServerName = oldAssignments.remove(encodedName);
+        }
         if (oldServerName != null && serverHoldings.containsKey(oldServerName)) {
           LOG.info("Offlined " + hri.getShortNameToLog() + " from " + oldServerName);
           removeFromServerHoldings(oldServerName, hri);
@@ -528,19 +547,24 @@ public class RegionStates {
     State newState =
       expectedState == null ? State.OFFLINE : expectedState;
     updateRegionState(hri, newState);
-
+    String encodedName = hri.getEncodedName();
     synchronized (this) {
-      regionsInTransition.remove(hri.getEncodedName());
+      regionsInTransition.remove(encodedName);
       ServerName oldServerName = regionAssignments.remove(hri);
-      if (oldServerName != null && serverHoldings.containsKey(oldServerName)
-          && (newState == State.MERGED || newState == State.SPLIT
+      if (oldServerName != null && serverHoldings.containsKey(oldServerName)) {
+        if (newState == State.MERGED || newState == State.SPLIT
             || hri.isMetaRegion() || tableStateManager.isTableState(hri.getTable(),
-              ZooKeeperProtos.Table.State.DISABLED, ZooKeeperProtos.Table.State.DISABLING))) {
-        // Offline the region only if it's merged/split, or the table is disabled/disabling.
-        // Otherwise, offline it from this server only when it is online on a different server.
-        LOG.info("Offlined " + hri.getShortNameToLog() + " from " + oldServerName);
-        removeFromServerHoldings(oldServerName, hri);
-        removeFromReplicaMapping(hri);
+              ZooKeeperProtos.Table.State.DISABLED, ZooKeeperProtos.Table.State.DISABLING)) {
+          // Offline the region only if it's merged/split, or the table is disabled/disabling.
+          // Otherwise, offline it from this server only when it is online on a different server.
+          LOG.info("Offlined " + hri.getShortNameToLog() + " from " + oldServerName);
+          removeFromServerHoldings(oldServerName, hri);
+          removeFromReplicaMapping(hri);
+        } else {
+          // Need to remember it so that we can offline it from this
+          // server when it is online on a different server.
+          oldAssignments.put(encodedName, oldServerName);
+        }
       }
     }
   }