From f5881a311612d6ad1c31fa682aa13b6156b80dad Mon Sep 17 00:00:00 2001 From: jxiang Date: Fri, 25 Oct 2013 16:17:29 +0000 Subject: [PATCH] HBASE-9748 Address outstanding comments raised for HBASE-9696 git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1535771 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop/hbase/master/RegionState.java | 26 ++- .../hadoop/hbase/zookeeper/ZKAssign.java | 66 +++++-- .../test/IntegrationTestBigLinkedList.java | 4 +- .../hbase/master/AssignmentManager.java | 161 ++++++------------ .../hadoop/hbase/master/BulkReOpen.java | 38 ++++- .../apache/hadoop/hbase/master/HMaster.java | 14 +- .../hadoop/hbase/master/RegionStates.java | 82 ++++----- .../master/handler/DeleteTableHandler.java | 1 - .../master/handler/ServerShutdownHandler.java | 2 +- .../regionserver/RegionMergeTransaction.java | 8 +- .../hbase/regionserver/SplitTransaction.java | 8 +- .../hbase/master/TestAssignmentManager.java | 2 +- .../hbase/master/TestMasterFailover.java | 4 +- .../TestRegionServerNoMaster.java | 9 +- .../TestSplitTransactionOnCluster.java | 6 +- .../handler/TestCloseRegionHandler.java | 5 +- 16 files changed, 232 insertions(+), 204 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java index 3c1fb092dc0..11703878228 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java @@ -202,24 +202,34 @@ public class RegionState implements org.apache.hadoop.io.Writable { return serverName != null && serverName.equals(sn); } - // Is a region in a state ready to go offline + /** + * Check if a region state can transition to offline + */ public boolean isReadyToOffline() { return isMerged() || isSplit() || isOffline() || isSplittingNew() || isMergingNew(); } - // Is a region in a state ready to go online + /** + * Check if a region state can transition to online + */ public boolean isReadyToOnline() { return isOpened() || isSplittingNew() || isMergingNew(); } - // Is a region in a state not in transition but not unassignable - public boolean isNotUnassignableNotInTransition() { - return isNotUnassignableNotInTransition(state); + /** + * Check if a region state is one of offline states that + * can't transition to pending_close/closing (unassign/offline) + */ + public boolean isUnassignable() { + return isUnassignable(state); } - // Check if a state is not in transition, but not unassignable - public static boolean isNotUnassignableNotInTransition(State state) { + /** + * Check if a region state is one of offline states that + * can't transition to pending_close/closing (unassign/offline) + */ + public static boolean isUnassignable(State state) { return state == State.MERGED || state == State.SPLIT || state == State.OFFLINE || state == State.SPLITTING_NEW || state == State.MERGING_NEW; } @@ -233,7 +243,7 @@ public class RegionState implements org.apache.hadoop.io.Writable { } /** - * A slower (but more easy-to-read) stringification + * A slower (but more easy-to-read) stringification */ public String toDescriptiveString() { long lstamp = stamp.get(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java index d022088772d..07d58795293 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java @@ -259,13 +259,15 @@ public class ZKAssign { * * @param zkw zk reference * @param encodedRegionName opened region to be deleted from zk + * @param sn the expected region transition target server name * @throws KeeperException if unexpected zookeeper exception * @throws KeeperException.NoNodeException if node does not exist */ public static boolean deleteOpenedNode(ZooKeeperWatcher zkw, - String encodedRegionName) + String encodedRegionName, ServerName sn) throws KeeperException, KeeperException.NoNodeException { - return deleteNode(zkw, encodedRegionName, EventType.RS_ZK_REGION_OPENED); + return deleteNode(zkw, encodedRegionName, + EventType.RS_ZK_REGION_OPENED, sn); } /** @@ -284,13 +286,15 @@ public class ZKAssign { * * @param zkw zk reference * @param encodedRegionName closed region to be deleted from zk + * @param sn the expected region transition target server name * @throws KeeperException if unexpected zookeeper exception * @throws KeeperException.NoNodeException if node does not exist */ public static boolean deleteOfflineNode(ZooKeeperWatcher zkw, - String encodedRegionName) + String encodedRegionName, ServerName sn) throws KeeperException, KeeperException.NoNodeException { - return deleteNode(zkw, encodedRegionName, EventType.M_ZK_REGION_OFFLINE); + return deleteNode(zkw, encodedRegionName, + EventType.M_ZK_REGION_OFFLINE, sn); } /** @@ -310,13 +314,15 @@ public class ZKAssign { * * @param zkw zk reference * @param encodedRegionName closed region to be deleted from zk + * @param sn the expected region transition target server name * @throws KeeperException if unexpected zookeeper exception * @throws KeeperException.NoNodeException if node does not exist */ public static boolean deleteClosedNode(ZooKeeperWatcher zkw, - String encodedRegionName) + String encodedRegionName, ServerName sn) throws KeeperException, KeeperException.NoNodeException { - return deleteNode(zkw, encodedRegionName, EventType.RS_ZK_REGION_CLOSED); + return deleteNode(zkw, encodedRegionName, + EventType.RS_ZK_REGION_CLOSED, sn); } /** @@ -336,14 +342,16 @@ public class ZKAssign { * * @param zkw zk reference * @param region closing region to be deleted from zk + * @param sn the expected region transition target server name * @throws KeeperException if unexpected zookeeper exception * @throws KeeperException.NoNodeException if node does not exist */ public static boolean deleteClosingNode(ZooKeeperWatcher zkw, - HRegionInfo region) + HRegionInfo region, ServerName sn) throws KeeperException, KeeperException.NoNodeException { String encodedRegionName = region.getEncodedName(); - return deleteNode(zkw, encodedRegionName, EventType.M_ZK_REGION_CLOSING); + return deleteNode(zkw, encodedRegionName, + EventType.M_ZK_REGION_CLOSING, sn); } /** @@ -364,13 +372,14 @@ public class ZKAssign { * @param zkw zk reference * @param encodedRegionName region to be deleted from zk * @param expectedState state region must be in for delete to complete + * @param sn the expected region transition target server name * @throws KeeperException if unexpected zookeeper exception * @throws KeeperException.NoNodeException if node does not exist */ public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName, - EventType expectedState) + EventType expectedState, ServerName sn) throws KeeperException, KeeperException.NoNodeException { - return deleteNode(zkw, encodedRegionName, expectedState, -1); + return deleteNode(zkw, encodedRegionName, expectedState, sn, -1); } /** @@ -399,6 +408,37 @@ public class ZKAssign { */ public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName, EventType expectedState, int expectedVersion) + throws KeeperException, KeeperException.NoNodeException { + return deleteNode(zkw, encodedRegionName, expectedState, null, expectedVersion); + } + + /** + * Deletes an existing unassigned node that is in the specified state for the + * specified region. + * + *

