You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ns...@apache.org on 2012/04/04 23:53:12 UTC

svn commit: r1309611 - in /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master: AssignmentManager.java HMaster.java handler/TableEventHandler.java

Author: nspiegelberg
Date: Wed Apr  4 21:53:12 2012
New Revision: 1309611

URL: http://svn.apache.org/viewvc?rev=1309611&view=rev
Log:
HBASE-5359 Alter in the shell can be too quick and return before the table is altered

Modified:
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java?rev=1309611&r1=1309610&r2=1309611&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java Wed Apr  4 21:53:12 2012
@@ -305,7 +305,8 @@ public class AssignmentManager extends Z
       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++;
       }
     }

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java?rev=1309611&r1=1309610&r2=1309611&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Wed Apr  4 21:53:12 2012
@@ -88,6 +88,7 @@ import org.apache.hadoop.hbase.master.ha
 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,
-      this, this, supportInstantSchemaChanges));
+    TableEventHandler tblHandle = new ModifyTableHandler(tableName, htd, this,
+        this, this, supportInstantSchemaChanges);
+    this.executorService.submit(tblHandle);
+    tblHandle.waitForPersist();
+
     if (cpHost != null) {
       cpHost.postModifyTable(tableName, htd);
     }

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java?rev=1309611&r1=1309610&r2=1309611&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java Wed Apr  4 21:53:12 2012
@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master.h
 
 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 
   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 
       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 
       throws IOException {
     if (canPerformSchemaChange()) {
       this.masterServices.getAssignmentManager().setRegionsToReopen(regions);
+      setPersist();
       if (reOpenAllRegions(regions)) {
         LOG.info("Completed table operation " + eventType + " on table " +
             Bytes.toString(tableName));
@@ -209,6 +215,30 @@ public abstract class TableEventHandler 
   }
 
   /**
+   * 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
    * @param status