HBASE-5359 Alter in the shell can be too quick and return before the table is altered

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1309611 13f79535-47bb-0310-9956-ffa450edef68

src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

@@ -305,7 +305,8 @@ public class AssignmentManager extends ZooKeeperListener {
       MetaReader.getTableRegions(this.master.getCatalogTracker(), tableName);
     Integer pending = 0;
     for(HRegionInfo hri : hris) {
-      if(regionsToReopen.get(hri.getEncodedName()) != null) {
+      String name = hri.getEncodedName();
+      if (regionsToReopen.containsKey(name) || regionsInTransition.containsKey(name)) {
         pending++;
       }
     }

src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -88,6 +88,7 @@ import org.apache.hadoop.hbase.master.handler.ModifyTableHandler;
 import org.apache.hadoop.hbase.master.handler.ServerShutdownHandler;
 import org.apache.hadoop.hbase.master.handler.TableAddFamilyHandler;
 import org.apache.hadoop.hbase.master.handler.TableDeleteFamilyHandler;
+import org.apache.hadoop.hbase.master.handler.TableEventHandler;
 import org.apache.hadoop.hbase.master.handler.TableModifyFamilyHandler;
 import org.apache.hadoop.hbase.master.metrics.MasterMetrics;
 import org.apache.hadoop.hbase.monitoring.MemoryBoundedLogMessageBuffer;
@@ -1314,6 +1315,10 @@ Server {
    */
   public Pair<Integer, Integer> getAlterStatus(byte[] tableName)
   throws IOException {
+    // TODO: currently, we query using the table name on the client side. this
+    // may overlap with other table operations or the table operation may
+    // have completed before querying this API. We need to refactor to a
+    // transaction system in the future to avoid these ambiguities.
     if (supportInstantSchemaChanges) {
       return getAlterStatusFromSchemaChangeTracker(tableName);
     }
@@ -1465,8 +1470,11 @@ Server {
     if (cpHost != null) {
       cpHost.preModifyTable(tableName, htd);
     }
-    this.executorService.submit(new ModifyTableHandler(tableName, htd, this,
+    TableEventHandler tblHandle = new ModifyTableHandler(tableName, htd, this,
-      this, this, supportInstantSchemaChanges));
+        this, this, supportInstantSchemaChanges);
+    this.executorService.submit(tblHandle);
+    tblHandle.waitForPersist();
+
     if (cpHost != null) {
       cpHost.postModifyTable(tableName, htd);
     }

src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java

@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master.handler;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
@@ -69,6 +70,7 @@ public abstract class TableEventHandler extends EventHandler {
   protected final byte [] tableName;
   protected final String tableNameStr;
   protected boolean instantAction = false;
+  protected boolean persistedToZk = false;
 
   public TableEventHandler(EventType eventType, byte [] tableName, Server server,
       MasterServices masterServices, HMasterInterface masterInterface,
@@ -97,6 +99,9 @@ public abstract class TableEventHandler extends EventHandler {
       LOG.error("Error manipulating table " + Bytes.toString(tableName), e);
     } catch (KeeperException e) {
       LOG.error("Error manipulating table " + Bytes.toString(tableName), e);
+    } finally {
+      // notify the waiting thread that we're done persisting the request
+      setPersist();
     }
   }
 
@@ -124,6 +129,7 @@ public abstract class TableEventHandler extends EventHandler {
       throws IOException {
     if (canPerformSchemaChange()) {
       this.masterServices.getAssignmentManager().setRegionsToReopen(regions);
+      setPersist();
       if (reOpenAllRegions(regions)) {
         LOG.info("Completed table operation " + eventType + " on table " +
             Bytes.toString(tableName));
@@ -208,6 +214,30 @@ public abstract class TableEventHandler extends EventHandler {
     return false;
   }
 
+  /**
+   * Table modifications are processed asynchronously, but provide an API for
+   * you to query their status.
+   *
+   * @throws IOException
+   */
+  public synchronized void waitForPersist() throws IOException {
+    if (!persistedToZk) {
+      try {
+        wait();
+      } catch (InterruptedException ie) {
+        throw (IOException) new InterruptedIOException().initCause(ie);
+      }
+      assert persistedToZk;
+    }
+  }
+
+  private synchronized void setPersist() {
+    if (!persistedToZk) {
+      persistedToZk = true;
+      notify();
+    }
+  }
+
   /**
    * Wait for region split transaction in progress (if any)
    * @param regions