From e55da9814fc6404d1e9a3f6f88c4a611bd25edcc Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Thu, 12 Jan 2017 12:58:34 -0800 Subject: [PATCH 2/2] HBASE-16786 Procedure V2 - Move ZK-lock's uses to Procedure framework locks (LockProcedure) - Matteo Bertozzi Locks are no longer hosted up in zookeeper but instead by the Master. --- .../apache/hadoop/hbase/procedure2/Procedure.java | 8 +- .../hbase/procedure2/ProcedureTestingUtility.java | 4 +- .../hadoop/hbase/rsgroup/RSGroupAdminServer.java | 14 +- .../hadoop/hbase/client/locking/EntityLock.java | 2 + .../hadoop/hbase/master/AssignmentManager.java | 8 +- .../hbase/master/ExpiredMobFileCleanerChore.java | 36 +- .../org/apache/hadoop/hbase/master/HMaster.java | 8 +- .../hbase/master/MasterMobCompactionThread.java | 19 +- .../apache/hadoop/hbase/master/MasterServices.java | 5 - .../hadoop/hbase/master/MobCompactionChore.java | 9 +- .../hadoop/hbase/master/TableLockManager.java | 453 --------------------- .../hadoop/hbase/master/locking/LockProcedure.java | 5 +- .../AbstractStateMachineNamespaceProcedure.java | 4 +- .../AbstractStateMachineTableProcedure.java | 4 +- .../master/procedure/CreateNamespaceProcedure.java | 2 +- .../master/procedure/CreateTableProcedure.java | 2 +- .../procedure/DispatchMergingRegionsProcedure.java | 4 +- .../hbase/master/procedure/MasterProcedureEnv.java | 3 +- .../master/procedure/MasterProcedureScheduler.java | 183 ++------- .../procedure/MergeTableRegionsProcedure.java | 4 +- .../master/procedure/ServerCrashProcedure.java | 4 +- .../procedure/SplitTableRegionProcedure.java | 4 +- .../hbase/master/snapshot/TakeSnapshotHandler.java | 40 +- .../java/org/apache/hadoop/hbase/mob/MobUtils.java | 30 +- .../hbase/regionserver/CompactSplitThread.java | 2 +- .../hadoop/hbase/regionserver/HMobStore.java | 68 ++-- .../hadoop/hbase/regionserver/HRegionServer.java | 44 +- .../hbase/regionserver/RegionMergeRequest.java | 92 +++-- .../hbase/regionserver/RegionServerServices.java | 14 +- .../org/apache/hadoop/hbase/util/HBaseFsck.java | 33 +- .../hadoop/hbase/util/hbck/TableLockChecker.java | 87 ---- .../hadoop/hbase/MockRegionServerServices.java | 14 +- .../hbase/master/MockNoopMasterServices.java | 5 - .../hadoop/hbase/master/MockRegionServer.java | 13 +- .../hadoop/hbase/master/TestTableLockManager.java | 433 -------------------- .../hbase/master/locking/TestLockProcedure.java | 9 - ...terProcedureSchedulerPerformanceEvaluation.java | 8 +- .../procedure/TestMasterProcedureScheduler.java | 42 +- .../TestMasterProcedureSchedulerConcurrency.java | 4 +- .../hadoop/hbase/util/TestHBaseFsckOneRS.java | 80 ---- .../hadoop/hbase/util/hbck/HbckTestingUtil.java | 1 - 41 files changed, 271 insertions(+), 1533 deletions(-) delete mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableLockManager.java delete mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/util/hbck/TableLockChecker.java delete mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestTableLockManager.java diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java index 3f3cf33..42567f9 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java @@ -148,14 +148,14 @@ public abstract class Procedure implements Comparable { * * @return true if the lock was acquired and false otherwise */ - protected boolean acquireLock(final TEnvironment env) { + protected boolean acquire(final TEnvironment env) { return true; } /** * The user should override this method, and release lock if necessary. */ - protected void releaseLock(final TEnvironment env) { + protected void release(final TEnvironment env) { // no-op } @@ -738,7 +738,7 @@ public abstract class Procedure implements Comparable { */ @InterfaceAudience.Private protected boolean doAcquireLock(final TEnvironment env) { - return acquireLock(env); + return acquire(env); } /** @@ -746,7 +746,7 @@ public abstract class Procedure implements Comparable { */ @InterfaceAudience.Private protected void doReleaseLock(final TEnvironment env) { - releaseLock(env); + release(env); } @Override diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/ProcedureTestingUtility.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/ProcedureTestingUtility.java index 8aa2088..74b0c1a 100644 --- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/ProcedureTestingUtility.java +++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/ProcedureTestingUtility.java @@ -415,12 +415,12 @@ public class ProcedureTestingUtility { // Mark acquire/release lock functions public for test uses. @Override - public boolean acquireLock(Void env) { + public boolean acquire(Void env) { return true; } @Override - public void releaseLock(Void env) { + public void release(Void env) { // no-op } } diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java index dc28f7d..bf0feab 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java @@ -53,7 +53,8 @@ import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.RegionPlan; import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.master.ServerManager; -import org.apache.hadoop.hbase.master.TableLockManager.TableLock; +import org.apache.hadoop.hbase.master.locking.LockManager; +import org.apache.hadoop.hbase.master.locking.LockProcedure; /** * Service to support Region Server Grouping (HBase-6721) @@ -273,10 +274,15 @@ public class RSGroupAdminServer extends RSGroupAdmin { master.getMasterCoprocessorHost().postMoveTables(tables, targetGroup); } } - for(TableName table: tables) { - TableLock lock = master.getTableLockManager().writeLock(table, "Group: table move"); + for (TableName table: tables) { + LockManager.MasterLock lock = master.getLockManager().createMasterLock(table, + LockProcedure.LockType.EXCLUSIVE, this.getClass().getName() + ": Group: table move"); try { - lock.acquire(); + try { + lock.acquire(); + } catch (InterruptedException e) { + throw new IOException("Interrupted when waiting for table lock", e); + } for (HRegionInfo region : master.getAssignmentManager().getRegionStates().getRegionsOfTable(table)) { master.getAssignmentManager().unassign(region); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/locking/EntityLock.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/locking/EntityLock.java index 990c76d..7a170ec 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/locking/EntityLock.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/locking/EntityLock.java @@ -164,6 +164,8 @@ public class EntityLock { /** * Sends rpc to the master to request lock. * The lock request is queued with other lock requests. + * Call {@link #await()} to wait on lock. + * Always call {@link #unlock()} after calling the below, even after error. */ public void requestLock() throws IOException { if (procId == null) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 3ab4678..3005334 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -112,8 +112,6 @@ public class AssignmentManager { private final MetricsAssignmentManager metricsAssignmentManager; - private final TableLockManager tableLockManager; - private AtomicInteger numRegionsOpened = new AtomicInteger(0); final private KeyLocker locker = new KeyLocker(); @@ -212,13 +210,10 @@ public class AssignmentManager { * @param balancer implementation of {@link LoadBalancer} * @param service Executor service * @param metricsMaster metrics manager - * @param tableLockManager TableLock manager * @throws IOException */ public AssignmentManager(MasterServices server, ServerManager serverManager, - final LoadBalancer balancer, - final ExecutorService service, MetricsMaster metricsMaster, - final TableLockManager tableLockManager, + final LoadBalancer balancer, final ExecutorService service, MetricsMaster metricsMaster, final TableStateManager tableStateManager) throws IOException { this.server = server; @@ -258,7 +253,6 @@ public class AssignmentManager { conf.getInt("hbase.bulk.assignment.perregion.open.time", 10000); this.metricsAssignmentManager = new MetricsAssignmentManager(); - this.tableLockManager = tableLockManager; // Configurations for retrying opening a region on receiving a FAILED_OPEN this.retryConfig = new RetryCounter.RetryConfig(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ExpiredMobFileCleanerChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ExpiredMobFileCleanerChore.java index 3261bd6..faa4f0e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ExpiredMobFileCleanerChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ExpiredMobFileCleanerChore.java @@ -18,7 +18,6 @@ package org.apache.hadoop.hbase.master; -import java.io.IOException; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -29,8 +28,8 @@ import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.ScheduledChore; import org.apache.hadoop.hbase.TableDescriptors; import org.apache.hadoop.hbase.classification.InterfaceAudience; -import org.apache.hadoop.hbase.exceptions.LockTimeoutException; -import org.apache.hadoop.hbase.master.TableLockManager.TableLock; +import org.apache.hadoop.hbase.master.locking.LockManager; +import org.apache.hadoop.hbase.master.locking.LockProcedure; import org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner; import org.apache.hadoop.hbase.mob.MobConstants; import org.apache.hadoop.hbase.mob.MobUtils; @@ -44,7 +43,6 @@ public class ExpiredMobFileCleanerChore extends ScheduledChore { private static final Log LOG = LogFactory.getLog(ExpiredMobFileCleanerChore.class); private final HMaster master; - private TableLockManager tableLockManager; private ExpiredMobFileCleaner cleaner; public ExpiredMobFileCleanerChore(HMaster master) { @@ -53,7 +51,6 @@ public class ExpiredMobFileCleanerChore extends ScheduledChore { .getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD, MobConstants.DEFAULT_MOB_CLEANER_PERIOD), TimeUnit.SECONDS); this.master = master; - this.tableLockManager = master.getTableLockManager(); cleaner = new ExpiredMobFileCleaner(); cleaner.setConf(master.getConfiguration()); } @@ -70,33 +67,14 @@ public class ExpiredMobFileCleanerChore extends ScheduledChore { if (hcd.isMobEnabled() && hcd.getMinVersions() == 0) { // clean only for mob-enabled column. // obtain a read table lock before cleaning, synchronize with MobFileCompactionChore. - boolean tableLocked = false; - TableLock lock = null; + final LockManager.MasterLock lock = master.getLockManager().createMasterLock( + MobUtils.getTableLockName(htd.getTableName()), LockProcedure.LockType.SHARED, + this.getClass().getSimpleName() + ": Cleaning expired mob files"); try { - // the tableLockManager might be null in testing. In that case, it is lock-free. - if (tableLockManager != null) { - lock = tableLockManager.readLock(MobUtils.getTableLockName(htd.getTableName()), - "Run ExpiredMobFileCleanerChore"); - lock.acquire(); - } - tableLocked = true; + lock.acquire(); cleaner.cleanExpiredMobFiles(htd.getTableName().getNameAsString(), hcd); - } catch (LockTimeoutException e) { - LOG.info("Fail to acquire the lock because of timeout, maybe a" - + " MobCompactor is running", e); - } catch (IOException e) { - LOG.error( - "Fail to clean the expired mob files for the column " + hcd.getNameAsString() - + " in the table " + htd.getNameAsString(), e); } finally { - if (lock != null && tableLocked) { - try { - lock.release(); - } catch (IOException e) { - LOG.error( - "Fail to release the read lock for the table " + htd.getNameAsString(), e); - } - } + lock.release(); } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index a41960b..5a5e955 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -646,8 +646,7 @@ public class HMaster extends HRegionServer implements MasterServices { this.splitOrMergeTracker.start(); this.assignmentManager = new AssignmentManager(this, serverManager, - this.balancer, this.service, this.metricsMaster, - this.tableLockManager, tableStateManager); + this.balancer, this.service, this.metricsMaster, tableStateManager); this.replicationManager = new ReplicationManager(conf, zooKeeper, this); @@ -733,8 +732,6 @@ public class HMaster extends HRegionServer implements MasterServices { this.serverManager = createServerManager(this); - // Invalidate all write locks held previously - this.tableLockManager.reapWriteLocks(); this.tableStateManager = new TableStateManager(this); status.setStatus("Initializing ZK system trackers"); @@ -3080,8 +3077,7 @@ public class HMaster extends HRegionServer implements MasterServices { */ public void requestMobCompaction(TableName tableName, List columns, boolean allFiles) throws IOException { - mobCompactThread.requestMobCompaction(conf, fs, tableName, columns, - tableLockManager, allFiles); + mobCompactThread.requestMobCompaction(conf, fs, tableName, columns, allFiles); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMobCompactionThread.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMobCompactionThread.java index c0a915b..fc0ecfb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMobCompactionThread.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMobCompactionThread.java @@ -34,6 +34,8 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.master.locking.LockManager; +import org.apache.hadoop.hbase.master.locking.LockProcedure; import org.apache.hadoop.hbase.mob.MobUtils; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; @@ -74,15 +76,13 @@ public class MasterMobCompactionThread { * @param fs The file system * @param tableName The table the compact * @param columns The column descriptors - * @param tableLockManager The tableLock manager * @param allFiles Whether add all mob files into the compaction. */ public void requestMobCompaction(Configuration conf, FileSystem fs, TableName tableName, - List columns, TableLockManager tableLockManager, boolean allFiles) - throws IOException { + List columns, boolean allFiles) throws IOException { master.reportMobCompactionStart(tableName); try { - masterMobPool.execute(new CompactionRunner(fs, tableName, columns, tableLockManager, + masterMobPool.execute(new CompactionRunner(fs, tableName, columns, allFiles, mobCompactorPool)); } catch (RejectedExecutionException e) { // in case the request is rejected by the pool @@ -103,27 +103,28 @@ public class MasterMobCompactionThread { private FileSystem fs; private TableName tableName; private List hcds; - private TableLockManager tableLockManager; private boolean allFiles; private ExecutorService pool; public CompactionRunner(FileSystem fs, TableName tableName, List hcds, - TableLockManager tableLockManager, boolean allFiles, ExecutorService pool) { + boolean allFiles, ExecutorService pool) { super(); this.fs = fs; this.tableName = tableName; this.hcds = hcds; - this.tableLockManager = tableLockManager; this.allFiles = allFiles; this.pool = pool; } @Override public void run() { + // These locks are on dummy table names, and only used for compaction/mob file cleaning. + final LockManager.MasterLock lock = master.getLockManager().createMasterLock( + MobUtils.getTableLockName(tableName), LockProcedure.LockType.EXCLUSIVE, + this.getClass().getName() + ": mob compaction"); try { for (HColumnDescriptor hcd : hcds) { - MobUtils.doMobCompaction(conf, fs, tableName, hcd, pool, tableLockManager, - allFiles); + MobUtils.doMobCompaction(conf, fs, tableName, hcd, pool, allFiles, lock); } } catch (IOException e) { LOG.error("Failed to perform the mob compaction", e); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java index 5019eda..ce6dc76 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java @@ -93,11 +93,6 @@ public interface MasterServices extends Server { ExecutorService getExecutorService(); /** - * @return Master's instance of {@link TableLockManager} - */ - TableLockManager getTableLockManager(); - - /** * @return Master's instance of {@link TableStateManager} */ TableStateManager getTableStateManager(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java index 4b956e6..42a5445 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java @@ -30,6 +30,8 @@ import org.apache.hadoop.hbase.ScheduledChore; import org.apache.hadoop.hbase.TableDescriptors; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.TableState; +import org.apache.hadoop.hbase.master.locking.LockManager; +import org.apache.hadoop.hbase.master.locking.LockProcedure; import org.apache.hadoop.hbase.mob.MobUtils; /** @@ -40,14 +42,12 @@ public class MobCompactionChore extends ScheduledChore { private static final Log LOG = LogFactory.getLog(MobCompactionChore.class); private HMaster master; - private TableLockManager tableLockManager; private ExecutorService pool; public MobCompactionChore(HMaster master, int period) { // use the period as initial delay. super(master.getServerName() + "-MobCompactionChore", master, period, period, TimeUnit.SECONDS); this.master = master; - this.tableLockManager = master.getTableLockManager(); this.pool = MobUtils.createMobCompactorThreadPool(master.getConfiguration()); } @@ -63,6 +63,9 @@ public class MobCompactionChore extends ScheduledChore { } boolean reported = false; try { + final LockManager.MasterLock lock = master.getLockManager().createMasterLock( + MobUtils.getTableLockName(htd.getTableName()), LockProcedure.LockType.EXCLUSIVE, + this.getClass().getName() + ": mob compaction"); for (HColumnDescriptor hcd : htd.getColumnFamilies()) { if (!hcd.isMobEnabled()) { continue; @@ -72,7 +75,7 @@ public class MobCompactionChore extends ScheduledChore { reported = true; } MobUtils.doMobCompaction(master.getConfiguration(), master.getFileSystem(), - htd.getTableName(), hcd, pool, tableLockManager, false); + htd.getTableName(), hcd, pool, false, lock); } } finally { if (reported) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableLockManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableLockManager.java deleted file mode 100644 index c8eefa3..0000000 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableLockManager.java +++ /dev/null @@ -1,453 +0,0 @@ -/* - * Copyright The Apache Software Foundation - * - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.master; - -import java.io.IOException; -import java.io.InterruptedIOException; -import java.util.List; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.hbase.InterProcessLock; -import org.apache.hadoop.hbase.InterProcessLock.MetadataHandler; -import org.apache.hadoop.hbase.InterProcessReadWriteLock; -import org.apache.hadoop.hbase.ServerName; -import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.classification.InterfaceAudience; -import org.apache.hadoop.hbase.exceptions.LockTimeoutException; -import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; -import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos; -import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.zookeeper.ZKUtil; -import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; -import org.apache.hadoop.hbase.zookeeper.lock.ZKInterProcessReadWriteLock; -import org.apache.zookeeper.KeeperException; - -/** - * A manager for distributed table level locks. - */ -@InterfaceAudience.Private -public abstract class TableLockManager { - - private static final Log LOG = LogFactory.getLog(TableLockManager.class); - - /** Configuration key for enabling table-level locks for schema changes */ - public static final String TABLE_LOCK_ENABLE = - "hbase.table.lock.enable"; - - /** by default we should enable table-level locks for schema changes */ - private static final boolean DEFAULT_TABLE_LOCK_ENABLE = true; - - /** Configuration key for time out for trying to acquire table locks */ - protected static final String TABLE_WRITE_LOCK_TIMEOUT_MS = - "hbase.table.write.lock.timeout.ms"; - - /** Configuration key for time out for trying to acquire table locks */ - protected static final String TABLE_READ_LOCK_TIMEOUT_MS = - "hbase.table.read.lock.timeout.ms"; - - protected static final long DEFAULT_TABLE_WRITE_LOCK_TIMEOUT_MS = - 600 * 1000; //10 min default - - protected static final long DEFAULT_TABLE_READ_LOCK_TIMEOUT_MS = - 600 * 1000; //10 min default - - public static final String TABLE_LOCK_EXPIRE_TIMEOUT = "hbase.table.lock.expire.ms"; - - public static final long DEFAULT_TABLE_LOCK_EXPIRE_TIMEOUT_MS = - 600 * 1000; //10 min default - - /** - * A distributed lock for a table. - */ - @InterfaceAudience.Private - public interface TableLock { - /** - * Acquire the lock, with the configured lock timeout. - * @throws LockTimeoutException If unable to acquire a lock within a specified - * time period (if any) - * @throws IOException If unrecoverable error occurs - */ - void acquire() throws IOException; - - /** - * Release the lock already held. - * @throws IOException If there is an unrecoverable error releasing the lock - */ - void release() throws IOException; - } - - /** - * Returns a TableLock for locking the table for exclusive access - * @param tableName Table to lock - * @param purpose Human readable reason for locking the table - * @return A new TableLock object for acquiring a write lock - */ - public abstract TableLock writeLock(TableName tableName, String purpose); - - /** - * Returns a TableLock for locking the table for shared access among read-lock holders - * @param tableName Table to lock - * @param purpose Human readable reason for locking the table - * @return A new TableLock object for acquiring a read lock - */ - public abstract TableLock readLock(TableName tableName, String purpose); - - /** - * Visits all table locks(read and write), and lock attempts with the given callback - * MetadataHandler. - * @param handler the metadata handler to call - * @throws IOException If there is an unrecoverable error - */ - public abstract void visitAllLocks(MetadataHandler handler) throws IOException; - - /** - * Force releases all table locks(read and write) that have been held longer than - * "hbase.table.lock.expire.ms". Assumption is that the clock skew between zookeeper - * and this servers is negligible. - * The behavior of the lock holders still thinking that they have the lock is undefined. - * @throws IOException If there is an unrecoverable error - */ - public abstract void reapAllExpiredLocks() throws IOException; - - /** - * Force releases table write locks and lock attempts even if this thread does - * not own the lock. The behavior of the lock holders still thinking that they - * have the lock is undefined. This should be used carefully and only when - * we can ensure that all write-lock holders have died. For example if only - * the master can hold write locks, then we can reap it's locks when the backup - * master starts. - * @throws IOException If there is an unrecoverable error - */ - public abstract void reapWriteLocks() throws IOException; - - /** - * Called after a table has been deleted, and after the table lock is released. - * TableLockManager should do cleanup for the table state. - * @param tableName name of the table - * @throws IOException If there is an unrecoverable error releasing the lock - */ - public abstract void tableDeleted(TableName tableName) - throws IOException; - - /** - * Creates and returns a TableLockManager according to the configuration - */ - public static TableLockManager createTableLockManager(Configuration conf, - ZooKeeperWatcher zkWatcher, ServerName serverName) { - // Initialize table level lock manager for schema changes, if enabled. - if (conf.getBoolean(TABLE_LOCK_ENABLE, - DEFAULT_TABLE_LOCK_ENABLE)) { - long writeLockTimeoutMs = conf.getLong(TABLE_WRITE_LOCK_TIMEOUT_MS, - DEFAULT_TABLE_WRITE_LOCK_TIMEOUT_MS); - long readLockTimeoutMs = conf.getLong(TABLE_READ_LOCK_TIMEOUT_MS, - DEFAULT_TABLE_READ_LOCK_TIMEOUT_MS); - long lockExpireTimeoutMs = conf.getLong(TABLE_LOCK_EXPIRE_TIMEOUT, - DEFAULT_TABLE_LOCK_EXPIRE_TIMEOUT_MS); - - return new ZKTableLockManager(zkWatcher, serverName, writeLockTimeoutMs, readLockTimeoutMs, lockExpireTimeoutMs); - } - - return new NullTableLockManager(); - } - - /** - * A null implementation - */ - @InterfaceAudience.Private - public static class NullTableLockManager extends TableLockManager { - static class NullTableLock implements TableLock { - @Override - public void acquire() throws IOException { - } - @Override - public void release() throws IOException { - } - } - @Override - public TableLock writeLock(TableName tableName, String purpose) { - return new NullTableLock(); - } - @Override - public TableLock readLock(TableName tableName, String purpose) { - return new NullTableLock(); - } - @Override - public void reapAllExpiredLocks() throws IOException { - } - @Override - public void reapWriteLocks() throws IOException { - } - @Override - public void tableDeleted(TableName tableName) throws IOException { - } - @Override - public void visitAllLocks(MetadataHandler handler) throws IOException { - } - } - - /** Public for hbck */ - public static ZooKeeperProtos.TableLock fromBytes(byte[] bytes) { - int pblen = ProtobufUtil.lengthOfPBMagic(); - if (bytes == null || bytes.length < pblen) { - return null; - } - try { - ZooKeeperProtos.TableLock.Builder builder = ZooKeeperProtos.TableLock.newBuilder(); - ProtobufUtil.mergeFrom(builder, bytes, pblen, bytes.length - pblen); - return builder.build(); - } catch (IOException ex) { - LOG.warn("Exception in deserialization", ex); - } - return null; - } - - /** - * ZooKeeper based TableLockManager - */ - @InterfaceAudience.Private - private static class ZKTableLockManager extends TableLockManager { - - private static final MetadataHandler METADATA_HANDLER = new MetadataHandler() { - @Override - public void handleMetadata(byte[] ownerMetadata) { - if (!LOG.isDebugEnabled()) { - return; - } - ZooKeeperProtos.TableLock data = fromBytes(ownerMetadata); - if (data == null) { - return; - } - LOG.debug("Table is locked by " + - String.format("[tableName=%s:%s, lockOwner=%s, threadId=%s, " + - "purpose=%s, isShared=%s, createTime=%s]", - data.getTableName().getNamespace().toStringUtf8(), - data.getTableName().getQualifier().toStringUtf8(), - ProtobufUtil.toServerName(data.getLockOwner()), data.getThreadId(), - data.getPurpose(), data.getIsShared(), data.getCreateTime())); - } - }; - - private static class TableLockImpl implements TableLock { - long lockTimeoutMs; - TableName tableName; - InterProcessLock lock; - boolean isShared; - ZooKeeperWatcher zkWatcher; - ServerName serverName; - String purpose; - - public TableLockImpl(TableName tableName, ZooKeeperWatcher zkWatcher, - ServerName serverName, long lockTimeoutMs, boolean isShared, String purpose) { - this.tableName = tableName; - this.zkWatcher = zkWatcher; - this.serverName = serverName; - this.lockTimeoutMs = lockTimeoutMs; - this.isShared = isShared; - this.purpose = purpose; - } - - @Override - public void acquire() throws IOException { - if (LOG.isTraceEnabled()) { - LOG.trace("Attempt to acquire table " + (isShared ? "read" : "write") + - " lock on: " + tableName + " for:" + purpose); - } - - lock = createTableLock(); - try { - if (lockTimeoutMs == -1) { - // Wait indefinitely - lock.acquire(); - } else { - if (!lock.tryAcquire(lockTimeoutMs)) { - throw new LockTimeoutException("Timed out acquiring " + - (isShared ? "read" : "write") + "lock for table:" + tableName + - "for:" + purpose + " after " + lockTimeoutMs + " ms."); - } - } - } catch (InterruptedException e) { - LOG.warn("Interrupted acquiring a lock for " + tableName, e); - Thread.currentThread().interrupt(); - throw new InterruptedIOException("Interrupted acquiring a lock"); - } - if (LOG.isTraceEnabled()) LOG.trace("Acquired table " + (isShared ? "read" : "write") - + " lock on " + tableName + " for " + purpose); - } - - @Override - public void release() throws IOException { - if (LOG.isTraceEnabled()) { - LOG.trace("Attempt to release table " + (isShared ? "read" : "write") - + " lock on " + tableName); - } - if (lock == null) { - throw new IllegalStateException("Table " + tableName + - " is not locked!"); - } - - try { - lock.release(); - } catch (InterruptedException e) { - LOG.warn("Interrupted while releasing a lock for " + tableName); - throw new InterruptedIOException(); - } - if (LOG.isTraceEnabled()) { - LOG.trace("Released table lock on " + tableName); - } - } - - private InterProcessLock createTableLock() { - String tableLockZNode = ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, - tableName.getNameAsString()); - - ZooKeeperProtos.TableLock data = ZooKeeperProtos.TableLock.newBuilder() - .setTableName(ProtobufUtil.toProtoTableName(tableName)) - .setLockOwner(ProtobufUtil.toServerName(serverName)) - .setThreadId(Thread.currentThread().getId()) - .setPurpose(purpose) - .setIsShared(isShared) - .setCreateTime(EnvironmentEdgeManager.currentTime()).build(); - byte[] lockMetadata = toBytes(data); - - InterProcessReadWriteLock lock = new ZKInterProcessReadWriteLock(zkWatcher, tableLockZNode, - METADATA_HANDLER); - return isShared ? lock.readLock(lockMetadata) : lock.writeLock(lockMetadata); - } - } - - private static byte[] toBytes(ZooKeeperProtos.TableLock data) { - return ProtobufUtil.prependPBMagic(data.toByteArray()); - } - - private final ServerName serverName; - private final ZooKeeperWatcher zkWatcher; - private final long writeLockTimeoutMs; - private final long readLockTimeoutMs; - private final long lockExpireTimeoutMs; - - /** - * Initialize a new manager for table-level locks. - * @param zkWatcher - * @param serverName Address of the server responsible for acquiring and - * releasing the table-level locks - * @param writeLockTimeoutMs Timeout (in milliseconds) for acquiring a write lock for a - * given table, or -1 for no timeout - * @param readLockTimeoutMs Timeout (in milliseconds) for acquiring a read lock for a - * given table, or -1 for no timeout - */ - public ZKTableLockManager(ZooKeeperWatcher zkWatcher, - ServerName serverName, long writeLockTimeoutMs, long readLockTimeoutMs, long lockExpireTimeoutMs) { - this.zkWatcher = zkWatcher; - this.serverName = serverName; - this.writeLockTimeoutMs = writeLockTimeoutMs; - this.readLockTimeoutMs = readLockTimeoutMs; - this.lockExpireTimeoutMs = lockExpireTimeoutMs; - } - - @Override - public TableLock writeLock(TableName tableName, String purpose) { - return new TableLockImpl(tableName, zkWatcher, - serverName, writeLockTimeoutMs, false, purpose); - } - - public TableLock readLock(TableName tableName, String purpose) { - return new TableLockImpl(tableName, zkWatcher, - serverName, readLockTimeoutMs, true, purpose); - } - - public void visitAllLocks(MetadataHandler handler) throws IOException { - for (String tableName : getTableNames()) { - String tableLockZNode = ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, tableName); - ZKInterProcessReadWriteLock lock = new ZKInterProcessReadWriteLock( - zkWatcher, tableLockZNode, null); - lock.readLock(null).visitLocks(handler); - lock.writeLock(null).visitLocks(handler); - } - } - - private List getTableNames() throws IOException { - - List tableNames; - try { - tableNames = ZKUtil.listChildrenNoWatch(zkWatcher, zkWatcher.znodePaths.tableLockZNode); - } catch (KeeperException e) { - LOG.error("Unexpected ZooKeeper error when listing children", e); - throw new IOException("Unexpected ZooKeeper exception", e); - } - return tableNames; - } - - @Override - public void reapWriteLocks() throws IOException { - //get the table names - try { - for (String tableName : getTableNames()) { - String tableLockZNode = ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, tableName); - ZKInterProcessReadWriteLock lock = new ZKInterProcessReadWriteLock( - zkWatcher, tableLockZNode, null); - lock.writeLock(null).reapAllLocks(); - } - } catch (IOException ex) { - throw ex; - } catch (Exception ex) { - LOG.warn("Caught exception while reaping table write locks", ex); - } - } - - @Override - public void reapAllExpiredLocks() throws IOException { - //get the table names - try { - for (String tableName : getTableNames()) { - String tableLockZNode = ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, tableName); - ZKInterProcessReadWriteLock lock = new ZKInterProcessReadWriteLock( - zkWatcher, tableLockZNode, null); - lock.readLock(null).reapExpiredLocks(lockExpireTimeoutMs); - lock.writeLock(null).reapExpiredLocks(lockExpireTimeoutMs); - } - } catch (IOException ex) { - throw ex; - } catch (Exception ex) { - throw new IOException(ex); - } - } - - @Override - public void tableDeleted(TableName tableName) throws IOException { - //table write lock from DeleteHandler is already released, just delete the parent znode - String tableNameStr = tableName.getNameAsString(); - String tableLockZNode = ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, tableNameStr); - try { - ZKUtil.deleteNode(zkWatcher, tableLockZNode); - } catch (KeeperException ex) { - if (ex.code() == KeeperException.Code.NOTEMPTY) { - //we might get this in rare occasions where a CREATE table or some other table operation - //is waiting to acquire the lock. In this case, parent znode won't be deleted. - LOG.warn("Could not delete the znode for table locks because NOTEMPTY: " - + tableLockZNode); - return; - } - throw new IOException(ex); - } - } - } -} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockProcedure.java index f793a65..5ab9959 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockProcedure.java @@ -1,4 +1,5 @@ /** + * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -288,7 +289,7 @@ public final class LockProcedure extends Procedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { boolean ret = lock.acquireLock(env); locked.set(ret); hasLock = ret; @@ -302,7 +303,7 @@ public final class LockProcedure extends Procedure } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { lock.releaseLock(env); hasLock = false; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineNamespaceProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineNamespaceProcedure.java index a514532..31d7df9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineNamespaceProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineNamespaceProcedure.java @@ -58,13 +58,13 @@ public abstract class AbstractStateMachineNamespaceProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireNamespaceExclusiveLock(this, getNamespaceName()); } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { env.getProcedureQueue().releaseNamespaceExclusiveLock(this, getNamespaceName()); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java index 7cced45..6fca5ae 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java @@ -75,13 +75,13 @@ public abstract class AbstractStateMachineTableProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (env.waitInitialized(this)) return false; return env.getProcedureQueue().tryAcquireTableExclusiveLock(this, getTableName()); } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { env.getProcedureQueue().releaseTableExclusiveLock(this, getTableName()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java index 982e880..22f4ccc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java @@ -160,7 +160,7 @@ public class CreateNamespaceProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (!env.getMasterServices().isInitialized()) { // Namespace manager might not be ready if master is not fully initialized, // return false to reject user namespace creation; return true for default diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java index 0d24f51..b59848d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java @@ -216,7 +216,7 @@ public class CreateTableProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (!getTableName().isSystemTable() && env.waitInitialized(this)) { return false; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DispatchMergingRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DispatchMergingRegionsProcedure.java index ee92932..4eed4d3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DispatchMergingRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DispatchMergingRegionsProcedure.java @@ -253,13 +253,13 @@ public class DispatchMergingRegionsProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { return !env.getProcedureQueue().waitRegions( this, getTableName(), regionsToMerge[0], regionsToMerge[1]); } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { env.getProcedureQueue().wakeRegions(this, getTableName(), regionsToMerge[0], regionsToMerge[1]); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureEnv.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureEnv.java index 9362f24..353342a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureEnv.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureEnv.java @@ -98,8 +98,7 @@ public class MasterProcedureEnv implements ConfigurationObserver { public MasterProcedureEnv(final MasterServices master) { this.master = master; - this.procSched = new MasterProcedureScheduler(master.getConfiguration(), - master.getTableLockManager()); + this.procSched = new MasterProcedureScheduler(master.getConfiguration()); } public User getRequestUser() { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java index 3f588ff..b9b7b59 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java @@ -20,7 +20,6 @@ package org.apache.hadoop.hbase.master.procedure; import com.google.common.annotations.VisibleForTesting; -import java.io.IOException; import java.util.ArrayDeque; import java.util.Arrays; import java.util.HashMap; @@ -35,8 +34,6 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; -import org.apache.hadoop.hbase.master.TableLockManager; -import org.apache.hadoop.hbase.master.TableLockManager.TableLock; import org.apache.hadoop.hbase.master.procedure.TableProcedureInterface.TableOperationType; import org.apache.hadoop.hbase.procedure2.AbstractProcedureScheduler; import org.apache.hadoop.hbase.procedure2.Procedure; @@ -67,8 +64,6 @@ import org.apache.hadoop.hbase.util.AvlUtil.AvlTreeIterator; public class MasterProcedureScheduler extends AbstractProcedureScheduler { private static final Log LOG = LogFactory.getLog(MasterProcedureScheduler.class); - private final TableLockManager lockManager; - private final static NamespaceQueueKeyComparator NAMESPACE_QUEUE_KEY_COMPARATOR = new NamespaceQueueKeyComparator(); private final static ServerQueueKeyComparator SERVER_QUEUE_KEY_COMPARATOR = @@ -87,9 +82,7 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { private final int userTablePriority; private final int sysTablePriority; - public MasterProcedureScheduler(final Configuration conf, final TableLockManager lockManager) { - this.lockManager = lockManager; - + public MasterProcedureScheduler(final Configuration conf) { // TODO: should this be part of the HTD? metaTablePriority = conf.getInt("hbase.master.procedure.queue.meta.table.priority", 3); sysTablePriority = conf.getInt("hbase.master.procedure.queue.system.table.priority", 2); @@ -456,7 +449,6 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { private final NamespaceQueue namespaceQueue; private HashMap regionEventMap; - private TableLock tableLock = null; public TableQueue(TableName tableName, NamespaceQueue namespaceQueue, int priority) { super(tableName, priority); @@ -544,65 +536,6 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { } throw new UnsupportedOperationException("unexpected type " + tpi.getTableOperationType()); } - - private synchronized boolean tryZkSharedLock(final TableLockManager lockManager, - final String purpose) { - // Since we only have one lock resource. We should only acquire zk lock if the znode - // does not exist. - // - if (isSingleSharedLock()) { - // Take zk-read-lock - TableName tableName = getKey(); - tableLock = lockManager.readLock(tableName, purpose); - try { - tableLock.acquire(); - } catch (IOException e) { - LOG.error("failed acquire read lock on " + tableName, e); - tableLock = null; - return false; - } - } - return true; - } - - private synchronized void releaseZkSharedLock(final TableLockManager lockManager) { - if (isSingleSharedLock()) { - releaseTableLock(lockManager, true); - } - } - - private synchronized boolean tryZkExclusiveLock(final TableLockManager lockManager, - final String purpose) { - // Take zk-write-lock - TableName tableName = getKey(); - tableLock = lockManager.writeLock(tableName, purpose); - try { - tableLock.acquire(); - } catch (IOException e) { - LOG.error("failed acquire write lock on " + tableName, e); - tableLock = null; - return false; - } - return true; - } - - private synchronized void releaseZkExclusiveLock(final TableLockManager lockManager) { - releaseTableLock(lockManager, true); - } - - private void releaseTableLock(final TableLockManager lockManager, boolean reset) { - for (int i = 0; i < 3; ++i) { - try { - tableLock.release(); - if (reset) { - tableLock = null; - } - break; - } catch (IOException e) { - LOG.warn("Could not release the table write-lock", e); - } - } - } } private static class NamespaceQueueKeyComparator implements AvlKeyComparator { @@ -665,35 +598,22 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { */ public boolean tryAcquireTableExclusiveLock(final Procedure procedure, final TableName table) { schedLock(); - TableQueue queue = getTableQueue(table); - if (!queue.getNamespaceQueue().trySharedLock()) { - schedUnlock(); - return false; - } - - if (!queue.tryExclusiveLock(procedure)) { - queue.getNamespaceQueue().releaseSharedLock(); - schedUnlock(); - return false; - } - - removeFromRunQueue(tableRunQueue, queue); - boolean hasParentLock = queue.hasParentLock(procedure); - schedUnlock(); + try { + final TableQueue queue = getTableQueue(table); + if (!queue.getNamespaceQueue().trySharedLock()) { + return false; + } - boolean hasXLock = true; - if (!hasParentLock) { - // Zk lock is expensive... - hasXLock = queue.tryZkExclusiveLock(lockManager, procedure.toString()); - if (!hasXLock) { - schedLock(); - if (!hasParentLock) queue.releaseExclusiveLock(procedure); + if (!queue.tryExclusiveLock(procedure)) { queue.getNamespaceQueue().releaseSharedLock(); - addToRunQueue(tableRunQueue, queue); - schedUnlock(); + return false; } + + removeFromRunQueue(tableRunQueue, queue); + return true; + } finally { + schedUnlock(); } - return hasXLock; } /** @@ -702,19 +622,17 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { * @param table the name of the table that has the exclusive lock */ public void releaseTableExclusiveLock(final Procedure procedure, final TableName table) { - final TableQueue queue = getTableQueueWithLock(table); - final boolean hasParentLock = queue.hasParentLock(procedure); - - if (!hasParentLock) { - // Zk lock is expensive... - queue.releaseZkExclusiveLock(lockManager); - } - schedLock(); - if (!hasParentLock) queue.releaseExclusiveLock(procedure); - queue.getNamespaceQueue().releaseSharedLock(); - addToRunQueue(tableRunQueue, queue); - schedUnlock(); + try { + final TableQueue queue = getTableQueue(table); + if (!queue.hasParentLock(procedure)) { + queue.releaseExclusiveLock(procedure); + } + queue.getNamespaceQueue().releaseSharedLock(); + addToRunQueue(tableRunQueue, queue); + } finally { + schedUnlock(); + } } /** @@ -731,29 +649,21 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { private TableQueue tryAcquireTableQueueSharedLock(final Procedure procedure, final TableName table) { schedLock(); - TableQueue queue = getTableQueue(table); - if (!queue.getNamespaceQueue().trySharedLock()) { - return null; - } + try { + final TableQueue queue = getTableQueue(table); + if (!queue.getNamespaceQueue().trySharedLock()) { + return null; + } - if (!queue.trySharedLock()) { - queue.getNamespaceQueue().releaseSharedLock(); - schedUnlock(); - return null; - } + if (!queue.trySharedLock()) { + queue.getNamespaceQueue().releaseSharedLock(); + return null; + } - // TODO: Zk lock is expensive and it would be perf bottleneck. Long term solution is - // to remove it. - if (!queue.tryZkSharedLock(lockManager, procedure.toString())) { - queue.releaseSharedLock(); - queue.getNamespaceQueue().releaseSharedLock(); + return queue; + } finally { schedUnlock(); - return null; } - - schedUnlock(); - - return queue; } /** @@ -762,17 +672,16 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { * @param table the name of the table that has the shared lock */ public void releaseTableSharedLock(final Procedure procedure, final TableName table) { - final TableQueue queue = getTableQueueWithLock(table); - schedLock(); - // Zk lock is expensive... - queue.releaseZkSharedLock(lockManager); - - queue.getNamespaceQueue().releaseSharedLock(); - if (queue.releaseSharedLock()) { - addToRunQueue(tableRunQueue, queue); + try { + final TableQueue queue = getTableQueue(table); + if (queue.releaseSharedLock()) { + addToRunQueue(tableRunQueue, queue); + } + queue.getNamespaceQueue().releaseSharedLock(); + } finally { + schedUnlock(); } - schedUnlock(); } /** @@ -796,14 +705,6 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler { if (AvlIterableList.isLinked(queue)) { tableRunQueue.remove(queue); } - - // Remove the table lock - try { - lockManager.tableDeleted(table); - } catch (IOException e) { - LOG.warn("Received exception from TableLockManager.tableDeleted:", e); //not critical - } - removeTableQueue(table); } else { // TODO: If there are no create, we can drop all the other ops diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MergeTableRegionsProcedure.java index c313700..5cee3c1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MergeTableRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MergeTableRegionsProcedure.java @@ -322,7 +322,7 @@ public class MergeTableRegionsProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (env.waitInitialized(this)) { return false; } @@ -331,7 +331,7 @@ public class MergeTableRegionsProcedure } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { env.getProcedureQueue().wakeRegions(this, getTableName(), regionsToMerge[0], regionsToMerge[1]); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java index 98a2152..f27e7c5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java @@ -562,13 +562,13 @@ implements ServerProcedureInterface { } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (env.waitServerCrashProcessingEnabled(this)) return false; return env.getProcedureQueue().tryAcquireServerExclusiveLock(this, getServerName()); } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { env.getProcedureQueue().releaseServerExclusiveLock(this, getServerName()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitTableRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitTableRegionProcedure.java index 4730ad8..75dccf2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitTableRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitTableRegionProcedure.java @@ -347,7 +347,7 @@ public class SplitTableRegionProcedure } @Override - protected boolean acquireLock(final MasterProcedureEnv env) { + protected boolean acquire(final MasterProcedureEnv env) { if (env.waitInitialized(this)) { return false; } @@ -355,7 +355,7 @@ public class SplitTableRegionProcedure } @Override - protected void releaseLock(final MasterProcedureEnv env) { + protected void release(final MasterProcedureEnv env) { env.getProcedureScheduler().wakeRegions(this, getTableName(), parentHRI); } 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 a0b6d25..992f28e 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 @@ -44,8 +44,8 @@ import org.apache.hadoop.hbase.executor.EventType; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.MetricsSnapshot; 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.master.locking.LockManager; +import org.apache.hadoop.hbase.master.locking.LockProcedure; import org.apache.hadoop.hbase.monitoring.MonitoredTask; import org.apache.hadoop.hbase.monitoring.TaskMonitor; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.SnapshotDescription; @@ -83,8 +83,7 @@ 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; + protected final LockManager.MasterLock tableLock; protected final MonitoredTask status; protected final TableName snapshotTable; protected final SnapshotManifest snapshotManifest; @@ -114,10 +113,9 @@ public abstract class TakeSnapshotHandler extends EventHandler implements Snapsh this.monitor = new ForeignExceptionDispatcher(snapshot.getName()); this.snapshotManifest = SnapshotManifest.create(conf, fs, workingDir, snapshot, monitor); - this.tableLockManager = master.getTableLockManager(); - this.tableLock = this.tableLockManager.writeLock( - snapshotTable, - EventType.C_M_SNAPSHOT_TABLE.toString()); + this.tableLock = master.getLockManager().createMasterLock( + snapshotTable, LockProcedure.LockType.EXCLUSIVE, + this.getClass().getName() + ": take snapshot " + snapshot.getName()); // prepare the verify this.verifier = new MasterSnapshotVerifier(masterServices, snapshot, rootDir); @@ -138,18 +136,14 @@ public abstract class TakeSnapshotHandler extends EventHandler implements Snapsh public TakeSnapshotHandler prepare() throws Exception { super.prepare(); - this.tableLock.acquire(); // after this, you should ensure to release this lock in - // case of exceptions - boolean success = false; + // after this, you should ensure to release this lock in case of exceptions + this.tableLock.acquire(); try { this.htd = loadTableDescriptor(); // check that .tableinfo is present - success = true; - } finally { - if (!success) { - releaseTableLock(); - } + } catch (Exception e) { + this.tableLock.release(); + throw e; } - return this; } @@ -234,17 +228,7 @@ public abstract class TakeSnapshotHandler extends EventHandler implements Snapsh LOG.error("Couldn't delete snapshot working directory:" + workingDir); } lock.unlock(); - releaseTableLock(); - } - } - - protected void releaseTableLock() { - if (this.tableLock != null) { - try { - this.tableLock.release(); - } catch (IOException ex) { - LOG.warn("Could not release the table lock", ex); - } + tableLock.release(); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java index 770c069..2592812 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java @@ -61,8 +61,8 @@ import org.apache.hadoop.hbase.io.crypto.Encryption; import org.apache.hadoop.hbase.io.hfile.CacheConfig; import org.apache.hadoop.hbase.io.hfile.HFileContext; import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder; -import org.apache.hadoop.hbase.master.TableLockManager; -import org.apache.hadoop.hbase.master.TableLockManager.TableLock; +import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.locking.LockManager; import org.apache.hadoop.hbase.mob.compactions.MobCompactor; import org.apache.hadoop.hbase.mob.compactions.PartitionedMobCompactor; import org.apache.hadoop.hbase.regionserver.BloomType; @@ -699,12 +699,11 @@ public final class MobUtils { * @param tableName the table the compact * @param hcd the column descriptor * @param pool the thread pool - * @param tableLockManager the tableLock manager * @param allFiles Whether add all mob files into the compaction. */ public static void doMobCompaction(Configuration conf, FileSystem fs, TableName tableName, - HColumnDescriptor hcd, ExecutorService pool, TableLockManager tableLockManager, - boolean allFiles) throws IOException { + HColumnDescriptor hcd, ExecutorService pool, boolean allFiles, LockManager.MasterLock lock) + throws IOException { String className = conf.get(MobConstants.MOB_COMPACTOR_CLASS_KEY, PartitionedMobCompactor.class.getName()); // instantiate the mob compactor. @@ -719,29 +718,14 @@ public final class MobUtils { // compact only for mob-enabled column. // obtain a write table lock before performing compaction to avoid race condition // with major compaction in mob-enabled column. - boolean tableLocked = false; - TableLock lock = null; try { - // the tableLockManager might be null in testing. In that case, it is lock-free. - if (tableLockManager != null) { - lock = tableLockManager.writeLock(MobUtils.getTableLockName(tableName), - "Run MobCompactor"); - lock.acquire(); - } - tableLocked = true; + lock.acquire(); compactor.compact(allFiles); } catch (Exception e) { LOG.error("Failed to compact the mob files for the column " + hcd.getNameAsString() - + " in the table " + tableName.getNameAsString(), e); + + " in the table " + tableName.getNameAsString(), e); } finally { - if (lock != null && tableLocked) { - try { - lock.release(); - } catch (IOException e) { - LOG.error( - "Failed to release the write lock for the table " + tableName.getNameAsString(), e); - } - } + lock.release(); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java index 1331b86..262a14a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java @@ -80,7 +80,7 @@ public class CompactSplitThread implements CompactionRequestor, PropagatingConfi public static final String REGION_SERVER_REGION_SPLIT_LIMIT = "hbase.regionserver.regionSplitLimit"; public static final int DEFAULT_REGION_SERVER_REGION_SPLIT_LIMIT= 1000; - + private final HRegionServer server; private final Configuration conf; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HMobStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HMobStore.java index 0bf6c9a..5274110 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HMobStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HMobStore.java @@ -20,12 +20,14 @@ package org.apache.hadoop.hbase.regionserver; import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; import java.util.NavigableSet; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -37,6 +39,7 @@ import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.KeyValue.Type; import org.apache.hadoop.hbase.TableName; @@ -45,6 +48,7 @@ import org.apache.hadoop.hbase.TagType; import org.apache.hadoop.hbase.TagUtil; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.client.locking.EntityLock; import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.filter.FilterList; import org.apache.hadoop.hbase.io.compress.Compression; @@ -52,8 +56,6 @@ import org.apache.hadoop.hbase.io.hfile.CacheConfig; import org.apache.hadoop.hbase.io.hfile.CorruptHFileException; import org.apache.hadoop.hbase.io.hfile.HFileContext; import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder; -import org.apache.hadoop.hbase.master.TableLockManager; -import org.apache.hadoop.hbase.master.TableLockManager.TableLock; import org.apache.hadoop.hbase.mob.MobCacheConfig; import org.apache.hadoop.hbase.mob.MobConstants; import org.apache.hadoop.hbase.mob.MobFile; @@ -100,8 +102,6 @@ public class HMobStore extends HStore { private volatile long mobScanCellsCount = 0; private volatile long mobScanCellsSize = 0; private HColumnDescriptor family; - private TableLockManager tableLockManager; - private TableName tableLockName; private Map> map = new ConcurrentHashMap>(); private final IdLock keyLock = new IdLock(); // When we add a MOB reference cell to the HFile, we will add 2 tags along with it @@ -126,10 +126,6 @@ public class HMobStore extends HStore { locations.add(HFileArchiveUtil.getStoreArchivePath(conf, tn, MobUtils.getMobRegionInfo(tn) .getEncodedName(), family.getNameAsString())); map.put(Bytes.toString(tn.getName()), locations); - if (region.getRegionServerServices() != null) { - tableLockManager = region.getRegionServerServices().getTableLockManager(); - tableLockName = MobUtils.getTableLockName(getTableName()); - } List tags = new ArrayList<>(2); tags.add(MobConstants.MOB_REF_TAG); Tag tableNameTag = new ArrayBackedTag(TagType.MOB_TABLE_NAME_TAG_TYPE, @@ -482,39 +478,39 @@ public class HMobStore extends HStore { // Acquire a table lock to coordinate. // 1. If no, mark the major compaction as retainDeleteMarkers and continue the compaction. // 2. If the lock is obtained, run the compaction directly. - TableLock lock = null; - if (tableLockManager != null) { - lock = tableLockManager.readLock(tableLockName, "Major compaction in HMobStore"); - } - boolean tableLocked = false; - String tableName = getTableName().getNameAsString(); - if (lock != null) { - try { - LOG.info("Start to acquire a read lock for the table[" + tableName - + "], ready to perform the major compaction"); - lock.acquire(); - tableLocked = true; - } catch (Exception e) { - LOG.error("Fail to lock the table " + tableName, e); - } - } else { - // If the tableLockManager is null, mark the tableLocked as true. - tableLocked = true; - } + EntityLock lock = null; try { - if (!tableLocked) { - LOG.warn("Cannot obtain the table lock, maybe a sweep tool is running on this table[" - + tableName + "], forcing the delete markers to be retained"); + if (region.getRegionServerServices() != null) { + List regionInfos = Collections.singletonList(region.getRegionInfo()); + // regionLock takes shared lock on table too. + lock = region.getRegionServerServices().regionLock(regionInfos, "Mob compaction", null); + int awaitTime = conf.getInt(HRegionServer.REGION_LOCK_AWAIT_TIME_SEC, + HRegionServer.DEFAULT_REGION_LOCK_AWAIT_TIME_SEC); + try { + LOG.info("Start to acquire a lock for MOB major compaction: " + lock); + lock.requestLock(); + lock.await(awaitTime, TimeUnit.SECONDS); + } catch (InterruptedException e) { + LOG.error("Interrupted exception when waiting for lock: " + lock, e); + lock.unlock(); + } + } else { + LOG.warn("Cannot obtain lock because RegionServices not available. Maybe running as " + + "compaction tool."); compaction.getRequest().forceRetainDeleteMarkers(); } + if (lock != null && !lock.isLocked()) { + // remove lock from queue on the master so that if it's granted in future, we don't + // keep holding it until compaction finishes + lock.unlock(); + compaction.getRequest().forceRetainDeleteMarkers(); + LOG.warn("Cannot obtain the table lock, maybe a sweep tool is running on this " + "table[" + + getTableName() + "], forcing the delete markers to be retained"); + } return super.compact(compaction, throughputController, user); } finally { - if (tableLocked && lock != null) { - try { - lock.release(); - } catch (IOException e) { - LOG.error("Fail to release the table lock " + tableName, e); - } + if (lock != null && lock.isLocked()) { + lock.unlock(); } } } else { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 5259961..7e1c980 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -62,6 +62,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.ChoreService; import org.apache.hadoop.hbase.ClockOutOfSyncException; import org.apache.hadoop.hbase.CoordinatedStateManager; @@ -81,12 +82,16 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.YouAreDeadException; import org.apache.hadoop.hbase.ZNodeClearer; import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.ClusterConnection; import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.ConnectionUtils; import org.apache.hadoop.hbase.client.NonceGenerator; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RpcRetryingCallerFactory; +import org.apache.hadoop.hbase.client.locking.EntityLock; +import org.apache.hadoop.hbase.client.locking.LockServiceClient; import org.apache.hadoop.hbase.conf.ConfigurationManager; import org.apache.hadoop.hbase.conf.ConfigurationObserver; import org.apache.hadoop.hbase.coordination.BaseCoordinatedStateManager; @@ -111,7 +116,6 @@ import org.apache.hadoop.hbase.ipc.ServerNotRunningYetException; import org.apache.hadoop.hbase.ipc.ServerRpcController; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.RegionState.State; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer; import org.apache.hadoop.hbase.mob.MobCacheConfig; import org.apache.hadoop.hbase.procedure.RegionServerProcedureManagerHost; @@ -147,6 +151,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.NameStringP import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionServerInfo; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier.RegionSpecifierType; +import org.apache.hadoop.hbase.shaded.protobuf.generated.LockServiceProtos.LockService; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetProcedureResultRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetProcedureResultResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProtos.GetLastFlushedSequenceIdRequest; @@ -214,6 +219,9 @@ import sun.misc.SignalHandler; public class HRegionServer extends HasThread implements RegionServerServices, LastSequenceId, ConfigurationObserver { + public static final String REGION_LOCK_AWAIT_TIME_SEC = + "hbase.regionserver.region.lock.await.time.sec"; + public static final int DEFAULT_REGION_LOCK_AWAIT_TIME_SEC = 300; // 5 min private static final Log LOG = LogFactory.getLog(HRegionServer.class); /** @@ -338,6 +346,7 @@ public class HRegionServer extends HasThread implements // Stub to do region server status calls against the master. private volatile RegionServerStatusService.BlockingInterface rssStub; + private volatile LockService.BlockingInterface lockStub; // RPC client. Used to make the stub above that does region server status checking. RpcClient rpcClient; @@ -464,9 +473,6 @@ public class HRegionServer extends HasThread implements private RegionServerQuotaManager rsQuotaManager; - // Table level lock manager for locking for region operations - protected TableLockManager tableLockManager; - /** * Nonce manager. Nonces are used to make operations like increment and append idempotent * in the case where client doesn't receive the response from a successful operation and @@ -604,9 +610,6 @@ public class HRegionServer extends HasThread implements this.csm.initialize(this); this.csm.start(); - tableLockManager = TableLockManager.createTableLockManager( - conf, zooKeeper, serverName); - masterAddressTracker = new MasterAddressTracker(getZooKeeper(), this); masterAddressTracker.start(); @@ -1134,6 +1137,9 @@ public class HRegionServer extends HasThread implements if (this.rssStub != null) { this.rssStub = null; } + if (this.lockStub != null) { + this.lockStub = null; + } if (this.rpcClient != null) { this.rpcClient.close(); } @@ -1529,11 +1535,6 @@ public class HRegionServer extends HasThread implements return regionServerAccounting; } - @Override - public TableLockManager getTableLockManager() { - return tableLockManager; - } - /* * @param r Region to get RegionLoad for. * @param regionLoadBldr the RegionLoad.Builder, can be null @@ -2385,7 +2386,8 @@ public class HRegionServer extends HasThread implements } ServerName sn = null; long previousLogTime = 0; - RegionServerStatusService.BlockingInterface intf = null; + RegionServerStatusService.BlockingInterface intRssStub = null; + LockService.BlockingInterface intLockStub = null; boolean interrupted = false; try { while (keepLooping()) { @@ -2409,14 +2411,16 @@ public class HRegionServer extends HasThread implements // If we are on the active master, use the shortcut if (this instanceof HMaster && sn.equals(getServerName())) { - intf = ((HMaster)this).getMasterRpcServices(); + intRssStub = ((HMaster)this).getMasterRpcServices(); + intLockStub = ((HMaster)this).getMasterRpcServices(); break; } try { BlockingRpcChannel channel = this.rpcClient.createBlockingRpcChannel(sn, userProvider.getCurrent(), shortOperationTimeout); - intf = RegionServerStatusService.newBlockingStub(channel); + intRssStub = RegionServerStatusService.newBlockingStub(channel); + intLockStub = LockService.newBlockingStub(channel); break; } catch (IOException e) { if (System.currentTimeMillis() > (previousLogTime + 1000)) { @@ -2439,7 +2443,8 @@ public class HRegionServer extends HasThread implements Thread.currentThread().interrupt(); } } - rssStub = intf; + this.rssStub = intRssStub; + this.lockStub = intLockStub; return sn; } @@ -3616,4 +3621,11 @@ public class HRegionServer extends HasThread implements public SecureBulkLoadManager getSecureBulkLoadManager() { return this.secureBulkLoadManager; } + + @Override + public EntityLock regionLock(List regionInfos, String description, + Abortable abort) throws IOException { + return new LockServiceClient(conf, lockStub, clusterConnection.getNonceGenerator()) + .regionLock(regionInfos, description, abort); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java index ce69ad3..48dc441 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeRequest.java @@ -19,12 +19,16 @@ package org.apache.hadoop.hbase.regionserver; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.DroppedSnapshotException; -import org.apache.hadoop.hbase.master.TableLockManager.TableLock; +import org.apache.hadoop.hbase.client.locking.EntityLock; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.ipc.RemoteException; @@ -42,9 +46,10 @@ class RegionMergeRequest implements Runnable { private final HRegion region_b; private final HRegionServer server; private final boolean forcible; - private TableLock tableLock; + private EntityLock tableLock; private final long masterSystemTime; private final User user; + private final int lockAwaitTimeSec; // time to wait for regions locks to be acquired RegionMergeRequest(Region a, Region b, HRegionServer hrs, boolean forcible, long masterSystemTime, User user) { @@ -55,6 +60,9 @@ class RegionMergeRequest implements Runnable { this.forcible = forcible; this.masterSystemTime = masterSystemTime; this.user = user; + this.lockAwaitTimeSec = server.getConfiguration().getInt( + HRegionServer.REGION_LOCK_AWAIT_TIME_SEC, + HRegionServer.DEFAULT_REGION_LOCK_AWAIT_TIME_SEC); } @Override @@ -72,26 +80,27 @@ class RegionMergeRequest implements Runnable { } try { final long startTime = EnvironmentEdgeManager.currentTime(); - RegionMergeTransactionImpl mt = new RegionMergeTransactionImpl(region_a, + RegionMergeTransactionImpl regionMergeTransaction = new RegionMergeTransactionImpl(region_a, region_b, forcible, masterSystemTime); //acquire a shared read lock on the table, so that table schema modifications //do not happen concurrently - tableLock = server.getTableLockManager().readLock(region_a.getTableDesc().getTableName() - , "MERGE_REGIONS:" + region_a.getRegionInfo().getRegionNameAsString() + ", " + - region_b.getRegionInfo().getRegionNameAsString()); - try { - tableLock.acquire(); - } catch (IOException ex) { - tableLock = null; - throw ex; + List regionInfos = new ArrayList<>(2); + regionInfos.add(region_a.getRegionInfo()); + regionInfos.add(region_b.getRegionInfo()); + tableLock = server.regionLock(regionInfos, + "MERGE_REGIONS:" + regionInfos.get(0).getRegionNameAsString() + ", " + + regionInfos.get(1).getRegionNameAsString(), null); + tableLock.requestLock(); + if(!tableLock.await(lockAwaitTimeSec, TimeUnit.SECONDS)) { + tableLock.unlock(); + return; } - // If prepare does not return true, for some reason -- logged inside in // the prepare call -- we are not ready to merge just now. Just return. - if (!mt.prepare(this.server)) return; + if (!regionMergeTransaction.prepare(this.server)) return; try { - mt.execute(this.server, this.server, this.user); + regionMergeTransaction.execute(this.server, this.server, this.user); } catch (Exception e) { if (this.server.isStopping() || this.server.isStopped()) { LOG.info( @@ -104,50 +113,45 @@ class RegionMergeRequest implements Runnable { server.abort("Replay of WAL required. Forcing server shutdown", e); return; } - try { - LOG.warn("Running rollback/cleanup of failed merge of " - + region_a +" and "+ region_b + "; " + e.getMessage(), e); - if (mt.rollback(this.server, this.server)) { - LOG.info("Successful rollback of failed merge of " - + region_a +" and "+ region_b); - } else { - this.server.abort("Abort; we got an error after point-of-no-return" - + "when merging " + region_a + " and " + region_b); - } - } catch (RuntimeException ee) { - String msg = "Failed rollback of failed merge of " - + region_a +" and "+ region_b + " -- aborting server"; - // If failed rollback, kill this server to avoid having a hole in - // table. - LOG.info(msg, ee); - this.server.abort(msg); - } + LOG.warn("Running rollback/cleanup of failed merge of " + + region_a +" and "+ region_b + "; " + e.getMessage(), e); + tryRollback(regionMergeTransaction); return; } LOG.info("Regions merged, hbase:meta updated, and report to master. region_a=" + region_a + ", region_b=" + region_b + ",merged region=" - + mt.getMergedRegionInfo().getRegionNameAsString() + + regionMergeTransaction.getMergedRegionInfo().getRegionNameAsString() + ". Region merge took " + StringUtils.formatTimeDiff(EnvironmentEdgeManager.currentTime(), startTime)); - } catch (IOException ex) { + } catch (Exception ex) { ex = ex instanceof RemoteException ? ((RemoteException) ex).unwrapRemoteException() : ex; LOG.error("Merge failed " + this, ex); server.checkFileSystem(); } finally { - releaseTableLock(); + try { + tableLock.unlock(); + } catch (IOException e) { + // do nothing, lock will be released eventually. + } } } - protected void releaseTableLock() { - if (this.tableLock != null) { - try { - this.tableLock.release(); - } catch (IOException ex) { - LOG.error("Could not release the table lock (something is really wrong). " - + "Aborting this server to avoid holding the lock forever."); - this.server.abort("Abort; we got an error when releasing the table lock " - + "on " + region_a.getRegionInfo().getRegionNameAsString()); + void tryRollback(RegionMergeTransaction regionMergeTransaction) { + try { + if (regionMergeTransaction.rollback(this.server, this.server)) { + LOG.info("Successful rollback of failed merge of " + + region_a +" and "+ region_b); + } else { + this.server.abort("Abort; we got an error after point-of-no-return" + + "when merging " + region_a + " and " + region_b); } + } catch (Exception e) { + String msg = "Failed rollback of failed merge of " + + region_a +" and "+ region_b + " -- aborting server"; + // If failed rollback, kill this server to avoid having a hole in + // table. + LOG.info(msg, e); + this.server.abort(msg); } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java index 5a6c7ed..c92124c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java @@ -25,14 +25,15 @@ import java.util.Set; import java.util.concurrent.ConcurrentMap; import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; +import org.apache.hadoop.hbase.client.locking.EntityLock; import org.apache.hadoop.hbase.executor.ExecutorService; import org.apache.hadoop.hbase.ipc.RpcServerInterface; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProtos.RegionStateTransition.TransitionCode; import org.apache.hadoop.hbase.quotas.RegionServerQuotaManager; import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController; @@ -77,11 +78,6 @@ public interface RegionServerServices extends OnlineRegions, FavoredNodesForRegi RegionServerAccounting getRegionServerAccounting(); /** - * @return RegionServer's instance of {@link TableLockManager} - */ - TableLockManager getTableLockManager(); - - /** * @return RegionServer's instance of {@link RegionServerQuotaManager} */ RegionServerQuotaManager getRegionServerQuotaManager(); @@ -271,4 +267,10 @@ public interface RegionServerServices extends OnlineRegions, FavoredNodesForRegi * @return the metrics tracker for the region server */ MetricsRegionServer getMetrics(); + + /** + * Master based locks on namespaces/tables/regions. + */ + EntityLock regionLock(List regionInfos, String description, + Abortable abort) throws IOException; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java index defffe3..553f756 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java @@ -129,7 +129,6 @@ import org.apache.hadoop.hbase.util.hbck.HFileCorruptionChecker; import org.apache.hadoop.hbase.util.hbck.ReplicationChecker; import org.apache.hadoop.hbase.util.hbck.TableIntegrityErrorHandler; import org.apache.hadoop.hbase.util.hbck.TableIntegrityErrorHandlerImpl; -import org.apache.hadoop.hbase.util.hbck.TableLockChecker; import org.apache.hadoop.hbase.wal.WAL; import org.apache.hadoop.hbase.wal.WALFactory; import org.apache.hadoop.hbase.wal.WALSplitter; @@ -249,7 +248,6 @@ public class HBaseFsck extends Configured implements Closeable { private boolean fixSplitParents = false; // fix lingering split parents private boolean fixReferenceFiles = false; // fix lingering reference store file private boolean fixEmptyMetaCells = false; // fix (remove) empty REGIONINFO_QUALIFIER rows - private boolean fixTableLocks = false; // fix table locks which are expired private boolean fixReplication = false; // fix undeleted replication queues for removed peer private boolean fixAny = false; // Set to true if any of the fix is required. @@ -768,8 +766,6 @@ public class HBaseFsck extends Configured implements Closeable { checkRegionBoundaries(); } - checkAndFixTableLocks(); - checkAndFixReplication(); // Remove the hbck znode @@ -1537,7 +1533,7 @@ public class HBaseFsck extends Configured implements Closeable { /** * Removes the empty Meta recovery WAL directory. - * @param walFactoryID A unique identifier for WAL factory which was used by Filesystem to make a + * @param walFactoryId A unique identifier for WAL factory which was used by Filesystem to make a * Meta recovery WAL directory inside WAL directory path. */ private void removeHBCKMetaRecoveryWALDir(String walFactoryId) throws IOException { @@ -3342,15 +3338,6 @@ public class HBaseFsck extends Configured implements Closeable { return hbi; } - private void checkAndFixTableLocks() throws IOException { - TableLockChecker checker = new TableLockChecker(zkw, errors); - checker.checkTableLocks(); - - if (this.fixTableLocks) { - checker.fixExpiredTableLocks(); - } - } - private void checkAndFixReplication() throws IOException { ReplicationChecker checker = new ReplicationChecker(getConf(), zkw, connection, errors); checker.checkUnDeletedQueues(); @@ -4316,15 +4303,6 @@ public class HBaseFsck extends Configured implements Closeable { } /** - * Set table locks fix mode. - * Delete table locks held for a long time - */ - public void setFixTableLocks(boolean shouldFix) { - fixTableLocks = shouldFix; - fixAny |= shouldFix; - } - - /** * Set replication fix mode. */ public void setFixReplication(boolean shouldFix) { @@ -4583,14 +4561,10 @@ public class HBaseFsck extends Configured implements Closeable { out.println(""); out.println(" Metadata Repair shortcuts"); out.println(" -repair Shortcut for -fixAssignments -fixMeta -fixHdfsHoles " + - "-fixHdfsOrphans -fixHdfsOverlaps -fixVersionFile -sidelineBigOverlaps -fixReferenceFiles -fixTableLocks"); + "-fixHdfsOrphans -fixHdfsOverlaps -fixVersionFile -sidelineBigOverlaps -fixReferenceFiles"); out.println(" -repairHoles Shortcut for -fixAssignments -fixMeta -fixHdfsHoles"); out.println(""); - out.println(" Table lock options"); - out.println(" -fixTableLocks Deletes table locks held for a long time (hbase.table.lock.expire.ms, 10min by default)"); - - out.println(""); out.println(" Replication options"); out.println(" -fixReplication Deletes replication queues for removed peers"); @@ -4728,7 +4702,6 @@ public class HBaseFsck extends Configured implements Closeable { setFixSplitParents(false); setCheckHdfs(true); setFixReferenceFiles(true); - setFixTableLocks(true); } else if (cmd.equals("-repairHoles")) { // this will make all missing hdfs regions available but may lose data setFixHdfsHoles(true); @@ -4775,8 +4748,6 @@ public class HBaseFsck extends Configured implements Closeable { setCheckMetaOnly(); } else if (cmd.equals("-boundaries")) { setRegionBoundariesCheck(); - } else if (cmd.equals("-fixTableLocks")) { - setFixTableLocks(true); } else if (cmd.equals("-fixReplication")) { setFixReplication(true); } else if (cmd.startsWith("-")) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/hbck/TableLockChecker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/hbck/TableLockChecker.java deleted file mode 100644 index 6777546..0000000 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/hbck/TableLockChecker.java +++ /dev/null @@ -1,87 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.hadoop.hbase.util.hbck; - -import java.io.IOException; - -import org.apache.hadoop.hbase.InterProcessLock.MetadataHandler; -import org.apache.hadoop.hbase.master.TableLockManager; -import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; -import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos; -import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.util.HBaseFsck; -import org.apache.hadoop.hbase.util.HBaseFsck.ErrorReporter; -import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; - -/** - * Utility to check and fix table locks. Need zookeeper connection. - */ -public class TableLockChecker { - - private ZooKeeperWatcher zkWatcher; - private ErrorReporter errorReporter; - long expireTimeout; - - public TableLockChecker(ZooKeeperWatcher zkWatcher, ErrorReporter errorReporter) { - this.zkWatcher = zkWatcher; - this.errorReporter = errorReporter; - expireTimeout = zkWatcher.getConfiguration().getLong( - TableLockManager.TABLE_LOCK_EXPIRE_TIMEOUT, - TableLockManager.DEFAULT_TABLE_LOCK_EXPIRE_TIMEOUT_MS); - } - - public void checkTableLocks() throws IOException { - TableLockManager tableLockManager - = TableLockManager.createTableLockManager(zkWatcher.getConfiguration(), zkWatcher, null); - final long expireDate = EnvironmentEdgeManager.currentTime() - expireTimeout; - - MetadataHandler handler = new MetadataHandler() { - @Override - public void handleMetadata(byte[] ownerMetadata) { - ZooKeeperProtos.TableLock data = TableLockManager.fromBytes(ownerMetadata); - String msg = "Table lock acquire attempt found:"; - if (data != null) { - msg = msg + - String.format("[tableName=%s:%s, lockOwner=%s, threadId=%s, " + - "purpose=%s, isShared=%s, createTime=%s]", - data.getTableName().getNamespace().toStringUtf8(), - data.getTableName().getQualifier().toStringUtf8(), - ProtobufUtil.toServerName(data.getLockOwner()), data.getThreadId(), - data.getPurpose(), data.getIsShared(), data.getCreateTime()); - } - - if (data != null && data.hasCreateTime() && data.getCreateTime() < expireDate) { - errorReporter.reportError(HBaseFsck.ErrorReporter.ERROR_CODE.EXPIRED_TABLE_LOCK, msg); - } else { - errorReporter.print(msg); - } - } - }; - - tableLockManager.visitAllLocks(handler); - } - - public void fixExpiredTableLocks() throws IOException { - TableLockManager tableLockManager - = TableLockManager.createTableLockManager(zkWatcher.getConfiguration(), zkWatcher, null); - - tableLockManager.reapAllExpiredLocks(); - } - -} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java index 404c9ae..5e2a70f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java @@ -32,11 +32,10 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.hbase.client.ClusterConnection; +import org.apache.hadoop.hbase.client.locking.EntityLock; import org.apache.hadoop.hbase.executor.ExecutorService; import org.apache.hadoop.hbase.fs.HFileSystem; import org.apache.hadoop.hbase.ipc.RpcServerInterface; -import org.apache.hadoop.hbase.master.TableLockManager; -import org.apache.hadoop.hbase.master.TableLockManager.NullTableLockManager; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProtos.RegionStateTransition.TransitionCode; import org.apache.hadoop.hbase.quotas.RegionServerQuotaManager; @@ -190,11 +189,6 @@ public class MockRegionServerServices implements RegionServerServices { } @Override - public TableLockManager getTableLockManager() { - return new NullTableLockManager(); - } - - @Override public RegionServerQuotaManager getRegionServerQuotaManager() { return null; } @@ -353,6 +347,12 @@ public class MockRegionServerServices implements RegionServerServices { } @Override + public EntityLock regionLock(List regionInfos, String description, Abortable abort) + throws IOException { + return null; + } + + @Override public SecureBulkLoadManager getSecureBulkLoadManager() { return null; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java index 28bf14a..7f45243 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java @@ -310,11 +310,6 @@ public class MockNoopMasterServices implements MasterServices, Server { } @Override - public TableLockManager getTableLockManager() { - return null; - } - - @Override public TableStateManager getTableStateManager() { return null; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java index 950ec92..f09197f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java @@ -31,6 +31,7 @@ import java.util.concurrent.ConcurrentSkipListMap; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.CellScannable; import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.ChoreService; @@ -47,7 +48,6 @@ import org.apache.hadoop.hbase.client.locking.EntityLock; import org.apache.hadoop.hbase.executor.ExecutorService; import org.apache.hadoop.hbase.ipc.HBaseRpcController; import org.apache.hadoop.hbase.ipc.RpcServerInterface; -import org.apache.hadoop.hbase.master.TableLockManager.NullTableLockManager; import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.CloseRegionRequest; @@ -336,11 +336,6 @@ ClientProtos.ClientService.BlockingInterface, RegionServerServices { } @Override - public TableLockManager getTableLockManager() { - return new NullTableLockManager(); - } - - @Override public RegionServerQuotaManager getRegionServerQuotaManager() { return null; } @@ -712,6 +707,12 @@ ClientProtos.ClientService.BlockingInterface, RegionServerServices { } @Override + public EntityLock regionLock(List regionInfos, String description, Abortable abort) + throws IOException { + return null; + } + + @Override public PrepareBulkLoadResponse prepareBulkLoad(RpcController controller, PrepareBulkLoadRequest request) throws ServiceException { return null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestTableLockManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestTableLockManager.java deleted file mode 100644 index 94b2bc1..0000000 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestTableLockManager.java +++ /dev/null @@ -1,433 +0,0 @@ -/* - * Copyright The Apache Software Foundation - * - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.master; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -import java.io.IOException; -import java.util.List; -import java.util.Random; -import java.util.concurrent.Callable; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.apache.hadoop.hbase.ChoreService; -import org.apache.hadoop.hbase.HBaseConfiguration; -import org.apache.hadoop.hbase.HBaseTestingUtility; -import org.apache.hadoop.hbase.HColumnDescriptor; -import org.apache.hadoop.hbase.HRegionInfo; -import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.InterProcessLock; -import org.apache.hadoop.hbase.NotServingRegionException; -import org.apache.hadoop.hbase.ScheduledChore; -import org.apache.hadoop.hbase.ServerName; -import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.TableNotDisabledException; -import org.apache.hadoop.hbase.Waiter; -import org.apache.hadoop.hbase.client.Admin; -import org.apache.hadoop.hbase.coprocessor.BaseMasterObserver; -import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; -import org.apache.hadoop.hbase.coprocessor.ObserverContext; -import org.apache.hadoop.hbase.regionserver.CompactingMemStore; -import org.apache.hadoop.hbase.regionserver.HRegion; -import org.apache.hadoop.hbase.testclassification.LargeTests; -import org.apache.hadoop.hbase.testclassification.MasterTests; -import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.LoadTestTool; -import org.apache.hadoop.hbase.util.StoppableImplementation; -import org.apache.hadoop.hbase.util.Threads; -import org.apache.hadoop.hbase.zookeeper.ZKUtil; -import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.experimental.categories.Category; - -/** - * Tests the default table lock manager - */ -@Category({MasterTests.class, LargeTests.class}) -public class TestTableLockManager { - - private static final Log LOG = - LogFactory.getLog(TestTableLockManager.class); - - private static final TableName TABLE_NAME = - TableName.valueOf("TestTableLevelLocks"); - - private static final byte[] FAMILY = Bytes.toBytes("f1"); - - private static final byte[] NEW_FAMILY = Bytes.toBytes("f2"); - - private final HBaseTestingUtility TEST_UTIL = - new HBaseTestingUtility(); - - private static final CountDownLatch deleteColumn = new CountDownLatch(1); - private static final CountDownLatch addColumn = new CountDownLatch(1); - - public void prepareMiniCluster() throws Exception { - TEST_UTIL.startMiniCluster(2); - TEST_UTIL.createTable(TABLE_NAME, FAMILY); - } - - public void prepareMiniZkCluster() throws Exception { - TEST_UTIL.startMiniZKCluster(1); - } - - @Before - public void setUp() throws IOException { - TEST_UTIL.getConfiguration().set(CompactingMemStore.COMPACTING_MEMSTORE_TYPE_KEY, - String.valueOf(HColumnDescriptor.MemoryCompaction.NONE)); - } - - @After - public void tearDown() throws Exception { - TEST_UTIL.shutdownMiniCluster(); - } - - public static class TestLockTimeoutExceptionMasterObserver extends BaseMasterObserver { - @Override - public void preDeleteColumnFamilyAction(ObserverContext ctx, - TableName tableName, byte[] columnFamily) throws IOException { - deleteColumn.countDown(); - } - @Override - public void postCompletedDeleteColumnFamilyAction( - ObserverContext ctx, - TableName tableName, byte[] columnFamily) throws IOException { - Threads.sleep(10000); - } - - @Override - public void preAddColumnFamilyAction(ObserverContext ctx, - TableName tableName, HColumnDescriptor columnFamily) throws IOException { - fail("Add column should have timeouted out for acquiring the table lock"); - } - } - - @Test(timeout = 600000) - public void testAlterAndDisable() throws Exception { - prepareMiniCluster(); - // Send a request to alter a table, then sleep during - // the alteration phase. In the mean time, from another - // thread, send a request to disable, and then delete a table. - - HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.getMasterCoprocessorHost().load(TestAlterAndDisableMasterObserver.class, - 0, TEST_UTIL.getConfiguration()); - - ExecutorService executor = Executors.newFixedThreadPool(2); - Future alterTableFuture = executor.submit(new Callable() { - @Override - public Object call() throws Exception { - Admin admin = TEST_UTIL.getHBaseAdmin(); - admin.addColumnFamily(TABLE_NAME, new HColumnDescriptor(NEW_FAMILY)); - LOG.info("Added new column family"); - HTableDescriptor tableDesc = admin.getTableDescriptor(TABLE_NAME); - assertTrue(tableDesc.getFamiliesKeys().contains(NEW_FAMILY)); - return null; - } - }); - Future disableTableFuture = executor.submit(new Callable() { - @Override - public Object call() throws Exception { - Admin admin = TEST_UTIL.getHBaseAdmin(); - admin.disableTable(TABLE_NAME); - assertTrue(admin.isTableDisabled(TABLE_NAME)); - admin.deleteTable(TABLE_NAME); - assertFalse(admin.tableExists(TABLE_NAME)); - return null; - } - }); - - try { - disableTableFuture.get(); - alterTableFuture.get(); - } catch (ExecutionException e) { - if (e.getCause() instanceof AssertionError) { - throw (AssertionError) e.getCause(); - } - throw e; - } - } - - public static class TestAlterAndDisableMasterObserver extends BaseMasterObserver { - @Override - public void preAddColumnFamilyAction(ObserverContext ctx, - TableName tableName, HColumnDescriptor columnFamily) throws IOException { - LOG.debug("addColumn called"); - addColumn.countDown(); - } - - @Override - public void postCompletedAddColumnFamilyAction( - ObserverContext ctx, - TableName tableName, HColumnDescriptor columnFamily) throws IOException { - Threads.sleep(6000); - try { - ctx.getEnvironment().getMasterServices().checkTableModifiable(tableName); - } catch(TableNotDisabledException expected) { - //pass - return; - } catch(IOException ex) { - } - fail("was expecting the table to be enabled"); - } - - @Override - public void preDisableTable(ObserverContext ctx, - TableName tableName) throws IOException { - try { - LOG.debug("Waiting for addColumn to be processed first"); - //wait for addColumn to be processed first - addColumn.await(); - LOG.debug("addColumn started, we can continue"); - } catch (InterruptedException ex) { - LOG.warn("Sleep interrupted while waiting for addColumn countdown"); - } - } - - @Override - public void postDisableTableHandler(ObserverContext ctx, - TableName tableName) throws IOException { - Threads.sleep(3000); - } - } - - @Test(timeout = 600000) - public void testDelete() throws Exception { - prepareMiniCluster(); - - Admin admin = TEST_UTIL.getHBaseAdmin(); - admin.disableTable(TABLE_NAME); - admin.deleteTable(TABLE_NAME); - - //ensure that znode for the table node has been deleted - final ZooKeeperWatcher zkWatcher = TEST_UTIL.getZooKeeperWatcher(); - final String znode = ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, - TABLE_NAME.getNameAsString()); - - TEST_UTIL.waitFor(5000, new Waiter.Predicate() { - @Override - public boolean evaluate() throws Exception { - int ver = ZKUtil.checkExists(zkWatcher, znode); - return ver < 0; - } - }); - int ver = ZKUtil.checkExists(zkWatcher, - ZKUtil.joinZNode(zkWatcher.znodePaths.tableLockZNode, TABLE_NAME.getNameAsString())); - assertTrue("Unexpected znode version " + ver, ver < 0); - - } - - public class TableLockCounter implements InterProcessLock.MetadataHandler { - - private int lockCount = 0; - - @Override - public void handleMetadata(byte[] metadata) { - lockCount++; - } - - public void reset() { - lockCount = 0; - } - - public int getLockCount() { - return lockCount; - } - } - - @Test(timeout = 600000) - public void testReapAllTableLocks() throws Exception { - prepareMiniZkCluster(); - ServerName serverName = ServerName.valueOf("localhost:10000", 0); - final TableLockManager lockManager = TableLockManager.createTableLockManager( - TEST_UTIL.getConfiguration(), TEST_UTIL.getZooKeeperWatcher(), serverName); - - String tables[] = {"table1", "table2", "table3", "table4"}; - ExecutorService executor = Executors.newFixedThreadPool(6); - - final CountDownLatch writeLocksObtained = new CountDownLatch(4); - final CountDownLatch writeLocksAttempted = new CountDownLatch(10); - //TODO: read lock tables - - //6 threads will be stuck waiting for the table lock - for (int i = 0; i < tables.length; i++) { - final String table = tables[i]; - for (int j = 0; j < i+1; j++) { //i+1 write locks attempted for table[i] - executor.submit(new Callable() { - @Override - public Void call() throws Exception { - writeLocksAttempted.countDown(); - lockManager.writeLock(TableName.valueOf(table), - "testReapAllTableLocks").acquire(); - writeLocksObtained.countDown(); - return null; - } - }); - } - } - - writeLocksObtained.await(); - writeLocksAttempted.await(); - - TableLockCounter counter = new TableLockCounter(); - do { - counter.reset(); - lockManager.visitAllLocks(counter); - Thread.sleep(10); - } while (counter.getLockCount() != 10); - - //now reap all table locks - lockManager.reapWriteLocks(); - TEST_UTIL.getConfiguration().setInt(TableLockManager.TABLE_WRITE_LOCK_TIMEOUT_MS, 0); - TableLockManager zeroTimeoutLockManager = TableLockManager.createTableLockManager( - TEST_UTIL.getConfiguration(), TEST_UTIL.getZooKeeperWatcher(), serverName); - - //should not throw table lock timeout exception - zeroTimeoutLockManager.writeLock( - TableName.valueOf(tables[tables.length - 1]), - "zero timeout") - .acquire(); - - executor.shutdownNow(); - } - - @Test(timeout = 600000) - public void testTableReadLock() throws Exception { - // test plan: write some data to the table. Continuously alter the table and - // force splits - // concurrently until we have 5 regions. verify the data just in case. - // Every region should contain the same table descriptor - // This is not an exact test - prepareMiniCluster(); - LoadTestTool loadTool = new LoadTestTool(); - loadTool.setConf(TEST_UTIL.getConfiguration()); - int numKeys = 10000; - final TableName tableName = TableName.valueOf("testTableReadLock"); - final Admin admin = TEST_UTIL.getHBaseAdmin(); - final HTableDescriptor desc = new HTableDescriptor(tableName); - final byte[] family = Bytes.toBytes("test_cf"); - desc.addFamily(new HColumnDescriptor(family)); - admin.createTable(desc); // create with one region - - // write some data, not much - int ret = loadTool.run(new String[] { "-tn", tableName.getNameAsString(), "-write", - String.format("%d:%d:%d", 1, 10, 10), "-num_keys", String.valueOf(numKeys), "-skip_init" }); - if (0 != ret) { - String errorMsg = "Load failed with error code " + ret; - LOG.error(errorMsg); - fail(errorMsg); - } - - int familyValues = admin.getTableDescriptor(tableName).getFamily(family).getValues().size(); - StoppableImplementation stopper = new StoppableImplementation(); - final ChoreService choreService = new ChoreService("TEST_SERVER_NAME"); - - //alter table every 10 sec - ScheduledChore alterThread = new ScheduledChore("Alter Chore", stopper, 10000) { - @Override - protected void chore() { - Random random = new Random(); - try { - HTableDescriptor htd = admin.getTableDescriptor(tableName); - String val = String.valueOf(random.nextInt()); - htd.getFamily(family).setValue(val, val); - desc.getFamily(family).setValue(val, val); // save it for later - // control - admin.modifyTable(tableName, htd); - } catch (Exception ex) { - LOG.warn("Caught exception", ex); - fail(ex.getMessage()); - } - } - }; - - //split table every 5 sec - ScheduledChore splitThread = new ScheduledChore("Split thread", stopper, 5000) { - @Override - public void chore() { - try { - HRegion region = TEST_UTIL.getSplittableRegion(tableName, -1); - if (region != null) { - byte[] regionName = region.getRegionInfo().getRegionName(); - admin.flushRegion(regionName); - admin.compactRegion(regionName); - admin.splitRegion(regionName); - } else { - LOG.warn("Could not find suitable region for the table. Possibly the " + - "region got closed and the attempts got over before " + - "the region could have got reassigned."); - } - } catch (NotServingRegionException nsre) { - // the region may be in transition - LOG.warn("Caught exception", nsre); - } catch (Exception ex) { - LOG.warn("Caught exception", ex); - fail(ex.getMessage()); - } - } - }; - - choreService.scheduleChore(alterThread); - choreService.scheduleChore(splitThread); - TEST_UTIL.waitTableEnabled(tableName); - while (true) { - List regions = admin.getTableRegions(tableName); - LOG.info(String.format("Table #regions: %d regions: %s:", regions.size(), regions)); - assertEquals(admin.getTableDescriptor(tableName), desc); - for (HRegion region : TEST_UTIL.getMiniHBaseCluster().getRegions(tableName)) { - HTableDescriptor regionTableDesc = region.getTableDesc(); - assertEquals(desc, regionTableDesc); - } - if (regions.size() >= 5) { - break; - } - Threads.sleep(1000); - } - stopper.stop("test finished"); - - int newFamilyValues = admin.getTableDescriptor(tableName).getFamily(family).getValues().size(); - LOG.info(String.format("Altered the table %d times", newFamilyValues - familyValues)); - assertTrue(newFamilyValues > familyValues); // at least one alter went - // through - - ret = loadTool.run(new String[] { "-tn", tableName.getNameAsString(), "-read", "100:10", - "-num_keys", String.valueOf(numKeys), "-skip_init" }); - if (0 != ret) { - String errorMsg = "Verify failed with error code " + ret; - LOG.error(errorMsg); - fail(errorMsg); - } - - admin.close(); - choreService.shutdown(); - } - -} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/locking/TestLockProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/locking/TestLockProcedure.java index be80646..f09ac07 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/locking/TestLockProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/locking/TestLockProcedure.java @@ -32,7 +32,6 @@ import org.apache.hadoop.hbase.client.locking.LockServiceClient; import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.master.MasterRpcServices; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; @@ -97,7 +96,6 @@ public class TestLockProcedure { conf.setBoolean("hbase.procedure.check.owner.set", false); // since rpc user will be null conf.setInt(LockProcedure.REMOTE_LOCKS_TIMEOUT_MS_CONF, HEARTBEAT_TIMEOUT); conf.setInt(LockProcedure.LOCAL_MASTER_LOCKS_TIMEOUT_MS_CONF, LOCAL_LOCKS_TIMEOUT); - conf.setInt(TableLockManager.TABLE_LOCK_EXPIRE_TIMEOUT, ZK_EXPIRATION); } @BeforeClass @@ -386,12 +384,6 @@ public class TestLockProcedure { ProcedureTestingUtility.waitProcedure(procExec, procId); assertEquals(false, procExec.isRunning()); ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false); - // Remove zk lock node otherwise recovered lock will keep waiting on it. Remove - // both exclusive and non-exclusive (the table shared lock that the region takes). - // Have to pause to let the locks 'expire' up in zk. See above configs where we - // set explict zk timeout on locks. - Thread.sleep(ZK_EXPIRATION + HEARTBEAT_TIMEOUT); - UTIL.getMiniHBaseCluster().getMaster().getTableLockManager().reapAllExpiredLocks(); ProcedureTestingUtility.restart(procExec); while (!procExec.isStarted(procId)) { Thread.sleep(250); @@ -442,7 +434,6 @@ public class TestLockProcedure { assertEquals(false, procExec.isRunning()); ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false); // remove zk lock node otherwise recovered lock will keep waiting on it. - UTIL.getMiniHBaseCluster().getMaster().getTableLockManager().reapWriteLocks(); ProcedureTestingUtility.restart(procExec); while (!procExec.isStarted(lockProc.getProcId())) { Thread.sleep(250); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureSchedulerPerformanceEvaluation.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureSchedulerPerformanceEvaluation.java index a63ac03..6463225 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureSchedulerPerformanceEvaluation.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureSchedulerPerformanceEvaluation.java @@ -28,7 +28,6 @@ import org.apache.commons.lang.ArrayUtils; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility.TestProcedure; import org.apache.hadoop.hbase.procedure2.util.StringUtils; @@ -213,9 +212,9 @@ public class MasterProcedureSchedulerPerformanceEvaluation extends AbstractHBase continue; } - if (proc.acquireLock(null)) { + if (proc.acquire(null)) { completed.incrementAndGet(); - proc.releaseLock(null); + proc.release(null); } else { procedureScheduler.yield(proc); } @@ -243,8 +242,7 @@ public class MasterProcedureSchedulerPerformanceEvaluation extends AbstractHBase @Override protected int doWork() throws Exception { - procedureScheduler = new MasterProcedureScheduler( - UTIL.getConfiguration(), new TableLockManager.NullTableLockManager()); + procedureScheduler = new MasterProcedureScheduler(UTIL.getConfiguration()); procedureScheduler.start(); setupOperations(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java index 7397168..dc60710 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java @@ -18,7 +18,6 @@ package org.apache.hadoop.hbase.master.procedure; -import java.io.File; import java.io.IOException; import java.util.Arrays; @@ -28,17 +27,13 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HRegionInfo; -import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureEvent; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility.TestProcedure; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.zookeeper.MiniZooKeeperCluster; -import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -58,7 +53,7 @@ public class TestMasterProcedureScheduler { @Before public void setUp() throws IOException { conf = HBaseConfiguration.create(); - queue = new MasterProcedureScheduler(conf, new TableLockManager.NullTableLockManager()); + queue = new MasterProcedureScheduler(conf); queue.start(); } @@ -334,35 +329,20 @@ public class TestMasterProcedureScheduler { } @Test - public void testSharedZkLock() throws Exception { + public void testSharedLock() throws Exception { final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); - final String dir = TEST_UTIL.getDataTestDir("TestSharedZkLock").toString(); - MiniZooKeeperCluster zkCluster = new MiniZooKeeperCluster(conf); - int zkPort = zkCluster.startup(new File(dir)); - try { - conf.set("hbase.zookeeper.quorum", "localhost:" + zkPort); - - ZooKeeperWatcher zkw = new ZooKeeperWatcher(conf, "testSchedWithZkLock", null, false); - ServerName mockName = ServerName.valueOf("localhost", 60000, 1); - MasterProcedureScheduler procQueue = new MasterProcedureScheduler( - conf, - TableLockManager.createTableLockManager(conf, zkw, mockName)); - - final TableName tableName = TableName.valueOf("testtb"); - TestTableProcedure procA = - new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.READ); - TestTableProcedure procB = - new TestTableProcedure(2, tableName, TableProcedureInterface.TableOperationType.READ); + final TableName tableName = TableName.valueOf("testtb"); + TestTableProcedure procA = + new TestTableProcedure(1, tableName, TableProcedureInterface.TableOperationType.READ); + TestTableProcedure procB = + new TestTableProcedure(2, tableName, TableProcedureInterface.TableOperationType.READ); - assertTrue(procQueue.tryAcquireTableSharedLock(procA, tableName)); - assertTrue(procQueue.tryAcquireTableSharedLock(procB, tableName)); + assertTrue(queue.tryAcquireTableSharedLock(procA, tableName)); + assertTrue(queue.tryAcquireTableSharedLock(procB, tableName)); - procQueue.releaseTableSharedLock(procA, tableName); - procQueue.releaseTableSharedLock(procB, tableName); - } finally { - zkCluster.shutdown(); - } + queue.releaseTableSharedLock(procA, tableName); + queue.releaseTableSharedLock(procB, tableName); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java index 511b3de..a8192be 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java @@ -30,7 +30,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.master.procedure.TestMasterProcedureScheduler.TestTableProcedure; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.util.StringUtils; @@ -44,7 +43,6 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @Category({MasterTests.class, MediumTests.class}) @@ -59,7 +57,7 @@ public class TestMasterProcedureSchedulerConcurrency { conf = HBaseConfiguration.create(); conf.set(CompactingMemStore.COMPACTING_MEMSTORE_TYPE_KEY, String.valueOf(HColumnDescriptor.MemoryCompaction.NONE)); - queue = new MasterProcedureScheduler(conf, new TableLockManager.NullTableLockManager()); + queue = new MasterProcedureScheduler(conf); queue.start(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsckOneRS.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsckOneRS.java index fcd5258..257dfc5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsckOneRS.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsckOneRS.java @@ -48,7 +48,6 @@ import org.apache.hadoop.hbase.io.hfile.TestHFile; import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.master.RegionStates; -import org.apache.hadoop.hbase.master.TableLockManager; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.master.procedure.SplitTableRegionProcedure; import org.apache.hadoop.hbase.regionserver.HRegion; @@ -1479,85 +1478,6 @@ public class TestHBaseFsckOneRS extends BaseTestHBaseFsck { } @Test(timeout=180000) - public void testCheckTableLocks() throws Exception { - IncrementingEnvironmentEdge edge = new IncrementingEnvironmentEdge(0); - EnvironmentEdgeManager.injectEdge(edge); - // check no errors - HBaseFsck hbck = doFsck(conf, false); - assertNoErrors(hbck); - - ServerName mockName = ServerName.valueOf("localhost", 60000, 1); - final TableName tableName = TableName.valueOf("foo"); - - // obtain one lock - final TableLockManager tableLockManager = - TableLockManager.createTableLockManager(conf, TEST_UTIL.getZooKeeperWatcher(), mockName); - TableLockManager.TableLock - writeLock = tableLockManager.writeLock(tableName, "testCheckTableLocks"); - writeLock.acquire(); - hbck = doFsck(conf, false); - assertNoErrors(hbck); // should not have expired, no problems - - edge.incrementTime(conf.getLong(TableLockManager.TABLE_LOCK_EXPIRE_TIMEOUT, - TableLockManager.DEFAULT_TABLE_LOCK_EXPIRE_TIMEOUT_MS)); // let table lock expire - - hbck = doFsck(conf, false); - assertErrors(hbck, new HBaseFsck.ErrorReporter.ERROR_CODE[] { - HBaseFsck.ErrorReporter.ERROR_CODE.EXPIRED_TABLE_LOCK}); - - final CountDownLatch latch = new CountDownLatch(1); - new Thread() { - @Override - public void run() { - TableLockManager.TableLock - readLock = tableLockManager.writeLock(tableName, "testCheckTableLocks"); - try { - latch.countDown(); - readLock.acquire(); - } catch (IOException ex) { - fail(); - } catch (IllegalStateException ex) { - return; // expected, since this will be reaped under us. - } - fail("should not have come here"); - }; - }.start(); - - latch.await(); // wait until thread starts - Threads.sleep(300); // wait some more to ensure writeLock.acquire() is called - - hbck = doFsck(conf, false); - // still one expired, one not-expired - assertErrors(hbck, new HBaseFsck.ErrorReporter.ERROR_CODE[] { - HBaseFsck.ErrorReporter.ERROR_CODE.EXPIRED_TABLE_LOCK}); - - edge.incrementTime(conf.getLong(TableLockManager.TABLE_LOCK_EXPIRE_TIMEOUT, - TableLockManager.DEFAULT_TABLE_LOCK_EXPIRE_TIMEOUT_MS)); // let table lock expire - - hbck = doFsck(conf, false); - assertErrors(hbck, new HBaseFsck.ErrorReporter.ERROR_CODE[] { - HBaseFsck.ErrorReporter.ERROR_CODE.EXPIRED_TABLE_LOCK, - HBaseFsck.ErrorReporter.ERROR_CODE.EXPIRED_TABLE_LOCK}); // both are expired - - Configuration localConf = new Configuration(conf); - // reaping from ZKInterProcessWriteLock uses znode cTime, - // which is not injectable through EnvironmentEdge - localConf.setLong(TableLockManager.TABLE_LOCK_EXPIRE_TIMEOUT, 1); - - Threads.sleep(10); - hbck = doFsck(localConf, true); // now fix both cases - - hbck = doFsck(localConf, false); - assertNoErrors(hbck); - - // ensure that locks are deleted - writeLock = tableLockManager.writeLock(tableName, "should acquire without blocking"); - writeLock.acquire(); // this should not block. - writeLock.release(); // release for clean state - tableLockManager.tableDeleted(tableName); - } - - @Test(timeout=180000) public void testCheckReplication() throws Exception { // check no errors HBaseFsck hbck = doFsck(conf, false); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java index d1e774e..0c9b036 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java @@ -61,7 +61,6 @@ public class HbckTestingUtil { fsck.setFixVersionFile(fixVersionFile); fsck.setFixReferenceFiles(fixReferenceFiles); fsck.setFixEmptyMetaCells(fixEmptyMetaRegionInfo); - fsck.setFixTableLocks(fixTableLocks); fsck.setFixReplication(fixReplication); if (table != null) { fsck.includeTable(table); -- 2.8.4 (Apple Git-73)