From 9a528fbc1715ceed8b7bbbaf786107a499e71065 Mon Sep 17 00:00:00 2001 From: Enis Soztutar Date: Sat, 23 Mar 2013 01:47:17 +0000 Subject: [PATCH] HBASE-7848 Use ZK-based read/write lock to make flush-type snapshot robust against table enable/disable/alter git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1460074 13f79535-47bb-0310-9956-ffa450edef68 --- .../DisabledTableSnapshotHandler.java | 5 +++ .../snapshot/EnabledTableSnapshotHandler.java | 7 +++- .../master/snapshot/SnapshotManager.java | 8 ++--- .../master/snapshot/TakeSnapshotHandler.java | 32 +++++++++++++++++-- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java index 7d2c50c1e5a..facef5690d1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/DisabledTableSnapshotHandler.java @@ -69,6 +69,11 @@ public class DisabledTableSnapshotHandler extends TakeSnapshotHandler { 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 diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java index 5a484537b4f..537fbfbfc14 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/EnabledTableSnapshotHandler.java @@ -28,11 +28,11 @@ import org.apache.hadoop.classification.InterfaceAudience; 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 extends TakeSnapshotHandler { 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. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index d6ae0dc8bc8..2aeb5f968b8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -428,10 +428,10 @@ public class SnapshotManager implements Stoppable { 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 Stoppable { 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 { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java index bc84ff852b7..e30825f0310 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java @@ -37,13 +37,15 @@ import org.apache.hadoop.hbase.catalog.MetaReader; 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.KeeperException; * 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 TakeSnapshotHandler extends EventHandler implements Snapsh 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 TakeSnapshotHandler extends EventHandler implements Snapsh 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 TakeSnapshotHandler extends EventHandler implements Snapsh 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 TakeSnapshotHandler extends EventHandler implements Snapsh } 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); + } } }