If a node does not already exist for this region, a + * {@link NoNodeException} will be thrown. + * + *

No watcher is set whether this succeeds or not. + * + *

Returns false if the node was not in the proper state but did exist. + * + *

This method is used when a region finishes opening/closing. + * The Master acknowledges completion + * of the specified regions transition to being closed/opened. + * + * @param zkw zk reference + * @param encodedRegionName region to be deleted from zk + * @param expectedState state region must be in for delete to complete + * @param sn the expected region transition target server name + * @param expectedVersion of the znode that is to be deleted. + * If expectedVersion need not be compared while deleting the znode + * pass -1 + * @throws KeeperException if unexpected zookeeper exception + * @throws KeeperException.NoNodeException if node does not exist + */ + public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName, + EventType expectedState, ServerName serverName, int expectedVersion) throws KeeperException, KeeperException.NoNodeException { if (LOG.isTraceEnabled()) { LOG.trace(zkw.prefix("Deleting existing unassigned " + @@ -419,6 +459,12 @@ public class ZKAssign { expectedState + " state but node is in " + et + " state")); return false; } + // Verify the server transition happens on is not changed + if (serverName != null && !rt.getServerName().equals(serverName)) { + LOG.warn(zkw.prefix("Attempting to delete unassigned node " + encodedRegionName + + " with target " + serverName + " but node has " + rt.getServerName())); + return false; + } if (expectedVersion != -1 && stat.getVersion() != expectedVersion) { LOG.warn("The node " + encodedRegionName + " we are trying to delete is not" + diff --git a/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestBigLinkedList.java b/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestBigLinkedList.java index 47f2fc9639c..3d62060b2b8 100644 --- a/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestBigLinkedList.java +++ b/hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestBigLinkedList.java @@ -715,7 +715,7 @@ public class IntegrationTestBigLinkedList extends IntegrationTestBase { String keyString = it.next().getName(); byte[] key = Bytes.toBytes(keyString); HRegionLocation loc = conn.relocateRegion(tableName, key); - LOG.error("undefined row " + keyString + " region " + loc); + LOG.error("undefined row " + keyString + ", " + loc); } g = counters.getGroup("unref"); it = g.iterator(); @@ -723,7 +723,7 @@ public class IntegrationTestBigLinkedList extends IntegrationTestBase { String keyString = it.next().getName(); byte[] key = Bytes.toBytes(keyString); HRegionLocation loc = conn.relocateRegion(tableName, key); - LOG.error("unreferred row " + keyString + " region " + loc); + LOG.error("unreferred row " + keyString + ", " + loc); } } return success; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 2a0e89bb1a5..54d47982b55 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -566,10 +566,11 @@ public class AssignmentManager extends ZooKeeperListener { } HRegionInfo hri = regionInfo; if (hri == null) { - // Get the region from region states map/meta. However, we - // may still can't get it, for example, for online region merge, - // the znode uses the new region to be created, which may not in meta - // yet if the merging is still going on during the master recovery. + // The region info is not passed in. We will try to find the region + // from region states map/meta based on the encoded region name. But we + // may not be able to find it. This is valid for online merge that + // the region may have not been created if the merge is not completed. + // Therefore, it is not in meta at master recovery time. hri = regionStates.getRegionInfo(rt.getRegionName()); EventType et = rt.getEventType(); if (hri == null && et != EventType.RS_ZK_REGION_MERGING @@ -601,9 +602,12 @@ public class AssignmentManager extends ZooKeeperListener { final byte[] regionName = rt.getRegionName(); final String encodedName = HRegionInfo.encodeRegionName(regionName); final String prettyPrintedRegionName = HRegionInfo.prettyPrint(encodedName); - LOG.info("Processing " + prettyPrintedRegionName + " in state " + et); + LOG.info("Processing " + prettyPrintedRegionName + " in state: " + et); if (regionStates.isRegionInTransition(encodedName)) { + LOG.info("Processed region " + prettyPrintedRegionName + " in state: " + + et + ", does nothing since the region is already in transition " + + regionStates.getRegionTransitionState(encodedName)); // Just return return; } @@ -696,29 +700,29 @@ public class AssignmentManager extends ZooKeeperListener { // Splitting region should be online. We could have skipped it during // user region rebuilding since we may consider the split is completed. // Put it in SPLITTING state to avoid complications. + regionStates.updateRegionState(regionInfo, State.OPEN, sn); regionStates.regionOnline(regionInfo, sn); regionStates.updateRegionState(rt, State.SPLITTING); } if (!handleRegionSplitting( rt, encodedName, prettyPrintedRegionName, sn)) { - deleteSplittingNode(encodedName); + deleteSplittingNode(encodedName, sn); } - LOG.info("Processed region " + prettyPrintedRegionName - + " in state : " + et); break; case RS_ZK_REQUEST_REGION_MERGE: case RS_ZK_REGION_MERGING: case RS_ZK_REGION_MERGED: if (!handleRegionMerging( rt, encodedName, prettyPrintedRegionName, sn)) { - deleteMergingNode(encodedName); + deleteMergingNode(encodedName, sn); } - LOG.info("Processed region " + prettyPrintedRegionName - + " in state : " + et); break; default: - throw new IllegalStateException("Received region in state :" + et + " is not valid."); + throw new IllegalStateException("Received region in state:" + et + " is not valid."); } + LOG.info("Processed region " + prettyPrintedRegionName + " in state " + + et + ", on " + (serverManager.isServerOnline(sn) ? "" : "dead ") + + "server: " + sn); } /** @@ -836,7 +840,7 @@ public class AssignmentManager extends ZooKeeperListener { case RS_ZK_REGION_SPLIT: if (!handleRegionSplitting( rt, encodedName, prettyPrintedRegionName, sn)) { - deleteSplittingNode(encodedName); + deleteSplittingNode(encodedName, sn); } break; @@ -847,7 +851,7 @@ public class AssignmentManager extends ZooKeeperListener { // However, the two merging regions are not new. They should be in state for merging. if (!handleRegionMerging( rt, encodedName, prettyPrintedRegionName, sn)) { - deleteMergingNode(encodedName); + deleteMergingNode(encodedName, sn); } break; @@ -1386,19 +1390,11 @@ public class AssignmentManager extends ZooKeeperListener { public void offlineDisabledRegion(HRegionInfo regionInfo) { // Disabling so should not be reassigned, just delete the CLOSED node LOG.debug("Table being disabled so deleting ZK node and removing from " + - "regions in transition, skipping assignment of region " + - regionInfo.getRegionNameAsString()); - try { - if (!ZKAssign.deleteClosedNode(watcher, regionInfo.getEncodedName())) { - // Could also be in OFFLINE mode - ZKAssign.deleteOfflineNode(watcher, regionInfo.getEncodedName()); - } - } catch (KeeperException.NoNodeException nne) { - LOG.debug("Tried to delete closed node for " + regionInfo + " but it " + - "does not exist so just offlining"); - } catch (KeeperException e) { - this.server.abort("Error deleting CLOSED node in ZK", e); - } + "regions in transition, skipping assignment of region " + + regionInfo.getRegionNameAsString()); + String encodedName = regionInfo.getEncodedName(); + deleteNodeInStates(encodedName, "closed", null, + EventType.RS_ZK_REGION_CLOSED, EventType.M_ZK_REGION_OFFLINE); regionOffline(regionInfo); } @@ -1678,9 +1674,11 @@ public class AssignmentManager extends ZooKeeperListener { } // ClosedRegionhandler can remove the server from this.regions if (!serverManager.isServerOnline(server)) { + LOG.debug("Offline " + region.getRegionNameAsString() + + ", no need to unassign since it's on a dead server: " + server); if (transitionInZK) { // delete the node. if no node exists need not bother. - deleteClosingOrClosedNode(region); + deleteClosingOrClosedNode(region, server); } if (state != null) { regionOffline(region); @@ -1711,8 +1709,10 @@ public class AssignmentManager extends ZooKeeperListener { } if (t instanceof NotServingRegionException || t instanceof RegionServerStoppedException) { + LOG.debug("Offline " + region.getRegionNameAsString() + + ", it's not any more on " + server, t); if (transitionInZK) { - deleteClosingOrClosedNode(region); + deleteClosingOrClosedNode(region, server); } if (state != null) { regionOffline(region); @@ -2075,21 +2075,9 @@ public class AssignmentManager extends ZooKeeperListener { // While trying to enable the table the regions of the table were // already enabled. LOG.debug("ALREADY_OPENED " + region.getRegionNameAsString() - + " to " + sn); - String encodedRegionName = region.getEncodedName(); - try { - ZKAssign.deleteOfflineNode(watcher, encodedRegionName); - } catch (KeeperException.NoNodeException e) { - if (LOG.isDebugEnabled()) { - LOG.debug("The unassigned node " + encodedRegionName - + " does not exist."); - } - } catch (KeeperException e) { - server.abort( - "Error deleting OFFLINED node in ZK for transition ZK node (" - + encodedRegionName + ")", e); - } - + + " to " + sn); + String encodedName = region.getEncodedName(); + deleteNodeInStates(encodedName, "offline", sn, EventType.M_ZK_REGION_OFFLINE); regionStates.regionOnline(region, sn); } @@ -2220,37 +2208,6 @@ public class AssignmentManager extends ZooKeeperListener { return existingPlan; } - /** - * Unassign the list of regions. Configuration knobs: - * hbase.bulk.waitbetween.reopen indicates the number of milliseconds to - * wait before unassigning another region from this region server - * - * @param regions - * @throws InterruptedException - */ - public void unassign(List regions) { - int waitTime = this.server.getConfiguration().getInt( - "hbase.bulk.waitbetween.reopen", 0); - for (HRegionInfo region : regions) { - if (regionStates.isRegionInTransition(region)) - continue; - unassign(region, false); - while (regionStates.isRegionInTransition(region)) { - try { - Thread.sleep(10); - } catch (InterruptedException e) { - // Do nothing, continue - } - } - if (waitTime > 0) - try { - Thread.sleep(waitTime); - } catch (InterruptedException e) { - // Do nothing, continue - } - } - } - /** * Unassigns the specified region. *

@@ -2301,7 +2258,7 @@ public class AssignmentManager extends ZooKeeperListener { // Region is not in transition. // We can unassign it only if it's not SPLIT/MERGED. state = regionStates.getRegionState(encodedName); - if (state != null && state.isNotUnassignableNotInTransition()) { + if (state != null && state.isUnassignable()) { LOG.info("Attempting to unassign " + state + ", ignored"); // Offline region will be reassigned below return; @@ -2394,9 +2351,9 @@ public class AssignmentManager extends ZooKeeperListener { /** * @param region regioninfo of znode to be deleted. */ - public void deleteClosingOrClosedNode(HRegionInfo region) { - String regionName = region.getEncodedName(); - deleteNodeInStates(regionName, "closing", EventType.M_ZK_REGION_CLOSING, + public void deleteClosingOrClosedNode(HRegionInfo region, ServerName sn) { + String encodedName = region.getEncodedName(); + deleteNodeInStates(encodedName, "closing", sn, EventType.M_ZK_REGION_CLOSING, EventType.RS_ZK_REGION_CLOSED); } @@ -3175,7 +3132,6 @@ public class AssignmentManager extends ZooKeeperListener { server.abort("Unexpected ZK exception deleting node " + hri, ke); } if (zkTable.isDisablingOrDisabledTable(hri.getTable())) { - regionStates.updateRegionState(hri, State.OFFLINE); regionStates.regionOffline(hri); it.remove(); continue; @@ -3266,32 +3222,34 @@ public class AssignmentManager extends ZooKeeperListener { return true; } - private boolean deleteNodeInStates( - String regionName, String desc, EventType... types) { + private boolean deleteNodeInStates(String encodedName, + String desc, ServerName sn, EventType... types) { try { for (EventType et: types) { - if (ZKAssign.deleteNode(watcher, regionName, et)) { + if (ZKAssign.deleteNode(watcher, encodedName, et, sn)) { return true; } } LOG.info("Failed to delete the " + desc + " node for " - + regionName + ". The node type may not match"); + + encodedName + ". The node type may not match"); } catch (NoNodeException e) { - LOG.debug("The " + desc + " node for " + regionName + " already deleted"); + if (LOG.isDebugEnabled()) { + LOG.debug("The " + desc + " node for " + encodedName + " already deleted"); + } } catch (KeeperException ke) { server.abort("Unexpected ZK exception deleting " + desc - + " node for the region " + regionName, ke); + + " node for the region " + encodedName, ke); } return false; } - private void deleteMergingNode(String encodedName) { - deleteNodeInStates(encodedName, "merging", EventType.RS_ZK_REGION_MERGING, + private void deleteMergingNode(String encodedName, ServerName sn) { + deleteNodeInStates(encodedName, "merging", sn, EventType.RS_ZK_REGION_MERGING, EventType.RS_ZK_REQUEST_REGION_MERGE, EventType.RS_ZK_REGION_MERGED); } - private void deleteSplittingNode(String encodedName) { - deleteNodeInStates(encodedName, "splitting", EventType.RS_ZK_REGION_SPLITTING, + private void deleteSplittingNode(String encodedName, ServerName sn) { + deleteNodeInStates(encodedName, "splitting", sn, EventType.RS_ZK_REGION_SPLITTING, EventType.RS_ZK_REQUEST_REGION_SPLIT, EventType.RS_ZK_REGION_SPLIT); } @@ -3359,21 +3317,16 @@ public class AssignmentManager extends ZooKeeperListener { } synchronized (regionStates) { - if (regionStates.getRegionState(p) == null) { - regionStates.createRegionState(p); - } regionStates.updateRegionState(hri_a, State.MERGING); regionStates.updateRegionState(hri_b, State.MERGING); + regionStates.updateRegionState(p, State.MERGING_NEW, sn); if (et != EventType.RS_ZK_REGION_MERGED) { - regionStates.updateRegionState(p, State.MERGING_NEW, sn);; regionStates.regionOffline(p, State.MERGING_NEW); this.mergingRegions.put(encodedName, new PairOfSameType(hri_a, hri_b)); } else { this.mergingRegions.remove(encodedName); - regionStates.updateRegionState(hri_a, State.MERGED); - regionStates.updateRegionState(hri_b, State.MERGED); regionOffline(hri_a, State.MERGED); regionOffline(hri_b, State.MERGED); regionOnline(p, sn); @@ -3388,8 +3341,8 @@ public class AssignmentManager extends ZooKeeperListener { while (!successful) { // It's possible that the RS tickles in between the reading of the // znode and the deleting, so it's safe to retry. - successful = ZKAssign.deleteNode( - watcher, encodedName, EventType.RS_ZK_REGION_MERGED); + successful = ZKAssign.deleteNode(watcher, encodedName, + EventType.RS_ZK_REGION_MERGED, sn); } } catch (KeeperException e) { if (e instanceof NoNodeException) { @@ -3486,13 +3439,6 @@ public class AssignmentManager extends ZooKeeperListener { } synchronized (regionStates) { - if (regionStates.getRegionState(hri_a) == null) { - regionStates.createRegionState(hri_a); - } - if (regionStates.getRegionState(hri_b) == null) { - regionStates.createRegionState(hri_b); - } - regionStates.updateRegionState(hri_a, State.SPLITTING_NEW, sn); regionStates.updateRegionState(hri_b, State.SPLITTING_NEW, sn); regionStates.regionOffline(hri_a, State.SPLITTING_NEW); @@ -3507,7 +3453,6 @@ public class AssignmentManager extends ZooKeeperListener { } if (et == EventType.RS_ZK_REGION_SPLIT) { - regionStates.updateRegionState(p, State.SPLIT); regionOffline(p, State.SPLIT); regionOnline(hri_a, sn); regionOnline(hri_b, sn); @@ -3522,8 +3467,8 @@ public class AssignmentManager extends ZooKeeperListener { while (!successful) { // It's possible that the RS tickles in between the reading of the // znode and the deleting, so it's safe to retry. - successful = ZKAssign.deleteNode( - watcher, encodedName, EventType.RS_ZK_REGION_SPLIT); + successful = ZKAssign.deleteNode(watcher, encodedName, + EventType.RS_ZK_REGION_SPLIT, sn); } } catch (KeeperException e) { if (e instanceof NoNodeException) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/BulkReOpen.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/BulkReOpen.java index 46a1f0baeaa..218d5e46024 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/BulkReOpen.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/BulkReOpen.java @@ -67,7 +67,12 @@ public class BulkReOpen extends BulkAssigner { assignmentManager.addPlans(plans); pool.execute(new Runnable() { public void run() { - assignmentManager.unassign(hris); + try { + unassign(hris); + } catch (Throwable t) { + LOG.warn("Failed bulking re-open " + hris.size() + + " region(s)", t); + } } }); } @@ -97,4 +102,35 @@ public class BulkReOpen extends BulkAssigner { public boolean bulkReOpen() throws InterruptedException, IOException { return bulkAssign(); } + + /** + * Unassign the list of regions. Configuration knobs: + * hbase.bulk.waitbetween.reopen indicates the number of milliseconds to + * wait before unassigning another region from this region server + * + * @param regions + * @throws InterruptedException + */ + private void unassign( + List regions) throws InterruptedException { + int waitTime = this.server.getConfiguration().getInt( + "hbase.bulk.waitbetween.reopen", 0); + RegionStates regionStates = assignmentManager.getRegionStates(); + for (HRegionInfo region : regions) { + if (server.isStopped()) { + return; + } + if (regionStates.isRegionInTransition(region)) { + continue; + } + assignmentManager.unassign(region, false); + while (regionStates.isRegionInTransition(region) + && !server.isStopped()) { + regionStates.waitForUpdate(100); + } + if (waitTime > 0 && !server.isStopped()) { + Thread.sleep(waitTime); + } + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index fcbdfda173c..089fb796ee7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -87,6 +87,7 @@ import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.ipc.RpcServer.BlockingServiceAndInterface; import org.apache.hadoop.hbase.ipc.RpcServerInterface; import org.apache.hadoop.hbase.ipc.ServerRpcController; +import org.apache.hadoop.hbase.master.RegionState.State; import org.apache.hadoop.hbase.master.balancer.BalancerChore; import org.apache.hadoop.hbase.master.balancer.ClusterStatusChore; import org.apache.hadoop.hbase.master.balancer.LoadBalancerFactory; @@ -990,17 +991,18 @@ MasterServices, Server { status.setStatus("Assigning hbase:meta region"); ServerName logReplayFailedMetaServer = null; - assignmentManager.getRegionStates().createRegionState(HRegionInfo.FIRST_META_REGIONINFO); + RegionStates regionStates = assignmentManager.getRegionStates(); + regionStates.createRegionState(HRegionInfo.FIRST_META_REGIONINFO); boolean rit = this.assignmentManager - .processRegionInTransitionAndBlockUntilAssigned(HRegionInfo.FIRST_META_REGIONINFO); + .processRegionInTransitionAndBlockUntilAssigned(HRegionInfo.FIRST_META_REGIONINFO); boolean metaRegionLocation = this.catalogTracker.verifyMetaRegionLocation(timeout); + ServerName currentMetaServer = this.catalogTracker.getMetaLocation(); if (!metaRegionLocation) { // Meta location is not verified. It should be in transition, or offline. // We will wait for it to be assigned in enableSSHandWaitForMeta below. assigned++; if (!rit) { // Assign meta since not already in transition - ServerName currentMetaServer = this.catalogTracker.getMetaLocation(); if (currentMetaServer != null) { if (expireIfOnline(currentMetaServer)) { splitMetaLogBeforeAssignment(currentMetaServer); @@ -1013,8 +1015,10 @@ MasterServices, Server { } } else { // Region already assigned. We didn't assign it. Add to in-memory state. - this.assignmentManager.regionOnline(HRegionInfo.FIRST_META_REGIONINFO, - this.catalogTracker.getMetaLocation()); + regionStates.updateRegionState( + HRegionInfo.FIRST_META_REGIONINFO, State.OPEN, currentMetaServer); + this.assignmentManager.regionOnline( + HRegionInfo.FIRST_META_REGIONINFO, currentMetaServer); } enableMeta(TableName.META_TABLE_NAME); 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 2e5c2964acf..893bd1296f1 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 @@ -158,8 +158,8 @@ public class RegionStates { /** * @return True if specified region in transition. */ - public synchronized boolean isRegionInTransition(final String regionName) { - return regionsInTransition.containsKey(regionName); + public synchronized boolean isRegionInTransition(final String encodedName) { + return regionsInTransition.containsKey(encodedName); } /** @@ -197,8 +197,8 @@ public class RegionStates { * @return True if specified region is in one of the specified states. */ public synchronized boolean isRegionInState( - final String regionName, final State... states) { - RegionState regionState = getRegionState(regionName); + final String encodedName, final State... states) { + RegionState regionState = getRegionState(encodedName); State s = regionState != null ? regionState.getState() : null; for (State state: states) { if (s == state) return true; @@ -226,8 +226,8 @@ public class RegionStates { * Get region transition state */ public synchronized RegionState - getRegionTransitionState(final String regionName) { - return regionsInTransition.get(regionName); + getRegionTransitionState(final String encodedName) { + return regionsInTransition.get(encodedName); } /** @@ -250,14 +250,14 @@ public class RegionStates { */ public synchronized RegionState createRegionState(final HRegionInfo hri) { State newState = (hri.isOffline() && hri.isSplit()) ? State.SPLIT : State.OFFLINE; - String regionName = hri.getEncodedName(); - RegionState regionState = regionStates.get(regionName); + String encodedName = hri.getEncodedName(); + RegionState regionState = regionStates.get(encodedName); if (regionState != null) { LOG.warn("Tried to create a state for a region already in RegionStates, " + "used existing: " + regionState + ", ignored new: " + newState); } else { regionState = new RegionState(hri, newState); - regionStates.put(regionName, regionState); + regionStates.put(encodedName, regionState); } return regionState; } @@ -313,28 +313,28 @@ public class RegionStates { + " on " + serverName + ", set to " + state); } - String regionName = hri.getEncodedName(); + String encodedName = hri.getEncodedName(); RegionState regionState = new RegionState( hri, state, System.currentTimeMillis(), newServerName); - RegionState oldState = regionStates.put(regionName, regionState); + RegionState oldState = regionStates.put(encodedName, regionState); if (oldState == null || oldState.getState() != regionState.getState()) { LOG.info("Transitioned " + oldState + " to " + regionState); } if (newServerName != null || ( state != State.PENDING_CLOSE && state != State.CLOSING)) { - regionsInTransition.put(regionName, regionState); + regionsInTransition.put(encodedName, regionState); } // For these states, region should be properly closed. // There should be no log splitting issue. if ((state == State.CLOSED || state == State.MERGED - || state == State.SPLIT) && lastAssignments.containsKey(regionName)) { + || state == State.SPLIT) && lastAssignments.containsKey(encodedName)) { ServerName oldServerName = oldState == null ? null : oldState.getServerName(); - ServerName last = lastAssignments.get(regionName); + ServerName last = lastAssignments.get(encodedName); if (last.equals(oldServerName)) { - lastAssignments.remove(regionName); + lastAssignments.remove(encodedName); } else { - LOG.warn(regionName + " moved to " + state + " on " + LOG.warn(encodedName + " moved to " + state + " on " + oldServerName + ", expected " + last); } } @@ -361,8 +361,8 @@ public class RegionStates { return; } - String regionName = hri.getEncodedName(); - RegionState oldState = regionStates.get(regionName); + String encodedName = hri.getEncodedName(); + RegionState oldState = regionStates.get(encodedName); if (oldState == null) { LOG.warn("Online region not in RegionStates: " + hri.getShortNameToLog()); } else { @@ -374,9 +374,9 @@ public class RegionStates { } } updateRegionState(hri, State.OPEN, serverName); - regionsInTransition.remove(regionName); + regionsInTransition.remove(encodedName); - lastAssignments.put(regionName, serverName); + lastAssignments.put(encodedName, serverName); ServerName oldServerName = regionAssignments.put(hri, serverName); if (!serverName.equals(oldServerName)) { LOG.info("Onlined " + hri.getShortNameToLog() + " on " + serverName); @@ -449,30 +449,13 @@ public class RegionStates { public synchronized void regionOffline( final HRegionInfo hri, final State expectedState) { Preconditions.checkArgument(expectedState == null - || RegionState.isNotUnassignableNotInTransition(expectedState), - "Offlined region should be in state OFFLINE/SPLIT/MERGED/" - + "SPLITTING_NEW/MERGING_NEW instead of " + expectedState); - String regionName = hri.getEncodedName(); - RegionState oldState = regionStates.get(regionName); - if (oldState == null) { - LOG.warn("Offline region not in RegionStates: " + hri.getShortNameToLog()); - } else if (LOG.isDebugEnabled()) { - ServerName sn = oldState.getServerName(); - if (!oldState.isReadyToOffline()) { - LOG.debug("Offline " + hri.getShortNameToLog() + " with current state=" - + oldState.getState() + ", expected state=OFFLINE/SPLIT/" - + "MERGED/SPLITTING_NEW/MERGING_NEW"); - } - if (sn != null && oldState.isOffline()) { - LOG.debug("Offline " + hri.getShortNameToLog() - + " with current state=OFFLINE, assigned to server: " - + sn + ", expected null"); - } - } - State newState = expectedState; - if (newState == null) newState = State.OFFLINE; + || RegionState.isUnassignable(expectedState), + "Offlined region should not be " + expectedState); + String encodedName = hri.getEncodedName(); + State newState = + expectedState == null ? State.OFFLINE : expectedState; updateRegionState(hri, newState); - regionsInTransition.remove(regionName); + regionsInTransition.remove(encodedName); ServerName oldServerName = regionAssignments.remove(hri); if (oldServerName != null) { @@ -518,7 +501,6 @@ public class RegionStates { } for (HRegionInfo hri : regionsToOffline) { - updateRegionState(hri, State.OFFLINE); regionOffline(hri); } @@ -604,8 +586,8 @@ public class RegionStates { * think it's online falsely. Therefore if a server is online, we still * need to confirm it reachable and having the expected start code. */ - synchronized boolean wasRegionOnDeadServer(final String regionName) { - ServerName server = lastAssignments.get(regionName); + synchronized boolean wasRegionOnDeadServer(final String encodedName) { + ServerName server = lastAssignments.get(encodedName); return isServerDeadAndNotProcessed(server); } @@ -639,8 +621,8 @@ public class RegionStates { * Get the last region server a region was on for purpose of re-assignment, * i.e. should the re-assignment be held back till log split is done? */ - synchronized ServerName getLastRegionServerOfRegion(final String regionName) { - return lastAssignments.get(regionName); + synchronized ServerName getLastRegionServerOfRegion(final String encodedName) { + return lastAssignments.get(encodedName); } synchronized void setLastRegionServerOfRegions( @@ -729,8 +711,8 @@ public class RegionStates { return regionStates.get(hri.getEncodedName()); } - protected synchronized RegionState getRegionState(final String regionName) { - return regionStates.get(regionName); + protected synchronized RegionState getRegionState(final String encodedName) { + return regionStates.get(encodedName); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java index a884a3d8bd8..26812af33ef 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java @@ -39,7 +39,6 @@ import org.apache.hadoop.hbase.master.MasterFileSystem; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.RegionStates; import org.apache.hadoop.hbase.master.RegionState.State; -import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Threads; import org.apache.zookeeper.KeeperException; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java index 59ea53d1acb..34a75e60eb4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java @@ -266,7 +266,7 @@ public class ServerShutdownHandler extends EventHandler { // but though we did assign we will not be clearing the znode in CLOSING state. // Doing this will have no harm. See HBASE-5927 regionStates.updateRegionState(hri, State.OFFLINE); - am.deleteClosingOrClosedNode(hri); + am.deleteClosingOrClosedNode(hri, rit.getServerName()); am.offlineDisabledRegion(hri); } else { LOG.warn("THIS SHOULD NOT HAPPEN: unexpected region in transition " diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java index a525cf14eca..c9067a1c129 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java @@ -526,7 +526,7 @@ public class RegionMergeTransaction { /** * Wait for the merging node to be transitioned from pending_merge - * to merging by master. That's how we are sure master has processed + * to merging by master. That's how we are sure master has processed * the event and is good with us to move on. If we don't get any update, * we periodically transition the node so that master gets the callback. * If the node is removed or is not in pending_merge state any more, @@ -725,12 +725,12 @@ public class RegionMergeTransaction { try { // Only delete if its in expected state; could have been hijacked. if (!ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), - RS_ZK_REQUEST_REGION_MERGE)) { + RS_ZK_REQUEST_REGION_MERGE, server.getServerName())) { ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), - RS_ZK_REGION_MERGING); + RS_ZK_REGION_MERGING, server.getServerName()); } } catch (KeeperException.NoNodeException e) { - LOG.warn("Failed cleanup zk node of " + hri.getRegionNameAsString(), e); + LOG.info("Failed cleanup zk node of " + hri.getRegionNameAsString(), e); } catch (KeeperException e) { server.abort("Failed cleanup zk node of " + hri.getRegionNameAsString(),e); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java index b728efffb47..a76171d03a0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java @@ -483,7 +483,7 @@ public class SplitTransaction { /** * Wait for the splitting node to be transitioned from pending_split - * to splitting by master. That's how we are sure master has processed + * to splitting by master. That's how we are sure master has processed * the event and is good with us to move on. If we don't get any update, * we periodically transition the node so that master gets the callback. * If the node is removed or is not in pending_split state any more, @@ -871,12 +871,12 @@ public class SplitTransaction { try { // Only delete if its in expected state; could have been hijacked. if (!ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), - RS_ZK_REQUEST_REGION_SPLIT)) { + RS_ZK_REQUEST_REGION_SPLIT, server.getServerName())) { ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), - RS_ZK_REGION_SPLITTING); + RS_ZK_REGION_SPLITTING, server.getServerName()); } } catch (KeeperException.NoNodeException e) { - LOG.warn("Failed cleanup zk node of " + hri.getRegionNameAsString(), e); + LOG.info("Failed cleanup zk node of " + hri.getRegionNameAsString(), e); } catch (KeeperException e) { server.abort("Failed cleanup of " + hri.getRegionNameAsString(), e); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java index b681db65777..ed850df1e33 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java @@ -682,7 +682,7 @@ public class TestAssignmentManager { // First make sure my mock up basically works. Unassign a region. unassign(am, SERVERNAME_A, hri); // This delete will fail if the previous unassign did wrong thing. - ZKAssign.deleteClosingNode(this.watcher, hri); + ZKAssign.deleteClosingNode(this.watcher, hri, SERVERNAME_A); // Now put a SPLITTING region in the way. I don't have to assert it // go put in place. This method puts it in place then asserts it still // owns it by moving state from SPLITTING to SPLITTING. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java index 3a833791219..cc1fa83b13f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java @@ -776,7 +776,7 @@ public class TestMasterFailover { byte [] bytes = ZKAssign.getData(zkw, region.getEncodedName()); RegionTransition rt = RegionTransition.parseFrom(bytes); if (rt != null && rt.getEventType().equals(EventType.RS_ZK_REGION_OPENED)) { - ZKAssign.deleteOpenedNode(zkw, region.getEncodedName()); + ZKAssign.deleteOpenedNode(zkw, region.getEncodedName(), rt.getServerName()); LOG.debug("DELETED " + rt); break; } @@ -794,7 +794,7 @@ public class TestMasterFailover { byte [] bytes = ZKAssign.getData(zkw, region.getEncodedName()); RegionTransition rt = RegionTransition.parseFrom(bytes); if (rt != null && rt.getEventType().equals(EventType.RS_ZK_REGION_OPENED)) { - ZKAssign.deleteOpenedNode(zkw, region.getEncodedName()); + ZKAssign.deleteOpenedNode(zkw, region.getEncodedName(), rt.getServerName()); break; } Thread.sleep(100); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java index 46ff9048011..5d70b97f19e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java @@ -123,7 +123,8 @@ public class TestRegionServerNoMaster { Assert.assertTrue(getRS().getRegion(regionName).isAvailable()); Assert.assertTrue( - ZKAssign.deleteOpenedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName())); + ZKAssign.deleteOpenedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName(), + getRS().getServerName())); } @@ -194,7 +195,8 @@ public class TestRegionServerNoMaster { checkRegionIsClosed(); - ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName()); + ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName(), + getRS().getServerName()); reopenRegion(); } @@ -274,7 +276,8 @@ public class TestRegionServerNoMaster { checkRegionIsClosed(); Assert.assertTrue( - ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName()) + ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName(), + getRS().getServerName()) ); reopenRegion(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java index 317d3500caa..6349c18affd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java @@ -419,8 +419,9 @@ public class TestSplitTransactionOnCluster { int regionCount = ProtobufUtil.getOnlineRegions(server).size(); // Insert into zk a blocking znode, a znode of same name as region // so it gets in way of our splitting. + ServerName fakedServer = new ServerName("any.old.server", 1234, -1); ZKAssign.createNodeClosing(TESTING_UTIL.getZooKeeperWatcher(), - hri, new ServerName("any.old.server", 1234, -1)); + hri, fakedServer); // Now try splitting.... should fail. And each should successfully // rollback. this.admin.split(hri.getRegionNameAsString()); @@ -432,7 +433,8 @@ public class TestSplitTransactionOnCluster { assertEquals(regionCount, ProtobufUtil.getOnlineRegions(server).size()); } // Now clear the zknode - ZKAssign.deleteClosingNode(TESTING_UTIL.getZooKeeperWatcher(), hri); + ZKAssign.deleteClosingNode(TESTING_UTIL.getZooKeeperWatcher(), + hri, fakedServer); // Now try splitting and it should work. split(hri, server, regionCount); // Get daughters diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java index 312966afe3c..f864ec5412f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java @@ -211,8 +211,9 @@ public class TestCloseRegionHandler { // This parse is not used? RegionTransition.parseFrom(ZKAssign.getData(server.getZooKeeper(), hri.getEncodedName())); // delete the node, which is what Master do after the region is opened - ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), EventType.RS_ZK_REGION_OPENED); - } + ZKAssign.deleteNode(server.getZooKeeper(), hri.getEncodedName(), + EventType.RS_ZK_REGION_OPENED, server.getServerName()); + } }