svn commit: r1468263 - in /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master: handler/TableEventHandler.java snapshot/RestoreSnapshotHandler.java snapshot/SnapshotManager.java snapshot/TakeSnapshotHandler.java

[en...@apache.org]

Author: enis
Date: Mon Apr 15 22:50:17 2013
New Revision: 1468263

URL: http://svn.apache.org/r1468263
Log:
HBASE-8341 RestoreSnapshotHandler.prepare() is not called by SnapshotManager and TakeSnapshotHandler should first acquire the table lock

Modified:
    hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java
    hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
    hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
    hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java

Modified: hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java?rev=1468263&r1=1468262&r2=1468263&view=diff
==============================================================================
--- hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java (original)
+++ hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/TableEventHandler.java Mon Apr 15 22:50:17 2013
@@ -31,13 +31,13 @@ import org.apache.commons.logging.LogFac
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.HTableDescriptor;
-import org.apache.hadoop.hbase.exceptions.InvalidFamilyOperationException;
 import org.apache.hadoop.hbase.Server;
 import org.apache.hadoop.hbase.ServerName;
-import org.apache.hadoop.hbase.exceptions.TableExistsException;
-import org.apache.hadoop.hbase.exceptions.TableNotDisabledException;
 import org.apache.hadoop.hbase.catalog.MetaReader;
 import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.exceptions.InvalidFamilyOperationException;
+import org.apache.hadoop.hbase.exceptions.TableExistsException;
+import org.apache.hadoop.hbase.exceptions.TableNotDisabledException;
 import org.apache.hadoop.hbase.executor.EventHandler;
 import org.apache.hadoop.hbase.executor.EventType;
 import org.apache.hadoop.hbase.master.BulkReOpen;
@@ -63,6 +63,7 @@ public abstract class TableEventHandler 
   protected final byte [] tableName;
   protected final String tableNameStr;
   protected TableLock tableLock;
+  private boolean isPrepareCalled = false;
 
   public TableEventHandler(EventType eventType, byte [] tableName, Server server,
       MasterServices masterServices) {
@@ -97,6 +98,7 @@ public abstract class TableEventHandler 
         releaseTableLock();
       }
     }
+    this.isPrepareCalled = true;
     return this;
   }
 
@@ -113,6 +115,11 @@ public abstract class TableEventHandler 
 
   @Override
   public void process() {
+    if (!isPrepareCalled) {
+      //For proper table locking semantics, the implementor should ensure to call
+      //TableEventHandler.prepare() before calling process()
+      throw new RuntimeException("Implementation should have called prepare() first");
+    }
     try {
       LOG.info("Handling table operation " + eventType + " on table " +
           Bytes.toString(tableName));

Modified: hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java?rev=1468263&r1=1468262&r2=1468263&view=diff
==============================================================================
--- hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java (original)
+++ hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java Mon Apr 15 22:50:17 2013
@@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.catalog.C
 import org.apache.hadoop.hbase.catalog.MetaEditor;
 import org.apache.hadoop.hbase.errorhandling.ForeignException;
 import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher;
+import org.apache.hadoop.hbase.exceptions.RestoreSnapshotException;
 import org.apache.hadoop.hbase.executor.EventType;
 import org.apache.hadoop.hbase.master.MasterFileSystem;
 import org.apache.hadoop.hbase.master.MasterServices;
@@ -44,7 +45,6 @@ import org.apache.hadoop.hbase.monitorin
 import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.SnapshotDescription;
 import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
-import org.apache.hadoop.hbase.exceptions.RestoreSnapshotException;
 import org.apache.hadoop.hbase.snapshot.RestoreSnapshotHelper;
 import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -91,6 +91,10 @@ public class RestoreSnapshotHandler exte
           + hTableDescriptor.getNameAsString());
   }
 
+  public RestoreSnapshotHandler prepare() throws IOException {
+    return (RestoreSnapshotHandler) super.prepare();
+  }
+
   /**
    * The restore table is executed in place.
    *  - The on-disk data will be restored - reference files are put in place without moving data

Modified: hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java?rev=1468263&r1=1468262&r2=1468263&view=diff
==============================================================================
--- hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java (original)
+++ hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java Mon Apr 15 22:50:17 2013
@@ -706,6 +706,8 @@ public class SnapshotManager implements 
       final HTableDescriptor hTableDescriptor) throws HBaseSnapshotException {
     String tableName = hTableDescriptor.getNameAsString();
 
+    // TODO: There is definite race condition for managing the single handler. We should fix
+    // and remove the limitation of single snapshot / restore at a time.
     // make sure we aren't running a snapshot on the same table
     if (isTakingSnapshot(tableName)) {
       throw new RestoreSnapshotException("Snapshot in progress on the restore table=" + tableName);
@@ -718,7 +720,7 @@ public class SnapshotManager implements 
 
     try {
       RestoreSnapshotHandler handler =
-        new RestoreSnapshotHandler(master, snapshot, hTableDescriptor, metricsMaster);
+        new RestoreSnapshotHandler(master, snapshot, hTableDescriptor, metricsMaster).prepare();
       this.executorService.submit(handler);
       restoreHandlers.put(hTableDescriptor.getNameAsString(), handler);
     } catch (Exception e) {

Modified: hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java?rev=1468263&r1=1468262&r2=1468263&view=diff
==============================================================================
--- hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java (original)
+++ hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java Mon Apr 15 22:50:17 2013
@@ -43,10 +43,10 @@ import org.apache.hadoop.hbase.executor.
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.MetricsMaster;
 import org.apache.hadoop.hbase.master.SnapshotSentinel;
-import org.apache.hadoop.hbase.monitoring.MonitoredTask;
-import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.master.TableLockManager;
 import org.apache.hadoop.hbase.master.TableLockManager.TableLock;
+import org.apache.hadoop.hbase.monitoring.MonitoredTask;
+import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.SnapshotDescription;
 import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
 import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
@@ -129,10 +129,18 @@ public abstract class TakeSnapshotHandle
 
   public TakeSnapshotHandler prepare() throws Exception {
     super.prepare();
-    loadTableDescriptor(); // check that .tableinfo is present
+    this.tableLock.acquire(); // after this, you should ensure to release this lock in
+                              // case of exceptions
+    boolean success = false;
+    try {
+      loadTableDescriptor(); // check that .tableinfo is present
+      success = true;
+    } finally {
+      if (!success) {
+        releaseTableLock();
+      }
+    }
 
-    this.tableLock.acquire(); //after this, you should ensure to release this lock in
-                              //case of exceptions
     return this;
   }