Index: src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java (revision 1166581) +++ src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java (working copy) @@ -1303,6 +1303,7 @@ } /** + * Tries to assign a region. Region could be reassigned to the same server. * @param regionName Region name to assign. * @param force True to force assign. * @throws MasterNotRunningException Index: src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (revision 1166996) +++ src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (working copy) @@ -60,9 +60,11 @@ import org.apache.hadoop.hbase.executor.ExecutorService; import org.apache.hadoop.hbase.executor.RegionTransitionData; import org.apache.hadoop.hbase.executor.EventHandler.EventType; +import org.apache.hadoop.hbase.regionserver.RegionAlreadyInTransitionException; import org.apache.hadoop.hbase.regionserver.RegionOpeningState; import org.apache.hadoop.hbase.ipc.ServerNotRunningYetException; import org.apache.hadoop.hbase.master.RegionPlan; +import org.apache.hadoop.hbase.master.AssignmentManager.RegionState.State; import org.apache.hadoop.hbase.master.handler.ClosedRegionHandler; import org.apache.hadoop.hbase.master.handler.DisableTableHandler; import org.apache.hadoop.hbase.master.handler.EnableTableHandler; @@ -94,6 +96,7 @@ * Handles existing regions in transition during master failover. */ public class AssignmentManager extends ZooKeeperListener { + private static final Log LOG = LogFactory.getLog(AssignmentManager.class); protected Server master; @@ -162,6 +165,9 @@ //Thread pool executor service for timeout monitor private java.util.concurrent.ExecutorService threadPoolExecutorService; + //String to compare the RegionsAlreadyInTransition from RS + private static final String ALREADY_TRANSITING = "for the region we are " + + "already trying to "; /** * Constructs a new assignment manager. @@ -1434,6 +1440,17 @@ } break; } catch (Throwable t) { + if (t instanceof RemoteException) { + t = ((RemoteException) t).unwrapRemoteException(); + if (t instanceof RegionAlreadyInTransitionException) { + String errorMsg = "Failed assignment of " + + state.getRegion().getRegionNameAsString() + " to " + + plan.getDestination() + " as the region was already " + + extractRegionState((RegionAlreadyInTransitionException) t) + + " in the RS " +plan.getDestination(); + LOG.error(errorMsg, t); + return; + } LOG.warn("Failed assignment of " + state.getRegion().getRegionNameAsString() + " to " + plan.getDestination() + ", trying to assign elsewhere instead; " + @@ -1450,8 +1467,16 @@ } } } + } } + private State extractRegionState(RegionAlreadyInTransitionException t) { + if (t.getMessage().contains(ALREADY_TRANSITING+"OPEN")) { + return RegionState.State.PENDING_OPEN; + } + return RegionState.State.PENDING_CLOSE; + } + private void debugLog(HRegionInfo region, String string) { if (region.isMetaTable() || region.isRootRegion()) { LOG.info(string); Index: src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (revision 1166996) +++ src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (working copy) @@ -43,6 +43,7 @@ import java.util.SortedMap; import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -155,6 +156,7 @@ */ public class HRegionServer implements HRegionInterface, HBaseRPCErrorHandler, Runnable, RegionServerServices { + public static final Log LOG = LogFactory.getLog(HRegionServer.class); // Set when a report to the master comes back with a message asking us to @@ -182,8 +184,8 @@ private Path rootDir; private final Random rand = new Random(); - private final Set regionsInTransitionInRS = - new ConcurrentSkipListSet(Bytes.BYTES_COMPARATOR); + private final ConcurrentSkipListMap regionsInTransitionInRS = + new ConcurrentSkipListMap(Bytes.BYTES_COMPARATOR); /** * Map of regions currently being served by this region server. Key is the @@ -306,6 +308,16 @@ */ private TableDescriptors tableDescriptors; + /* + * Strings to be used in forming the exception message for + * RegionsAlreadyInTransitionException. The above message is used + * to extract the status in the master. + */ + private static final String ALREADY_TRANSITING = "for the region we are already trying to "; + private static final String RECEIVED = "received"; + private static final String OPEN = "OPEN"; + private static final String CLOSE = "CLOSE"; + private static final String SPACE = " "; /** * Starts a HRegionServer at the default location * @@ -803,7 +815,7 @@ // iterator of onlineRegions to close all user regions. for (Map.Entry e : this.onlineRegions.entrySet()) { HRegionInfo hri = e.getValue().getRegionInfo(); - if (!this.regionsInTransitionInRS.contains(hri.getEncodedNameAsBytes())) { + if (!this.regionsInTransitionInRS.containsKey(hri.getEncodedNameAsBytes())) { // Don't update zk with this close transition; pass false. closeRegion(hri, abort, false); } @@ -2343,8 +2355,13 @@ public RegionOpeningState openRegion(HRegionInfo region, int versionOfOfflineNode) throws IOException { checkOpen(); - if (this.regionsInTransitionInRS.contains(region.getEncodedNameAsBytes())) { - throw new RegionAlreadyInTransitionException("open", region.getEncodedName()); + byte[] encodedName = region.getEncodedNameAsBytes(); + if (this.regionsInTransitionInRS.containsKey(encodedName)) { + boolean actionType = this.regionsInTransitionInRS.get(encodedName); + throw new RegionAlreadyInTransitionException(REGIONSERVER + SPACE + + this.getServerName() + RECEIVED + SPACE + OPEN + SPACE + + ALREADY_TRANSITING + (actionType ? OPEN : CLOSE) + "; " + + region.getRegionNameAsString()); } HRegion onlineRegion = this.getFromOnlineRegions(region.getEncodedName()); if (null != onlineRegion) { @@ -2354,7 +2371,7 @@ } LOG.info("Received request to open region: " + region.getRegionNameAsString()); - this.regionsInTransitionInRS.add(region.getEncodedNameAsBytes()); + this.regionsInTransitionInRS.putIfAbsent(encodedName, true); HTableDescriptor htd = this.tableDescriptors.get(region.getTableName()); // Need to pass the expected version in the constructor. if (region.isRootRegion()) { @@ -2399,8 +2416,14 @@ throw new NotServingRegionException("Received close for " + region.getRegionNameAsString() + " but we are not serving it"); } - if (this.regionsInTransitionInRS.contains(region.getEncodedNameAsBytes())) { - throw new RegionAlreadyInTransitionException("close", region.getEncodedName()); + byte[] encodedName = region + .getEncodedNameAsBytes(); + if (this.regionsInTransitionInRS.containsKey(encodedName)) { + boolean actionType = this.regionsInTransitionInRS.get(encodedName); + throw new RegionAlreadyInTransitionException(REGIONSERVER + SPACE + + this.getServerName() + RECEIVED + SPACE + CLOSE + SPACE + + ALREADY_TRANSITING + (actionType ? OPEN : CLOSE) + "; " + + region.getRegionNameAsString()); } return closeRegion(region, false, zk); } @@ -2421,12 +2444,12 @@ */ protected boolean closeRegion(HRegionInfo region, final boolean abort, final boolean zk) { - if (this.regionsInTransitionInRS.contains(region.getEncodedNameAsBytes())) { + if (this.regionsInTransitionInRS.containsKey(region.getEncodedNameAsBytes())) { LOG.warn("Received close for region we are already opening or closing; " + region.getEncodedName()); return false; } - this.regionsInTransitionInRS.add(region.getEncodedNameAsBytes()); + this.regionsInTransitionInRS.putIfAbsent(region.getEncodedNameAsBytes(), false); CloseRegionHandler crh = null; if (region.isRootRegion()) { crh = new CloseRootHandler(this, this, region, abort, zk); @@ -3022,7 +3045,7 @@ } - public Set getRegionsInTransitionInRS() { + public ConcurrentSkipListMap getRegionsInTransitionInRS() { return this.regionsInTransitionInRS; } Index: src/main/java/org/apache/hadoop/hbase/regionserver/RegionAlreadyInTransitionException.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/regionserver/RegionAlreadyInTransitionException.java (revision 1161985) +++ src/main/java/org/apache/hadoop/hbase/regionserver/RegionAlreadyInTransitionException.java (working copy) @@ -27,8 +27,7 @@ */ public class RegionAlreadyInTransitionException extends IOException { - public RegionAlreadyInTransitionException(String action, String region) { - super("Received " + action + " for region we are" + - " already opening or closing; " + region); + public RegionAlreadyInTransitionException(String s) { + super(s); } } Index: src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java (revision 1161985) +++ src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java (working copy) @@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.regionserver.wal.HLog; import org.apache.zookeeper.KeeperException; import java.util.Set; +import java.util.concurrent.ConcurrentSkipListMap; /** * Services provided by {@link HRegionServer} @@ -74,7 +75,7 @@ /** * Get the regions that are currently being opened or closed in the RS - * @return set of regions in transition in this RS + * @return map of regions in transition in this RS */ - public Set getRegionsInTransitionInRS(); + public ConcurrentSkipListMap getRegionsInTransitionInRS(); } \ No newline at end of file Index: src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java (revision 1161985) +++ src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java (working copy) @@ -49,6 +49,8 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; + +import static org.junit.Assert.*; import static org.junit.Assert.assertTrue; /** @@ -239,7 +241,7 @@ HRegionInfo hri = getNonMetaRegion(hr0.getOnlineRegions()); // fake that hr1 is processing the region - hr1.getRegionsInTransitionInRS().add(hri.getEncodedNameAsBytes()); + hr1.getRegionsInTransitionInRS().putIfAbsent(hri.getEncodedNameAsBytes(), true); AtomicBoolean reopenEventProcessed = new AtomicBoolean(false); EventHandlerListener openListener = @@ -252,13 +254,10 @@ TEST_UTIL.getHBaseAdmin().move(hri.getEncodedNameAsBytes(), Bytes.toBytes(hr1.getServerName().toString())); - while (!reopenEventProcessed.get()) { - Threads.sleep(100); - } - // make sure the region came back - assertTrue(hr1.getOnlineRegion(hri.getEncodedNameAsBytes()) == null); + assertEquals(hr1.getOnlineRegion(hri.getEncodedNameAsBytes()), null); + // remove the block and reset the boolean hr1.getRegionsInTransitionInRS().remove(hri.getEncodedNameAsBytes()); reopenEventProcessed.set(false); Index: src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java (revision 1169601) +++ src/test/java/org/apache/hadoop/hbase/regionserver/handler/MockRegionServerServices.java (working copy) @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentSkipListMap; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ServerName; @@ -42,7 +43,8 @@ class MockRegionServerServices implements RegionServerServices { private final Map regions = new HashMap(); private boolean stopping = false; - private final Set rit = new HashSet(); + private final ConcurrentSkipListMap rit = + new ConcurrentSkipListMap(); @Override public boolean removeFromOnlineRegions(String encodedRegionName) { @@ -80,7 +82,7 @@ } @Override - public Set getRegionsInTransitionInRS() { + public ConcurrentSkipListMap getRegionsInTransitionInRS() { return rit; }