Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1460074 - in
 /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot:
 DisabledTableSnapshotHandler.java EnabledTableSnapshotHandler.java
 SnapshotManager.java TakeSnapshotHandler.java
Date: Sat, 23 Mar 2013 01:47:17 -0000
To: commits@hbase.apache.org
From: enis@apache.org
Message-Id: <20130323014717.9E63523888CD@eris.apache.org>

Author: enis
Date: Sat Mar 23 01:47:17 2013
New Revision: 1460074

URL: http://svn.apache.org/r1460074
Log:
HBASE-7848 Use ZK-based read/write lock to make flush-type snapshot robust against table enable/disable/alter

Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java?rev=1460074&r1=1460073&r2=1460074&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java Sat Mar 23 01:47:17 2013
@@ -69,6 +69,11 @@ public class DisabledTableSnapshotHandle
     timeoutInjector = TakeSnapshotUtils.getMasterTimerAndBindToMonitor(snapshot, conf, monitor);
   }
 
+  @Override
+  public DisabledTableSnapshotHandler prepare() throws Exception {
+    return (DisabledTableSnapshotHandler) super.prepare();
+  }
+
   // TODO consider parallelizing these operations since they are independent. Right now its just
   // easier to keep them serial though
   @Override

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java?rev=1460074&r1=1460073&r2=1460074&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java Sat Mar 23 01:47:17 2013
@@ -28,11 +28,11 @@ import org.apache.hadoop.classification.
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.errorhandling.ForeignException;
+import org.apache.hadoop.hbase.exceptions.HBaseSnapshotException;
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.procedure.Procedure;
 import org.apache.hadoop.hbase.procedure.ProcedureCoordinator;
 import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.SnapshotDescription;
-import org.apache.hadoop.hbase.exceptions.HBaseSnapshotException;
 import org.apache.hadoop.hbase.util.Pair;
 
 import com.google.common.collect.Lists;
@@ -54,6 +54,11 @@ public class EnabledTableSnapshotHandler
     this.coordinator = manager.getCoordinator();
   }
 
+  @Override
+  public EnabledTableSnapshotHandler prepare() throws Exception {
+    return (EnabledTableSnapshotHandler) super.prepare();
+  }
+
   // TODO consider switching over to using regionnames, rather than server names. This would allow
   // regions to migrate during a snapshot, and then be involved when they are ready. Still want to
   // enforce a snapshot time constraints, but lets us be potentially a bit more robust.

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java?rev=1460074&r1=1460073&r2=1460074&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java Sat Mar 23 01:47:17 2013
@@ -428,10 +428,10 @@ public class SnapshotManager implements 
       throws HBaseSnapshotException {
     TakeSnapshotHandler handler;
     try {
-      handler = new EnabledTableSnapshotHandler(snapshot, master, this);
+      handler = new EnabledTableSnapshotHandler(snapshot, master, this).prepare();
       this.executorService.submit(handler);
       this.handler = handler;
-    } catch (IOException e) {
+    } catch (Exception e) {
       // cleanup the working directory by trying to delete it from the fs.
       Path workingDir = SnapshotDescriptionUtils.getWorkingSnapshotDir(snapshot, rootDir);
       try {
@@ -537,10 +537,10 @@ public class SnapshotManager implements 
 
     DisabledTableSnapshotHandler handler;
     try {
-      handler = new DisabledTableSnapshotHandler(snapshot, this.master);
+      handler = new DisabledTableSnapshotHandler(snapshot, this.master).prepare();
       this.executorService.submit(handler);
       this.handler = handler;
-    } catch (IOException e) {
+    } catch (Exception e) {
       // cleanup the working directory by trying to delete it from the fs.
       Path workingDir = SnapshotDescriptionUtils.getWorkingSnapshotDir(snapshot, rootDir);
       try {

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java?rev=1460074&r1=1460073&r2=1460074&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java Sat Mar 23 01:47:17 2013
@@ -37,13 +37,15 @@ import org.apache.hadoop.hbase.catalog.M
 import org.apache.hadoop.hbase.errorhandling.ForeignException;
 import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher;
 import org.apache.hadoop.hbase.errorhandling.ForeignExceptionSnare;
+import org.apache.hadoop.hbase.exceptions.SnapshotCreationException;
 import org.apache.hadoop.hbase.executor.EventHandler;
 import org.apache.hadoop.hbase.executor.EventType;
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.SnapshotSentinel;
+import org.apache.hadoop.hbase.master.TableLockManager;
+import org.apache.hadoop.hbase.master.TableLockManager.TableLock;
 import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.SnapshotDescription;
 import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
-import org.apache.hadoop.hbase.exceptions.SnapshotCreationException;
 import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
 import org.apache.hadoop.hbase.snapshot.TableInfoCopyTask;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -54,7 +56,7 @@ import org.apache.zookeeper.KeeperExcept
  * A handler for taking snapshots from the master.
  *
  * This is not a subclass of TableEventHandler because using that would incur an extra META scan.
- * 
+ *
  * The {@link #snapshotRegions(List)} call should get implemented for each snapshot flavor.
  */
 @InterfaceAudience.Private
@@ -74,6 +76,8 @@ public abstract class TakeSnapshotHandle
   protected final Path workingDir;
   private final MasterSnapshotVerifier verifier;
   protected final ForeignExceptionDispatcher monitor;
+  protected final TableLockManager tableLockManager;
+  protected final TableLock tableLock;
 
   /**
    * @param snapshot descriptor of the snapshot to take
@@ -95,7 +99,9 @@ public abstract class TakeSnapshotHandle
     this.workingDir = SnapshotDescriptionUtils.getWorkingSnapshotDir(snapshot, rootDir);
     this.monitor =  new ForeignExceptionDispatcher();
 
-    loadTableDescriptor(); // check that .tableinfo is present
+    this.tableLockManager = master.getTableLockManager();
+    this.tableLock = this.tableLockManager.writeLock(Bytes.toBytes(snapshot.getTable())
+      , EventType.C_M_SNAPSHOT_TABLE.toString());
 
     // prepare the verify
     this.verifier = new MasterSnapshotVerifier(masterServices, snapshot, rootDir);
@@ -112,6 +118,15 @@ public abstract class TakeSnapshotHandle
     return htd;
   }
 
+  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
+    return this;
+  }
+
   /**
    * Execute the core common portions of taking a snapshot. The {@link #snapshotRegions(List)}
    * call should get implemented for each snapshot flavor.
@@ -166,6 +181,17 @@ public abstract class TakeSnapshotHandle
       } catch (IOException e) {
         LOG.error("Couldn't delete snapshot working directory:" + workingDir);
       }
+      releaseTableLock();
+    }
+  }
+
+  protected void releaseTableLock() {
+    if (this.tableLock != null) {
+      try {
+        this.tableLock.release();
+      } catch (IOException ex) {
+        LOG.warn("Could not release the table lock", ex);
+      }
     }
   }