hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nkey...@apache.org
Subject svn commit: r1427064 - in /hbase/trunk/hbase-server/src: main/java/org/apache/hadoop/hbase/client/ main/java/org/apache/hadoop/hbase/master/ main/java/org/apache/hadoop/hbase/regionserver/ main/java/org/apache/hadoop/hbase/regionserver/handler/ main/ja...
Date Mon, 31 Dec 2012 09:50:28 GMT
Author: nkeywal
Date: Mon Dec 31 09:50:27 2012
New Revision: 1427064

URL: http://svn.apache.org/viewvc?rev=1427064&view=rev
Log:
HBASE-7390 Add extra test cases for assignement on the region server and fix the related issues

Added:
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java
Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/HTable.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/HTable.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/HTable.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/HTable.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/client/HTable.java Mon Dec 31 09:50:27 2012
@@ -241,7 +241,6 @@ public class HTable implements HTableInt
 
   /**
    * setup this HTable's parameter based on the passed configuration
-   * @param conf
    */
   private void finishSetup() throws IOException {
     this.connection.locateRegion(tableName, HConstants.EMPTY_START_ROW);
@@ -343,11 +342,10 @@ public class HTable implements HTableInt
   }
 
   /**
-   * Finds the region on which the given row is being served.
+   * Finds the region on which the given row is being served. Does not reload the cache.
    * @param row Row to find.
    * @return Location of the row.
    * @throws IOException if a remote or network exception occurs
-   * @deprecated use {@link #getRegionLocation(byte [], boolean)} instead
    */
   public HRegionLocation getRegionLocation(final byte [] row)
   throws IOException {
@@ -357,8 +355,7 @@ public class HTable implements HTableInt
   /**
    * Finds the region on which the given row is being served.
    * @param row Row to find.
-   * @param reload whether or not to reload information or just use cached
-   * information
+   * @param reload true to reload information or false to use cached information
    * @return Location of the row.
    * @throws IOException if a remote or network exception occurs
    */

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Mon Dec 31 09:50:27 2012
@@ -162,8 +162,10 @@ public class AssignmentManager extends Z
    * that ServerShutdownHandler can be fully enabled and re-assign regions
    * of dead servers. So that when re-assignment happens, AssignmentManager
    * has proper region states.
+   *
+   * Protected to ease testing.
    */
-  final AtomicBoolean failoverCleanupDone = new AtomicBoolean(false);
+  protected final AtomicBoolean failoverCleanupDone = new AtomicBoolean(false);
 
   /**
    * Constructs a new assignment manager.
@@ -1461,6 +1463,7 @@ public class AssignmentManager extends Z
           return;
         }
         // This never happens. Currently regionserver close always return true.
+        // Todo; this can now happen (0.96) if there is an exception in a coprocessor
         LOG.warn("Server " + server + " region CLOSE RPC returned false for " +
           region.getRegionNameAsString());
       } catch (Throwable t) {

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Mon Dec 31 09:50:27 2012
@@ -52,6 +52,7 @@ import java.util.concurrent.locks.Reentr
 
 import javax.management.ObjectName;
 
+import com.google.protobuf.Message;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience;
@@ -213,7 +214,6 @@ import org.cliffc.high_scale_lib.Counter
 
 import com.google.common.base.Function;
 import com.google.protobuf.ByteString;
-import com.google.protobuf.Message;
 import com.google.protobuf.RpcController;
 import com.google.protobuf.ServiceException;
 
@@ -240,7 +240,7 @@ public class  HRegionServer implements C
   //RegionName vs current action in progress
   //true - if open region action in progress
   //false - if close region action in progress
-  protected final ConcurrentSkipListMap<byte[], Boolean> regionsInTransitionInRS =
+  protected final ConcurrentMap<byte[], Boolean> regionsInTransitionInRS =
     new ConcurrentSkipListMap<byte[], Boolean>(Bytes.BYTES_COMPARATOR);
 
   protected long maxScannerResultSize;
@@ -443,7 +443,7 @@ public class  HRegionServer implements C
     this.isOnline = false;
     checkCodecs(this.conf);
 
-    // do we use checksum verfication in the hbase? If hbase checksum verification
+    // do we use checksum verification in the hbase? If hbase checksum verification
     // is enabled, then we automatically switch off hdfs checksum verification.
     this.useHBaseChecksum = conf.getBoolean(
       HConstants.HBASE_CHECKSUM_VERIFICATION, false);
@@ -665,7 +665,7 @@ public class  HRegionServer implements C
       if (rpcArgClass == null || from.getRequest().isEmpty()) {
         return HConstants.NORMAL_QOS;
       }
-      Object deserializedRequestObj = null;
+      Object deserializedRequestObj;
       //check whether the request has reference to Meta region
       try {
         Method parseFrom = methodMap.get("parseFrom").get(rpcArgClass);
@@ -960,7 +960,7 @@ public class  HRegionServer implements C
 
     //fsOk flag may be changed when closing regions throws exception.
     if (!this.killed && this.fsOk) {
-      closeWAL(abortRequested ? false : true);
+      closeWAL(!abortRequested);
     }
 
     // Make sure the proxy is down.
@@ -1106,7 +1106,7 @@ public class  HRegionServer implements C
             && !closedRegions.contains(hri.getEncodedName())) {
           closedRegions.add(hri.getEncodedName());
           // Don't update zk with this close transition; pass false.
-          closeRegion(hri, abort, false);
+          closeRegionIgnoreErrors(hri, abort);
         }
       }
       // No regions in RIT, we could stop waiting now.
@@ -1167,7 +1167,7 @@ public class  HRegionServer implements C
             this.serverNameFromMasterPOV.getHostname());
           continue;
         }
-        String value = e.getValue().toString();
+        String value = e.getValue();
         if (LOG.isDebugEnabled()) {
           LOG.debug("Config from master: " + key + "=" + value);
         }
@@ -1440,7 +1440,7 @@ public class  HRegionServer implements C
   }
 
   /*
-   * Start maintanence Threads, Server, Worker and lease checker threads.
+   * Start maintenance Threads, Server, Worker and lease checker threads.
    * Install an UncaughtExceptionHandler that calls abort of RegionServer if we
    * get an unhandled exception. We cannot set the handler on all threads.
    * Server's internal Listener thread is off limits. For Server, if an OOME, it
@@ -1892,8 +1892,8 @@ public class  HRegionServer implements C
     } finally {
       this.lock.writeLock().unlock();
     }
-    if (meta != null) closeRegion(meta.getRegionInfo(), abort, false);
-    if (root != null) closeRegion(root.getRegionInfo(), abort, false);
+    if (meta != null) closeRegionIgnoreErrors(meta.getRegionInfo(), abort);
+    if (root != null) closeRegionIgnoreErrors(root.getRegionInfo(), abort);
   }
 
   /**
@@ -1909,7 +1909,7 @@ public class  HRegionServer implements C
         HRegion r = e.getValue();
         if (!r.getRegionInfo().isMetaTable() && r.isAvailable()) {
           // Don't update zk with this close transition; pass false.
-          closeRegion(r.getRegionInfo(), abort, false);
+          closeRegionIgnoreErrors(r.getRegionInfo(), abort);
         }
       }
     } finally {
@@ -1984,7 +1984,7 @@ public class  HRegionServer implements C
         });
     // Copy over all regions. Regions are sorted by size with biggest first.
     for (HRegion region : this.onlineRegions.values()) {
-      sortedRegions.put(Long.valueOf(region.memstoreSize.get()), region);
+      sortedRegions.put(region.memstoreSize.get(), region);
     }
     return sortedRegions;
   }
@@ -2106,7 +2106,7 @@ public class  HRegionServer implements C
   }
 
 
-  public ConcurrentSkipListMap<byte[], Boolean> getRegionsInTransitionInRS() {
+  public ConcurrentMap<byte[], Boolean> getRegionsInTransitionInRS() {
     return this.regionsInTransitionInRS;
   }
 
@@ -2267,7 +2267,7 @@ public class  HRegionServer implements C
     for (HRegion region: regions) {
       coprocessors.addAll(region.getCoprocessorHost().getCoprocessors());
     }
-    return coprocessors.toArray(new String[0]);
+    return coprocessors.toArray(new String[coprocessors.size()]);
   }
 
   /**
@@ -2367,70 +2367,103 @@ public class  HRegionServer implements C
     }
   }
 
-  protected void checkIfRegionInTransition(byte[] regionEncodedName,
-      String currentAction) throws RegionAlreadyInTransitionException {
-    if (this.regionsInTransitionInRS.containsKey(regionEncodedName)) {
-      boolean openAction = this.regionsInTransitionInRS.get(regionEncodedName);
-      // The below exception message will be used in master.
-      throw new RegionAlreadyInTransitionException("Received:" + currentAction +
-        " for the region:" + Bytes.toString(regionEncodedName) +
-        " ,which we are already trying to " +
-        (openAction ? OPEN : CLOSE)+ ".");
-    }
-  }
 
   /**
+   * Try to close the region, logs a warning on failure but continues.
    * @param region Region to close
-   * @param abort True if we are aborting
-   * @param zk True if we are to update zk about the region close; if the close
-   * was orchestrated by master, then update zk.  If the close is being run by
-   * the regionserver because its going down, don't update zk.
-   * @return True if closed a region.
    */
-  protected boolean closeRegion(HRegionInfo region, final boolean abort,
-      final boolean zk) {
-    return closeRegion(region, abort, zk, -1, null);
+  private void closeRegionIgnoreErrors(HRegionInfo region, final boolean abort) {
+    try {
+      if (!closeRegion(region.getEncodedName(), abort, false, -1, null)) {
+        LOG.warn("Failed to close " + region.getRegionNameAsString() +
+            " - ignoring and continuing");
+      }
+    } catch (NotServingRegionException e) {
+      LOG.warn("Failed to close " + region.getRegionNameAsString() +
+          " - ignoring and continuing", e);
+    }
   }
 
-    /**
-   * @param region Region to close
+  /**
+   * Close asynchronously a region, can be called from the master or internally by the regionserver
+   * when stopping. If called from the master, the region will update the znode status.
+   *
+   * <p>
+   * If an opening was in progress, this method will cancel it, but will not start a new close. The
+   * coprocessors are not called in this case. A NotServingRegionException exception is thrown.
+   * </p>
+
+   * <p>
+   *   If a close was in progress, this new request will be ignored, and an exception thrown.
+   * </p>
+   *
+   * @param encodedName Region to close
    * @param abort True if we are aborting
    * @param zk True if we are to update zk about the region close; if the close
    * was orchestrated by master, then update zk.  If the close is being run by
    * the regionserver because its going down, don't update zk.
-   * @param versionOfClosingNode
-   *   the version of znode to compare when RS transitions the znode from
+   * @param versionOfClosingNode the version of znode to compare when RS transitions the znode from
    *   CLOSING state.
    * @return True if closed a region.
+   * @throws NotServingRegionException if the region is not online or if a close
+   * request in in progress.
    */
-  protected boolean closeRegion(HRegionInfo region, final boolean abort,
-      final boolean zk, final int versionOfClosingNode, ServerName sn) {
+  protected boolean closeRegion(String encodedName, final boolean abort,
+      final boolean zk, final int versionOfClosingNode, final ServerName sn)
+      throws NotServingRegionException {
     //Check for permissions to close.
-    HRegion actualRegion = this.getFromOnlineRegions(region.getEncodedName());
+    final HRegion actualRegion = this.getFromOnlineRegions(encodedName);
     if ((actualRegion != null) && (actualRegion.getCoprocessorHost() != null)) {
       try {
         actualRegion.getCoprocessorHost().preClose(false);
       } catch (IOException exp) {
-        LOG.warn("Unable to close region", exp);
+        LOG.warn("Unable to close region: the coprocessor launched an error ", exp);
         return false;
       }
     }
 
-    if (this.regionsInTransitionInRS.containsKey(region.getEncodedNameAsBytes())) {
-      LOG.warn("Received close for region we are already opening or closing; " +
-        region.getEncodedName());
-      return false;
-    }
-    this.regionsInTransitionInRS.putIfAbsent(region.getEncodedNameAsBytes(), false);
-    CloseRegionHandler crh = null;
-    if (region.isRootRegion()) {
-      crh = new CloseRootHandler(this, this, region, abort, zk,
-        versionOfClosingNode);
-    } else if (region.isMetaRegion()) {
-      crh = new CloseMetaHandler(this, this, region, abort, zk,
-        versionOfClosingNode);
+    final Boolean previous = this.regionsInTransitionInRS.putIfAbsent(encodedName.getBytes(),
+        Boolean.FALSE);
+
+    if (Boolean.TRUE.equals(previous)) {
+      LOG.info("Received CLOSE for the region:" + encodedName + " , which we are already " +
+          "trying to OPEN. Cancelling OPENING.");
+      if (!regionsInTransitionInRS.replace(encodedName.getBytes(), previous, Boolean.FALSE)){
+        // The replace failed. That should be an exceptional case, but theoretically it can happen.
+        // We're going to try to do a standard close then.
+        LOG.warn("The opening for region " + encodedName + " was done before we could cancel it." +
+            " Doing a standard close now");
+        return closeRegion(encodedName, abort, zk, versionOfClosingNode, sn);
+      } else {
+        LOG.info("The opening previously in progress has been cancelled by a CLOSE request.");
+        // The master deletes the znode when it receives this exception.
+        throw new NotServingRegionException("The region " + encodedName +
+            " was opening but not yet served. Opening is cancelled.");
+      }
+    } else if (Boolean.FALSE.equals(previous)) {
+      LOG.info("Received CLOSE for the region: " + encodedName +
+          " ,which we are already trying to CLOSE");
+      // The master deletes the znode when it receives this exception.
+      throw new NotServingRegionException("The region " + encodedName +
+          " was already closing. New CLOSE request is ignored.");
+    }
+
+    if (actualRegion == null){
+      LOG.error("Received CLOSE for a region which is not online, and we're not opening.");
+      this.regionsInTransitionInRS.remove(encodedName.getBytes());
+      // The master deletes the znode when it receives this exception.
+      throw new NotServingRegionException("The region " + encodedName +
+          " is not online, and is not opening.");
+    }
+
+    CloseRegionHandler crh;
+    final HRegionInfo hri = actualRegion.getRegionInfo();
+    if (hri.isRootRegion()) {
+      crh = new CloseRootHandler(this, this, hri, abort, zk, versionOfClosingNode);
+    } else if (hri.isMetaRegion()) {
+      crh = new CloseMetaHandler(this, this, hri, abort, zk, versionOfClosingNode);
     } else {
-      crh = new CloseRegionHandler(this, this, region, abort, zk, versionOfClosingNode, sn);
+      crh = new CloseRegionHandler(this, this, hri, abort, zk, versionOfClosingNode, sn);
     }
     this.service.submit(crh);
     return true;
@@ -2517,7 +2550,7 @@ public class  HRegionServer implements C
   protected Throwable cleanup(final Throwable t, final String msg) {
     // Don't log as error if NSRE; NSRE is 'normal' operation.
     if (t instanceof NotServingRegionException) {
-      LOG.debug("NotServingRegionException; " +  t.getMessage());
+      LOG.debug("NotServingRegionException; " + t.getMessage());
       return t;
     }
     if (msg == null) {
@@ -2682,7 +2715,7 @@ public class  HRegionServer implements C
             if (region.getCoprocessorHost() != null) {
               exists = region.getCoprocessorHost().postExists(clientGet, exists);
             }
-            existence = Boolean.valueOf(exists);
+            existence = exists;
           }
         }
       }
@@ -2717,7 +2750,7 @@ public class  HRegionServer implements C
       if (!region.getRegionInfo().isMetaTable()) {
         cacheFlusher.reclaimMemStoreMemory();
       }
-      Integer lock = null;
+      Integer lock;
       Result r = null;
       Boolean processed = null;
       MutateType type = mutate.getMutateType();
@@ -2750,7 +2783,7 @@ public class  HRegionServer implements C
               result = region.getCoprocessorHost().postCheckAndPut(row, family,
                 qualifier, compareOp, comparator, put, result);
             }
-            processed = Boolean.valueOf(result);
+            processed = result;
           }
         } else {
           region.put(put, lock);
@@ -2779,7 +2812,7 @@ public class  HRegionServer implements C
               result = region.getCoprocessorHost().postCheckAndDelete(row, family,
                 qualifier, compareOp, comparator, delete, result);
             }
-            processed = Boolean.valueOf(result);
+            processed = result;
           }
         } else {
           region.delete(delete, lock, delete.getWriteToWAL());
@@ -3088,7 +3121,7 @@ public class  HRegionServer implements C
         region.releaseRowLock(r);
         this.leases.cancelLease(lockName);
         LOG.debug("Row lock " + lockId
-          + " has been explicitly released by client");
+            + " has been explicitly released by client");
         return UnlockRowResponse.newBuilder().build();
       } catch (Throwable t) {
         throw convertThrowableToIOE(cleanup(t));
@@ -3286,7 +3319,7 @@ public class  HRegionServer implements C
     try {
       HRegion region = getRegion(request.getRegion());
       requestCount.increment();
-      Set<byte[]> columnFamilies = null;
+      Set<byte[]> columnFamilies;
       if (request.getFamilyCount() == 0) {
         columnFamilies = region.getStores().keySet();
       } else {
@@ -3328,8 +3361,24 @@ public class  HRegionServer implements C
   // Region open/close direct RPCs
 
   /**
-   * Open a region on the region server.
+   * Open asynchronously a region or a set of regions on the region server.
    *
+   * The opening is coordinated by ZooKeeper, and this method requires the znode to be created
+   *  before being called. As a consequence, this method should be called only from the master.
+   * <p>
+   * Different manages states for the region are:<ul>
+   *  <li>region not opened: the region opening will start asynchronously.</li>
+   *  <li>a close is already in progress: this is considered as an error.</li>
+   *  <li>an open is already in progress: this new open request will be ignored. This is important
+   *  because the Master can do multiple requests if it crashes.</li>
+   *  <li>the region is already opened:  this new open request will be ignored./li>
+   *  </ul>
+   * </p>
+   * <p>
+   * Bulk assign: If there are more than 1 region to open, it will be considered as a bulk assign.
+   * For a single region opening, errors are sent through a ServiceException. For bulk assign,
+   * errors are put in the response as FAILED_OPENING.
+   * </p>
    * @param controller the RPC controller
    * @param request the request
    * @throws ServiceException
@@ -3345,22 +3394,22 @@ public class  HRegionServer implements C
     }
     requestCount.increment();
     OpenRegionResponse.Builder builder = OpenRegionResponse.newBuilder();
-    int regionCount = request.getOpenInfoCount();
-    Map<String, HTableDescriptor> htds =
-      new HashMap<String, HTableDescriptor>(regionCount);
-    boolean isBulkAssign = regionCount > 1;
+    final int regionCount = request.getOpenInfoCount();
+    final Map<String, HTableDescriptor> htds = new HashMap<String, HTableDescriptor>(regionCount);
+    final boolean isBulkAssign = regionCount > 1;
     for (RegionOpenInfo regionOpenInfo : request.getOpenInfoList()) {
-      HRegionInfo region = HRegionInfo.convert(regionOpenInfo.getRegion());
+      final HRegionInfo region = HRegionInfo.convert(regionOpenInfo.getRegion());
+
       int versionOfOfflineNode = -1;
       if (regionOpenInfo.hasVersionOfOfflineNode()) {
         versionOfOfflineNode = regionOpenInfo.getVersionOfOfflineNode();
       }
+      HTableDescriptor htd;
       try {
-        checkIfRegionInTransition(region.getEncodedNameAsBytes(), OPEN);
-        HRegion onlineRegion = getFromOnlineRegions(region.getEncodedName());
-        if (null != onlineRegion) {
+        final HRegion onlineRegion = getFromOnlineRegions(region.getEncodedName());
+        if (onlineRegion != null) {
           //Check if the region can actually be opened.
-          if( onlineRegion.getCoprocessorHost() != null){
+          if (onlineRegion.getCoprocessorHost() != null) {
             onlineRegion.getCoprocessorHost().preOpen();
           }
           // See HBASE-5094. Cross check with META if still this RS is owning
@@ -3373,39 +3422,55 @@ public class  HRegionServer implements C
             builder.addOpeningState(RegionOpeningState.ALREADY_OPENED);
             continue;
           } else {
-            LOG.warn("The region " + region.getEncodedName()
-                + " is online on this server but META does not have this server.");
+            LOG.warn("The region " + region.getEncodedName() + " is online on this server" +
+                " but META does not have this server - continue opening.");
             removeFromOnlineRegions(region.getEncodedName(), null);
           }
         }
         LOG.info("Received request to open region: " + region.getRegionNameAsString() + " on "
             + this.serverNameFromMasterPOV);
-        HTableDescriptor htd = htds.get(region.getTableNameAsString());
+        htd = htds.get(region.getTableNameAsString());
         if (htd == null) {
           htd = this.tableDescriptors.get(region.getTableName());
           htds.put(region.getTableNameAsString(), htd);
         }
-        this.regionsInTransitionInRS.putIfAbsent(
-            region.getEncodedNameAsBytes(), true);
-        // Need to pass the expected version in the constructor.
-        if (region.isRootRegion()) {
-          this.service.submit(new OpenRootHandler(this, this, region, htd,
-              versionOfOfflineNode));
-        } else if (region.isMetaRegion()) {
-          this.service.submit(new OpenMetaHandler(this, this, region, htd,
-              versionOfOfflineNode));
-        } else {
-          this.service.submit(new OpenRegionHandler(this, this, region, htd,
-              versionOfOfflineNode));
+
+        final Boolean previous = this.regionsInTransitionInRS.putIfAbsent(
+            region.getEncodedNameAsBytes(), Boolean.TRUE);
+
+        if (Boolean.FALSE.equals(previous)) {
+          // There is a close in progress. We need to mark this open as failed in ZK.
+          OpenRegionHandler.
+              tryTransitionFromOfflineToFailedOpen(this, region, versionOfOfflineNode);
+
+          throw new RegionAlreadyInTransitionException("Received OPEN for the region:" +
+              region.getRegionNameAsString() + " , which we are already trying to CLOSE ");
+        }
+
+        if (Boolean.TRUE.equals(previous)) {
+          // An open is in progress. This is supported, but let's log this.
+          LOG.info("Receiving OPEN for the region:" +
+              region.getRegionNameAsString() + " , which we are already trying to OPEN" +
+              " - ignoring this new request for this region.");
+        }
+
+        if (previous == null) {
+          // If there is no action in progress, we can submit a specific handler.
+          // Need to pass the expected version in the constructor.
+          if (region.isRootRegion()) {
+            this.service.submit(new OpenRootHandler(this, this, region, htd,
+                versionOfOfflineNode));
+          } else if (region.isMetaRegion()) {
+            this.service.submit(new OpenMetaHandler(this, this, region, htd,
+                versionOfOfflineNode));
+          } else {
+            this.service.submit(new OpenRegionHandler(this, this, region, htd,
+                versionOfOfflineNode));
+          }
         }
+
         builder.addOpeningState(RegionOpeningState.OPENED);
-      } catch (RegionAlreadyInTransitionException rie) {
-        LOG.warn("Region is already in transition", rie);
-        if (isBulkAssign) {
-          builder.addOpeningState(RegionOpeningState.OPENED);
-        } else {
-          throw new ServiceException(rie);
-        }
+
       } catch (IOException ie) {
         LOG.warn("Failed opening region " + region.getRegionNameAsString(), ie);
         if (isBulkAssign) {
@@ -3415,6 +3480,7 @@ public class  HRegionServer implements C
         }
       }
     }
+
     return builder.build();
   }
 
@@ -3439,31 +3505,22 @@ public class  HRegionServer implements C
 
     try {
       checkOpen();
-      String encodedRegionName =
-        ProtobufUtil.getRegionEncodedName(request.getRegion());
-      byte[] encodedName = Bytes.toBytes(encodedRegionName);
-      HRegion region = getRegionByEncodedName(encodedRegionName);
-      if(region.getCoprocessorHost() != null){
+      final String encodedRegionName = ProtobufUtil.getRegionEncodedName(request.getRegion());
+
+      // Can be null if we're calling close on a region that's not online
+      final HRegion region = this.getFromOnlineRegions(encodedRegionName);
+      if ((region  != null) && (region .getCoprocessorHost() != null)) {
         region.getCoprocessorHost().preClose(false);
       }
-      Boolean openAction = regionsInTransitionInRS.get(encodedName);
-      if (openAction != null) {
-        if (openAction.booleanValue()) {
-          regionsInTransitionInRS.replace(encodedName, openAction, Boolean.FALSE);
-        }
-        checkIfRegionInTransition(encodedName, CLOSE);
-      }
 
       requestCount.increment();
-      LOG.info("Received close region: " + region.getRegionNameAsString() +
-        ". Version of ZK closing node:" + versionOfClosingNode +
+      LOG.info("Received close region: " + encodedRegionName +
+          "Transitioning in ZK: " + (zk ? "yes" : "no") +
+          ". Version of ZK closing node:" + versionOfClosingNode +
         ". Destination server:" + sn);
-      HRegionInfo regionInfo = region.getRegionInfo();
-      checkIfRegionInTransition(encodedName, CLOSE);
-      boolean closed = closeRegion(
-        regionInfo, false, zk, versionOfClosingNode, sn);
-      CloseRegionResponse.Builder builder =
-        CloseRegionResponse.newBuilder().setClosed(closed);
+
+      boolean closed = closeRegion(encodedRegionName, false, zk, versionOfClosingNode, sn);
+      CloseRegionResponse.Builder builder = CloseRegionResponse.newBuilder().setClosed(closed);
       return builder.build();
     } catch (IOException ie) {
       throw new ServiceException(ie);
@@ -3554,7 +3611,7 @@ public class  HRegionServer implements C
         store = region.getStore(family);
         if (store == null) {
           throw new ServiceException(new IOException("column family " + Bytes.toString(family) +
-            " does not exist in region " + new String(region.getRegionNameAsString())));
+            " does not exist in region " + region.getRegionNameAsString()));
         }
       }
       if (request.hasMajor()) {
@@ -3681,7 +3738,7 @@ public class  HRegionServer implements C
   protected HRegion getRegion(
       final RegionSpecifier regionSpecifier) throws IOException {
     return getRegionByEncodedName(
-      ProtobufUtil.getRegionEncodedName(regionSpecifier));
+        ProtobufUtil.getRegionEncodedName(regionSpecifier));
   }
 
   /**
@@ -3757,7 +3814,7 @@ public class  HRegionServer implements C
       ActionResult result = resultBuilder.build();
       int i = 0;
       for (Mutate m : mutates) {
-        Mutation mutation = null;
+        Mutation mutation;
         if (m.getMutateType() == MutateType.PUT) {
           mutation = ProtobufUtil.toPut(m);
           batchContainsPuts = true;
@@ -3850,10 +3907,10 @@ public class  HRegionServer implements C
   }
 
 
-  // This map will containsall the regions that we closed for a move.
+  // This map will contains all the regions that we closed for a move.
   //  We add the time it was moved as we don't want to keep too old information
   protected Map<String, Pair<Long, ServerName>> movedRegions =
-    new ConcurrentHashMap<String, Pair<Long, ServerName>>(3000);
+      new ConcurrentHashMap<String, Pair<Long, ServerName>>(3000);
 
   // We need a timeout. If not there is a risk of giving a wrong information: this would double
   //  the number of network calls instead of reducing them.
@@ -3867,8 +3924,8 @@ public class  HRegionServer implements C
     final  Long time = System.currentTimeMillis();
 
     movedRegions.put(
-      encodedName,
-      new Pair<Long, ServerName>(time, destination));
+        encodedName,
+        new Pair<Long, ServerName>(time, destination));
   }
 
   private ServerName getMovedRegion(final String encodedRegionName) {

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java Mon Dec 31 09:50:27 2012
@@ -19,7 +19,7 @@
 package org.apache.hadoop.hbase.regionserver;
 
 import java.io.IOException;
-import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.fs.FileSystem;
@@ -79,7 +79,7 @@ public interface RegionServerServices ex
    * Get the regions that are currently being opened or closed in the RS
    * @return map of regions in transition in this RS
    */
-  public Map<byte[], Boolean> getRegionsInTransitionInRS();
+  public ConcurrentMap<byte[], Boolean> getRegionsInTransitionInRS();
 
   /**
    * @return Return the FileSystem object used by the regionserver

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java Mon Dec 31 09:50:27 2012
@@ -126,13 +126,21 @@ public class CloseRegionHandler extends 
       // Check that this region is being served here
       HRegion region = this.rsServices.getFromOnlineRegions(encodedRegionName);
       if (region == null) {
-        LOG.warn("Received CLOSE for region " + name +
-            " but currently not serving");
+        LOG.warn("Received CLOSE for region " + name + " but currently not serving - ignoring");
+        if (zk){
+          LOG.error("The znode is not modified as we are not serving " + name);
+        }
+        // TODO: do better than a simple warning
         return;
       }
 
       // Close the region
       try {
+        if (zk && !ZKAssign.checkClosingState(server.getZooKeeper(), regionInfo, expectedVersion)){
+          // bad znode state
+          return; // We're node deleting the znode, but it's not ours...
+        }
+
         // TODO: If we need to keep updating CLOSING stamp to prevent against
         // a timeout if this is long-running, need to spin up a thread?
         if (region.close(abort) == null) {

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java Mon Dec 31 09:50:27 2012
@@ -85,30 +85,50 @@ public class OpenRegionHandler extends E
 
   @Override
   public void process() throws IOException {
+    boolean openSuccessful = false;
+    final String regionName = regionInfo.getRegionNameAsString();
+
     try {
-      final String name = regionInfo.getRegionNameAsString();
       if (this.server.isStopped() || this.rsServices.isStopping()) {
         return;
       }
       final String encodedName = regionInfo.getEncodedName();
 
+      // 3 different difficult situations can occur
+      // 1) The opening was cancelled. This is an expected situation
+      // 2) The region was hijacked, we no longer have the znode
+      // 3) The region is now marked as online while we're suppose to open. This would be a bug.
+
       // Check that this region is not already online
-      HRegion region = this.rsServices.getFromOnlineRegions(encodedName);
+      if (this.rsServices.getFromOnlineRegions(encodedName) != null) {
+        LOG.error("Region " + encodedName +
+            " was already online when we started processing the opening. " +
+            "Marking this new attempt as failed");
+        tryTransitionFromOfflineToFailedOpen(this.rsServices, regionInfo, this.version);
+        return;
+      }
 
+      // Check that we're still supposed to open the region and transition.
       // If fails, just return.  Someone stole the region from under us.
-      // Calling transitionZookeeperOfflineToOpening initalizes this.version.
-      if (!transitionZookeeperOfflineToOpening(encodedName,
-          versionOfOfflineNode)) {
-        LOG.warn("Region was hijacked? It no longer exists, encodedName=" +
-          encodedName);
+      // Calling transitionZookeeperOfflineToOpening initializes this.version.
+      if (!isRegionStillOpening()){
+        LOG.error("Region " + encodedName + " opening cancelled");
+        tryTransitionFromOfflineToFailedOpen(this.rsServices, regionInfo, this.version);
+        return;
+      }
+
+      if (!transitionZookeeperOfflineToOpening(encodedName, versionOfOfflineNode)) {
+        LOG.warn("Region was hijacked? Opening cancelled for encodedName=" + encodedName);
+        // This is a desperate attempt: the znode is unlikely to be ours. But we can't do more.
+        tryTransitionFromOfflineToFailedOpen(this.rsServices, regionInfo, this.version);
         return;
       }
 
       // Open region.  After a successful open, failures in subsequent
       // processing needs to do a close as part of cleanup.
-      region = openRegion();
+      HRegion region = openRegion();
       if (region == null) {
-        tryTransitionToFailedOpen(regionInfo);
+        tryTransitionFromOpeningToFailedOpen(regionInfo);
         return;
       }
       boolean failed = true;
@@ -120,37 +140,63 @@ public class OpenRegionHandler extends E
       if (failed || this.server.isStopped() ||
           this.rsServices.isStopping()) {
         cleanupFailedOpen(region);
-        tryTransitionToFailedOpen(regionInfo);
+        tryTransitionFromOpeningToFailedOpen(regionInfo);
         return;
       }
 
-      if (!transitionToOpened(region)) {
+
+      if (!isRegionStillOpening() || !transitionToOpened(region)) {
         // If we fail to transition to opened, it's because of one of two cases:
         //    (a) we lost our ZK lease
         // OR (b) someone else opened the region before us
-        // In either case, we don't need to transition to FAILED_OPEN state.
-        // In case (a), the Master will process us as a dead server. In case
-        // (b) the region is already being handled elsewhere anyway.
+        // OR (c) someone cancelled the open
+        // In all cases, we try to transition to failed_open to be safe.
         cleanupFailedOpen(region);
+        tryTransitionFromOpeningToFailedOpen(regionInfo);
         return;
       }
 
-      // One more check to make sure we are opening instead of closing
-      if (!isRegionStillOpening()) {
-        LOG.warn("Open region aborted since it isn't opening any more");
-        cleanupFailedOpen(region);
-        return;
-      }
+      // We have a znode in the opened state now. We can't really delete it as the master job.
+      // Transitioning to failed open would create a race condition if the master has already
+      // acted the transition to opened.
+      // Cancelling the open is dangerous, because we would have a state where the master thinks
+      // the region is opened while the region is actually closed. It is a dangerous state
+      // to be in. For this reason, from now on, we're not going back. There is a message in the
+      // finally close to let the admin knows where we stand.
+
 
       // Successful region open, and add it to OnlineRegions
       this.rsServices.addToOnlineRegions(region);
+      openSuccessful = true;
 
       // Done!  Successful region open
-      LOG.debug("Opened " + name + " on server:" +
+      LOG.debug("Opened " + regionName + " on server:" +
         this.server.getServerName());
+
+
     } finally {
-      this.rsServices.getRegionsInTransitionInRS().
+      final Boolean current = this.rsServices.getRegionsInTransitionInRS().
           remove(this.regionInfo.getEncodedNameAsBytes());
+
+      // Let's check if we have met a race condition on open cancellation....
+      // A better solution would be to not have any race condition.
+      // this.rsServices.getRegionsInTransitionInRS().remove(
+      //  this.regionInfo.getEncodedNameAsBytes(), Boolean.TRUE);
+      // would help, but we would still have a consistency issue to manage with
+      // 1) this.rsServices.addToOnlineRegions(region);
+      // 2) the ZK state.
+      if (openSuccessful) {
+        if (current == null) {  // Should NEVER happen, but let's be paranoid.
+          LOG.error("Bad state: we've just opened a region that was NOT in transition. Region=" +
+              regionName
+          );
+        } else if (Boolean.FALSE.equals(current)) { // Can happen, if we're really unlucky.
+          LOG.error("Race condition: we've finished to open a region, while a close was requested "
+              + " on region=" + regionName + ". It can be a critical error, as a region that" +
+              " should be closed is now opened."
+          );
+        }
+      }
     }
   }
 
@@ -226,7 +272,8 @@ public class OpenRegionHandler extends E
   /**
    * Thread to run region post open tasks. Call {@link #getException()} after
    * the thread finishes to check for exceptions running
-   * {@link RegionServerServices#postOpenDeployTasks(HRegion, org.apache.hadoop.hbase.catalog.CatalogTracker, boolean)}
+   * {@link RegionServerServices#postOpenDeployTasks(
+   * HRegion, org.apache.hadoop.hbase.catalog.CatalogTracker, boolean)}
    * .
    */
   static class PostOpenDeployTasksThread extends Thread {
@@ -277,10 +324,6 @@ public class OpenRegionHandler extends E
    * @throws IOException
    */
   private boolean transitionToOpened(final HRegion r) throws IOException {
-    if (!isRegionStillOpening()) {
-      LOG.warn("Open region aborted since it isn't opening any more");
-      return false;
-    }
     boolean result = false;
     HRegionInfo hri = r.getRegionInfo();
     final String name = hri.getRegionNameAsString();
@@ -310,11 +353,12 @@ public class OpenRegionHandler extends E
    * @param hri Region we're working on.
    * @return whether znode is successfully transitioned to FAILED_OPEN state.
    */
-  private boolean tryTransitionToFailedOpen(final HRegionInfo hri) {
+  private boolean tryTransitionFromOpeningToFailedOpen(final HRegionInfo hri) {
     boolean result = false;
     final String name = hri.getRegionNameAsString();
     try {
-      LOG.info("Opening of region " + hri + " failed, marking as FAILED_OPEN in ZK");
+      LOG.info("Opening of region " + hri + " failed, transitioning" +
+          " from OPENING to FAILED_OPEN in ZK, expecting version " + this.version);
       if (ZKAssign.transitionNode(
           this.server.getZooKeeper(), hri,
           this.server.getServerName(),
@@ -335,6 +379,43 @@ public class OpenRegionHandler extends E
   }
 
   /**
+   * Try to transition to open. This function is static to make it usable before creating the
+   *  handler.
+   *
+   * This is not guaranteed to succeed, we just do our best.
+   *
+   * @param rsServices
+   * @param hri Region we're working on.
+   * @param versionOfOfflineNode version to checked.
+   * @return whether znode is successfully transitioned to FAILED_OPEN state.
+   */
+  public static boolean tryTransitionFromOfflineToFailedOpen(RegionServerServices rsServices,
+       final HRegionInfo hri, final int versionOfOfflineNode) {
+    boolean result = false;
+    final String name = hri.getRegionNameAsString();
+    try {
+      LOG.info("Opening of region " + hri + " failed, transitioning" +
+          " from OFFLINE to FAILED_OPEN in ZK, expecting version " + versionOfOfflineNode);
+      if (ZKAssign.transitionNode(
+          rsServices.getZooKeeper(), hri,
+          rsServices.getServerName(),
+          EventType.M_ZK_REGION_OFFLINE,
+          EventType.RS_ZK_REGION_FAILED_OPEN,
+          versionOfOfflineNode) == -1) {
+        LOG.warn("Unable to mark region " + hri + " as FAILED_OPEN. " +
+            "It's likely that the master already timed out this open " +
+            "attempt, and thus another RS already has the region.");
+      } else {
+        result = true;
+      }
+    } catch (KeeperException e) {
+      LOG.error("Failed transitioning node " + name + " from OFFLINE to FAILED_OPEN", e);
+    }
+    return result;
+  }
+
+
+  /**
    * @return Instance of HRegion if successful open else null.
    */
   HRegion openRegion() {
@@ -379,7 +460,7 @@ public class OpenRegionHandler extends E
   private boolean isRegionStillOpening() {
     byte[] encodedName = regionInfo.getEncodedNameAsBytes();
     Boolean action = rsServices.getRegionsInTransitionInRS().get(encodedName);
-    return action != null && action.booleanValue();
+    return Boolean.TRUE.equals(action); // true means opening for RIT
   }
 
   /**
@@ -392,10 +473,6 @@ public class OpenRegionHandler extends E
    */
   boolean transitionZookeeperOfflineToOpening(final String encodedName,
       int versionOfOfflineNode) {
-    if (!isRegionStillOpening()) {
-      LOG.warn("Open region aborted since it isn't opening any more");
-      return false;
-    }
     // TODO: should also handle transition from CLOSED?
     try {
       // Initialize the znode version.
@@ -405,6 +482,8 @@ public class OpenRegionHandler extends E
     } catch (KeeperException e) {
       LOG.error("Error transition from OFFLINE to OPENING for region=" +
         encodedName, e);
+      this.version = -1;
+      return false;
     }
     boolean b = isGoodVersion();
     if (!b) {
@@ -436,6 +515,7 @@ public class OpenRegionHandler extends E
       server.abort("Exception refreshing OPENING; region=" + encodedName +
         ", context=" + context, e);
       this.version = -1;
+      return false;
     }
     boolean b = isGoodVersion();
     if (!b) {

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKAssign.java Mon Dec 31 09:50:27 2012
@@ -176,7 +176,8 @@ public class ZKAssign {
     LOG.debug(zkw.prefix("Async create of unassigned node for " +
       region.getEncodedName() + " with OFFLINE state"));
     RegionTransition rt =
-      RegionTransition.createRegionTransition(EventType.M_ZK_REGION_OFFLINE, region.getRegionName(), serverName);
+      RegionTransition.createRegionTransition(
+          EventType.M_ZK_REGION_OFFLINE, region.getRegionName(), serverName);
     String node = getNodeName(zkw, region.getEncodedName());
     ZKUtil.asyncCreate(zkw, node, rt.toByteArray(), cb, ctx);
   }
@@ -258,14 +259,14 @@ public class ZKAssign {
    * of the specified regions transition.
    *
    * @param zkw zk reference
-   * @param regionName opened region to be deleted from zk
+   * @param encodedRegionName opened region to be deleted from zk
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteOpenedNode(ZooKeeperWatcher zkw,
-      String regionName)
+      String encodedRegionName)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, regionName, EventType.RS_ZK_REGION_OPENED);
+    return deleteNode(zkw, encodedRegionName, EventType.RS_ZK_REGION_OPENED);
   }
 
   /**
@@ -283,14 +284,14 @@ public class ZKAssign {
    * that has died are all set to OFFLINE before being processed.
    *
    * @param zkw zk reference
-   * @param regionName closed region to be deleted from zk
+   * @param encodedRegionName closed region to be deleted from zk
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteOfflineNode(ZooKeeperWatcher zkw,
-      String regionName)
+      String encodedRegionName)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, regionName, EventType.M_ZK_REGION_OFFLINE);
+    return deleteNode(zkw, encodedRegionName, EventType.M_ZK_REGION_OFFLINE);
   }
 
   /**
@@ -309,14 +310,14 @@ public class ZKAssign {
    * of the specified regions transition to being closed.
    *
    * @param zkw zk reference
-   * @param regionName closed region to be deleted from zk
+   * @param encodedRegionName closed region to be deleted from zk
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
   public static boolean deleteClosedNode(ZooKeeperWatcher zkw,
-      String regionName)
+      String encodedRegionName)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, regionName, EventType.RS_ZK_REGION_CLOSED);
+    return deleteNode(zkw, encodedRegionName, EventType.RS_ZK_REGION_CLOSED);
   }
 
   /**
@@ -342,8 +343,8 @@ public class ZKAssign {
   public static boolean deleteClosingNode(ZooKeeperWatcher zkw,
       HRegionInfo region)
   throws KeeperException, KeeperException.NoNodeException {
-    String regionName = region.getEncodedName();
-    return deleteNode(zkw, regionName, EventType.M_ZK_REGION_CLOSING);
+    String encodedRegionName = region.getEncodedName();
+    return deleteNode(zkw, encodedRegionName, EventType.M_ZK_REGION_CLOSING);
   }
 
   /**
@@ -362,15 +363,15 @@ public class ZKAssign {
    * of the specified regions transition to being closed/opened.
    *
    * @param zkw zk reference
-   * @param regionName region to be deleted from zk
+   * @param encodedRegionName region to be deleted from zk
    * @param expectedState state region must be in for delete to complete
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
-  public static boolean deleteNode(ZooKeeperWatcher zkw, String regionName,
+  public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName,
       EventType expectedState)
   throws KeeperException, KeeperException.NoNodeException {
-    return deleteNode(zkw, regionName, expectedState, -1);
+    return deleteNode(zkw, encodedRegionName, expectedState, -1);
   }
 
   /**
@@ -389,7 +390,7 @@ public class ZKAssign {
    * of the specified regions transition to being closed/opened.
    *
    * @param zkw zk reference
-   * @param regionName region to be deleted from zk
+   * @param encodedRegionName region to be deleted from zk
    * @param expectedState state region must be in for delete to complete
    * @param expectedVersion of the znode that is to be deleted.
    *        If expectedVersion need not be compared while deleting the znode
@@ -397,12 +398,12 @@ public class ZKAssign {
    * @throws KeeperException if unexpected zookeeper exception
    * @throws KeeperException.NoNodeException if node does not exist
    */
-  public static boolean deleteNode(ZooKeeperWatcher zkw, String regionName,
+  public static boolean deleteNode(ZooKeeperWatcher zkw, String encodedRegionName,
       EventType expectedState, int expectedVersion)
   throws KeeperException, KeeperException.NoNodeException {
     LOG.debug(zkw.prefix("Deleting existing unassigned " +
-      "node for " + regionName + " that is in expected state " + expectedState));
-    String node = getNodeName(zkw, regionName);
+      "node for " + encodedRegionName + " that is in expected state " + expectedState));
+    String node = getNodeName(zkw, encodedRegionName);
     zkw.sync(node);
     Stat stat = new Stat();
     byte [] bytes = ZKUtil.getDataNoWatch(zkw, node, stat);
@@ -413,24 +414,24 @@ public class ZKAssign {
     RegionTransition rt = getRegionTransition(bytes);
     EventType et = rt.getEventType();
     if (!et.equals(expectedState)) {
-      LOG.warn(zkw.prefix("Attempting to delete unassigned node " + regionName + " in " +
+      LOG.warn(zkw.prefix("Attempting to delete unassigned node " + encodedRegionName + " in " +
         expectedState + " state but node is in " + et + " state"));
       return false;
     }
     if (expectedVersion != -1
         && stat.getVersion() != expectedVersion) {
-      LOG.warn("The node " + regionName + " we are trying to delete is not" +
+      LOG.warn("The node " + encodedRegionName + " we are trying to delete is not" +
         " the expected one. Got a version mismatch");
       return false;
     }
     if(!ZKUtil.deleteNode(zkw, node, stat.getVersion())) {
       LOG.warn(zkw.prefix("Attempting to delete " +
-          "unassigned node " + regionName + " in " + expectedState +
+          "unassigned node " + encodedRegionName + " in " + expectedState +
           " state but after verifying state, we got a version mismatch"));
       return false;
     }
     LOG.debug(zkw.prefix("Successfully deleted unassigned node for region " +
-        regionName + " in expected state " + expectedState));
+        encodedRegionName + " in expected state " + expectedState));
     return true;
   }
 
@@ -631,6 +632,51 @@ public class ZKAssign {
   }
 
   /**
+   *
+   * @param zkw zk reference
+   * @param region region to be closed
+   * @param expectedVersion expected version of the znode
+   * @return true if the znode exists, has the right version and the right state. False otherwise.
+   * @throws KeeperException
+   */
+  public static boolean checkClosingState(ZooKeeperWatcher zkw, HRegionInfo region,
+                                          int expectedVersion) throws KeeperException {
+
+    final String encoded = getNodeName(zkw, region.getEncodedName());
+    zkw.sync(encoded);
+
+    // Read existing data of the node
+    Stat stat = new Stat();
+    byte[] existingBytes = ZKUtil.getDataNoWatch(zkw, encoded, stat);
+
+    if (existingBytes == null) {
+      LOG.warn(zkw.prefix("Attempt to check the " +
+          "closing node for " + encoded +
+          ". The node does not exist"));
+      return false;
+    }
+
+    if (expectedVersion != -1 && stat.getVersion() != expectedVersion) {
+      LOG.warn(zkw.prefix("Attempt to check the " +
+          "closing node for " + encoded +
+          ". The node existed but was version " + stat.getVersion() +
+          " not the expected version " + expectedVersion));
+      return false;
+    }
+
+    RegionTransition rt = getRegionTransition(existingBytes);
+
+    if (!EventType.M_ZK_REGION_CLOSING.equals(rt.getEventType())) {
+      LOG.warn(zkw.prefix("Attempt to check the " +
+          "closing node for " + encoded +
+          ". The node existed but was in an unexpected state: " + rt.getEventType()));
+      return false;
+    }
+
+    return true;
+  }
+
+  /**
    * Method that actually performs unassigned node transitions.
    *
    * <p>Attempts to transition the unassigned node for the specified region
@@ -722,7 +768,8 @@ public class ZKAssign {
 
     // Write new data, ensuring data has not changed since we last read it
     try {
-      rt = RegionTransition.createRegionTransition(endState, region.getRegionName(), serverName, payload);
+      rt = RegionTransition.createRegionTransition(
+          endState, region.getRegionName(), serverName, payload);
       if(!ZKUtil.setData(zkw, node, rt.toByteArray(), stat.getVersion())) {
         LOG.warn(zkw.prefix("Attempt to transition the " +
         "unassigned node for " + encoded +

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java Mon Dec 31 09:50:27 2012
@@ -1035,7 +1035,13 @@ public class ZKUtil {
       waitForZKConnectionIfAuthenticating(zkw);
       zkw.getRecoverableZooKeeper().create(znode, data, createACL(zkw, znode),
           CreateMode.PERSISTENT);
-      return zkw.getRecoverableZooKeeper().exists(znode, zkw).getVersion();
+      Stat stat = zkw.getRecoverableZooKeeper().exists(znode, zkw);
+      if (stat == null){
+        // Likely a race condition. Someone deleted the znode.
+        throw KeeperException.create(KeeperException.Code.SYSTEMERROR,
+            "ZK.exists returned null (i.e.: znode does not exist) for znode=" + znode);
+      }
+     return stat.getVersion();
     } catch (InterruptedException e) {
       zkw.interruptedException(e);
       return -1;

Modified: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java (original)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java Mon Dec 31 09:50:27 2012
@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.TreeMap;
+import java.util.concurrent.ConcurrentSkipListMap;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
@@ -324,7 +325,7 @@ class MockRegionServer implements AdminP
   }
 
   @Override
-  public Map<byte[], Boolean> getRegionsInTransitionInRS() {
+  public ConcurrentSkipListMap<byte[], Boolean> getRegionsInTransitionInRS() {
     // TODO Auto-generated method stub
     return null;
   }

Added: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java?rev=1427064&view=auto
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java (added)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java Mon Dec 31 09:50:27 2012
@@ -0,0 +1,356 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hbase.regionserver;
+
+import com.google.protobuf.ServiceException;
+import junit.framework.Assert;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.MediumTests;
+import org.apache.hadoop.hbase.NotServingRegionException;
+import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.executor.EventHandler;
+import org.apache.hadoop.hbase.protobuf.RequestConverter;
+import org.apache.hadoop.hbase.protobuf.generated.AdminProtos;
+import org.apache.hadoop.hbase.regionserver.handler.OpenRegionHandler;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.zookeeper.ZKAssign;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+
+/**
+ * Tests on the region server, without the master.
+ */
+@Category(MediumTests.class)
+public class TestRegionServerNoMaster {
+
+  private static final int NB_SERVERS = 1;
+  private static HTable table;
+  private static final byte[] row = "ee".getBytes();
+
+  private static HRegionInfo hri;
+
+  private static byte[] regionName;
+  private static final HBaseTestingUtility HTU = new HBaseTestingUtility();
+
+
+  @BeforeClass
+  public static void before() throws Exception {
+    HTU.startMiniCluster(NB_SERVERS);
+    final byte[] tableName = Bytes.toBytes(TestRegionServerNoMaster.class.getName());
+
+    // Create table then get the single region for our new table.
+    table = HTU.createTable(tableName, HConstants.CATALOG_FAMILY);
+    Put p = new Put(row);
+    p.add(HConstants.CATALOG_FAMILY, row, row);
+    table.put(p);
+
+    hri = table.getRegionLocation(row, false).getRegionInfo();
+    regionName = hri.getRegionName();
+
+    // No master
+    HTU.getHBaseCluster().getMaster().stopMaster();
+  }
+
+  @AfterClass
+  public static void afterClass() throws Exception {
+    table.close();
+    HTU.shutdownMiniCluster();
+  }
+
+  @After
+  public void after() throws Exception {
+    // Clean the state if the test failed before cleaning the znode
+    // It does not manage all bad failures, so if there are multiple failures, only
+    //  the first one should be looked at.
+    ZKAssign.deleteNodeFailSilent(HTU.getZooKeeperWatcher(), hri);
+  }
+
+
+  private static HRegionServer getRS() {
+    return HTU.getHBaseCluster().getLiveRegionServerThreads().get(0).getRegionServer();
+  }
+
+
+  /**
+   * Reopen the region. Reused in multiple tests as we always leave the region open after a test.
+   */
+  private void reopenRegion() throws Exception {
+    // We reopen. We need a ZK node here, as a open is always triggered by a master.
+    ZKAssign.createNodeOffline(HTU.getZooKeeperWatcher(), hri, getRS().getServerName());
+    // first version is '0'
+    AdminProtos.OpenRegionRequest orr = RequestConverter.buildOpenRegionRequest(hri, 0);
+    AdminProtos.OpenRegionResponse responseOpen = getRS().openRegion(null, orr);
+    Assert.assertTrue(responseOpen.getOpeningStateCount() == 1);
+    Assert.assertTrue(responseOpen.getOpeningState(0).
+        equals(AdminProtos.OpenRegionResponse.RegionOpeningState.OPENED));
+
+
+    checkRegionIsOpened();
+  }
+
+  private void checkRegionIsOpened() throws Exception {
+
+    while (!getRS().getRegionsInTransitionInRS().isEmpty()) {
+      Thread.sleep(1);
+    }
+
+    Assert.assertTrue(getRS().getRegion(regionName).isAvailable());
+
+    Assert.assertTrue(
+        ZKAssign.deleteOpenedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName()));
+  }
+
+
+  private void checkRegionIsClosed() throws Exception {
+
+    while (!getRS().getRegionsInTransitionInRS().isEmpty()) {
+      Thread.sleep(1);
+    }
+
+    try {
+      Assert.assertFalse(getRS().getRegion(regionName).isAvailable());
+    } catch (NotServingRegionException expected) {
+      // That's how it work: if the region is closed we have an exception.
+    }
+
+    // We don't delete the znode here, because there is not always a znode.
+  }
+
+
+  /**
+   * Close the region without using ZK
+   */
+  private void closeNoZK() throws Exception {
+    // no transition in ZK
+    AdminProtos.CloseRegionRequest crr =
+        RequestConverter.buildCloseRegionRequest(regionName, false);
+    AdminProtos.CloseRegionResponse responseClose = getRS().closeRegion(null, crr);
+    Assert.assertTrue(responseClose.getClosed());
+
+    // now waiting & checking. After a while, the transition should be done and the region closed
+    checkRegionIsClosed();
+  }
+
+
+  @Test(timeout = 20000)
+  public void testCloseByRegionServer() throws Exception {
+    closeNoZK();
+    reopenRegion();
+  }
+
+  @Test(timeout = 20000)
+  public void testCloseByMasterWithoutZNode() throws Exception {
+
+    // Transition in ZK on. This should fail, as there is no znode
+    AdminProtos.CloseRegionRequest crr = RequestConverter.buildCloseRegionRequest(
+        regionName, true);
+    AdminProtos.CloseRegionResponse responseClose = getRS().closeRegion(null, crr);
+    Assert.assertTrue(responseClose.getClosed());
+
+    // now waiting. After a while, the transition should be done
+    while (!getRS().getRegionsInTransitionInRS().isEmpty()) {
+      Thread.sleep(1);
+    }
+
+    // the region is still available, the close got rejected at the end
+    Assert.assertTrue("The close should have failed", getRS().getRegion(regionName).isAvailable());
+  }
+
+  @Test(timeout = 20000)
+  public void testOpenCloseByMasterWithZNode() throws Exception {
+
+    ZKAssign.createNodeClosing(HTU.getZooKeeperWatcher(), hri, getRS().getServerName());
+
+    AdminProtos.CloseRegionRequest crr = RequestConverter.buildCloseRegionRequest(
+        regionName, true);
+    AdminProtos.CloseRegionResponse responseClose = getRS().closeRegion(null, crr);
+    Assert.assertTrue(responseClose.getClosed());
+
+    checkRegionIsClosed();
+
+    ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName());
+
+    reopenRegion();
+  }
+
+  /**
+   * Test that we can send multiple openRegion to the region server.
+   * This is used when:
+   * - there is a SocketTimeout: in this case, the master does not know if the region server
+   * received the request before the timeout.
+   * - We have a socket error during the operation: same stuff: we don't know
+   * - a master failover: if we find a znode in thz M_ZK_REGION_OFFLINE, we don't know if
+   * the region server has received the query or not. Only solution to be efficient: re-ask
+   * immediately.
+   */
+  @Test(timeout = 20000)
+  public void testMultipleOpen() throws Exception {
+
+    // We close
+    closeNoZK();
+    checkRegionIsClosed();
+
+    // We reopen. We need a ZK node here, as a open is always triggered by a master.
+    ZKAssign.createNodeOffline(HTU.getZooKeeperWatcher(), hri, getRS().getServerName());
+
+    // We're sending multiple requests in a row. The region server must handle this nicely.
+    for (int i = 0; i < 10; i++) {
+      AdminProtos.OpenRegionRequest orr = RequestConverter.buildOpenRegionRequest(hri, 0);
+      AdminProtos.OpenRegionResponse responseOpen = getRS().openRegion(null, orr);
+      Assert.assertTrue(responseOpen.getOpeningStateCount() == 1);
+
+      AdminProtos.OpenRegionResponse.RegionOpeningState ors = responseOpen.getOpeningState(0);
+      Assert.assertTrue("request " + i + " failed",
+          ors.equals(AdminProtos.OpenRegionResponse.RegionOpeningState.OPENED) ||
+              ors.equals(AdminProtos.OpenRegionResponse.RegionOpeningState.ALREADY_OPENED)
+      );
+    }
+
+    checkRegionIsOpened();
+  }
+
+  @Test(timeout = 20000)
+  public void testMultipleCloseFromMaster() throws Exception {
+
+    // As opening, we must support multiple requests on the same region
+    ZKAssign.createNodeClosing(HTU.getZooKeeperWatcher(), hri, getRS().getServerName());
+    for (int i = 0; i < 10; i++) {
+      AdminProtos.CloseRegionRequest crr =
+          RequestConverter.buildCloseRegionRequest(regionName, 0, null, true);
+      try {
+        AdminProtos.CloseRegionResponse responseClose = getRS().closeRegion(null, crr);
+        Assert.assertEquals("The first request should succeeds", 0, i);
+        Assert.assertTrue("request " + i + " failed",
+            responseClose.getClosed() || responseClose.hasClosed());
+      } catch (ServiceException se) {
+        Assert.assertTrue("The next queries should throw an exception.", i > 0);
+      }
+    }
+
+    checkRegionIsClosed();
+
+    Assert.assertTrue(
+        ZKAssign.deleteClosedNode(HTU.getZooKeeperWatcher(), hri.getEncodedName())
+    );
+
+    reopenRegion();
+  }
+
+  /**
+   * Test that if we do a close while opening it stops the opening.
+   */
+  @Test(timeout = 20000)
+  public void testCancelOpeningWithoutZK() throws Exception {
+    // We close
+    closeNoZK();
+    checkRegionIsClosed();
+
+    // Let do the initial steps, without having a handler
+    ZKAssign.createNodeOffline(HTU.getZooKeeperWatcher(), hri, getRS().getServerName());
+    getRS().getRegionsInTransitionInRS().put(hri.getEncodedNameAsBytes(), Boolean.TRUE);
+
+    // That's a close without ZK.
+    AdminProtos.CloseRegionRequest crr =
+        RequestConverter.buildCloseRegionRequest(regionName, false);
+    try {
+      getRS().closeRegion(null, crr);
+      Assert.assertTrue(false);
+    } catch (ServiceException expected) {
+    }
+
+    // The state in RIT should have changed to close
+    Assert.assertEquals(Boolean.FALSE, getRS().getRegionsInTransitionInRS().get(
+        hri.getEncodedNameAsBytes()));
+
+    // Let's start the open handler
+    HTableDescriptor htd = getRS().tableDescriptors.get(hri.getTableName());
+    getRS().service.submit(new OpenRegionHandler(getRS(), getRS(), hri, htd, 0));
+
+    // The open handler should have removed the region from RIT but kept the region closed
+    checkRegionIsClosed();
+
+    // The open handler should have updated the value in ZK.
+    Assert.assertTrue(ZKAssign.deleteNode(
+        getRS().getZooKeeperWatcher(), hri.getEncodedName(),
+        EventHandler.EventType.RS_ZK_REGION_FAILED_OPEN, 1)
+    );
+
+    reopenRegion();
+  }
+
+  /**
+   * Test an open then a close with ZK. This is going to mess-up the ZK states, so
+   * the opening will fail as well because it doesn't find what it expects in ZK.
+   */
+  @Test(timeout = 20000)
+  public void testCancelOpeningWithZK() throws Exception {
+    // We close
+    closeNoZK();
+    checkRegionIsClosed();
+
+    // Let do the initial steps, without having a handler
+    getRS().getRegionsInTransitionInRS().put(hri.getEncodedNameAsBytes(), Boolean.TRUE);
+
+    // That's a close without ZK.
+    ZKAssign.createNodeClosing(HTU.getZooKeeperWatcher(), hri, getRS().getServerName());
+    AdminProtos.CloseRegionRequest crr =
+        RequestConverter.buildCloseRegionRequest(regionName, false);
+    try {
+      getRS().closeRegion(null, crr);
+      Assert.assertTrue(false);
+    } catch (ServiceException expected) {
+      Assert.assertTrue(expected.getCause() instanceof NotServingRegionException);
+    }
+
+    // The close should have left the ZK state as it is: it's the job the AM to delete it
+    Assert.assertTrue(ZKAssign.deleteNode(
+        getRS().getZooKeeperWatcher(), hri.getEncodedName(),
+        EventHandler.EventType.M_ZK_REGION_CLOSING, 0)
+    );
+
+    // The state in RIT should have changed to close
+    Assert.assertEquals(Boolean.FALSE, getRS().getRegionsInTransitionInRS().get(
+        hri.getEncodedNameAsBytes()));
+
+    // Let's start the open handler
+    // It should not succeed for two reasons:
+    //  1) There is no ZK node
+    //  2) The region in RIT was changed.
+    // The order is more or less implementation dependant.
+    HTableDescriptor htd = getRS().tableDescriptors.get(hri.getTableName());
+    getRS().service.submit(new OpenRegionHandler(getRS(), getRS(), hri, htd, 0));
+
+    // The open handler should have removed the region from RIT but kept the region closed
+    checkRegionIsClosed();
+
+    // We should not find any znode here.
+    Assert.assertEquals(-1, ZKAssign.getVersion(HTU.getZooKeeperWatcher(), hri));
+
+    reopenRegion();
+  }
+}

Modified: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java (original)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java Mon Dec 31 09:50:27 2012
@@ -88,7 +88,7 @@ public class TestOpenRegionHandler {
   @Test public void testYankingRegionFromUnderIt()
   throws IOException, NodeExistsException, KeeperException {
     final Server server = new MockServer(HTU);
-    final RegionServerServices rss = new MockRegionServerServices();
+    final RegionServerServices rss = new MockRegionServerServices(HTU.getZooKeeperWatcher());
 
     HTableDescriptor htd = TEST_HTD;
     final HRegionInfo hri = TEST_HRI;

Modified: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java?rev=1427064&r1=1427063&r2=1427064&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java (original)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/MockRegionServerServices.java Mon Dec 31 09:50:27 2012
@@ -36,7 +36,6 @@ import org.apache.hadoop.hbase.regionser
 import org.apache.hadoop.hbase.regionserver.RegionServerAccounting;
 import org.apache.hadoop.hbase.regionserver.RegionServerServices;
 import org.apache.hadoop.hbase.regionserver.wal.HLog;
-import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher;
 import org.apache.zookeeper.KeeperException;
 
@@ -49,6 +48,15 @@ public class MockRegionServerServices im
   private final ConcurrentSkipListMap<byte[], Boolean> rit = 
     new ConcurrentSkipListMap<byte[], Boolean>(Bytes.BYTES_COMPARATOR);
   private HFileSystem hfs = null;
+  private ZooKeeperWatcher zkw = null;
+
+  public MockRegionServerServices(ZooKeeperWatcher zkw){
+    this.zkw = zkw;
+  }
+
+  public MockRegionServerServices(){
+    this(null);
+  }
 
   @Override
   public boolean removeFromOnlineRegions(String encodedRegionName, ServerName destination) {
@@ -112,7 +120,7 @@ public class MockRegionServerServices im
 
   @Override
   public ZooKeeperWatcher getZooKeeper() {
-    return null;
+    return zkw;
   }
   
   public RegionServerAccounting getRegionServerAccounting() {



Mime
View raw message