You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by vj...@apache.org on 2024/01/23 02:18:13 UTC
(hbase) branch master updated: HBASE-28271 Infinite waiting on lock acquisition by snapshot can result in unresponsive master (#5603)
This is an automated email from the ASF dual-hosted git repository.
vjasani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 877830e06c8 HBASE-28271 Infinite waiting on lock acquisition by snapshot can result in unresponsive master (#5603)
877830e06c8 is described below
commit 877830e06c875f9ae1e0d84104c6733cf935babd
Author: Viraj Jasani <vj...@apache.org>
AuthorDate: Mon Jan 22 17:18:06 2024 -0900
HBASE-28271 Infinite waiting on lock acquisition by snapshot can result in unresponsive master (#5603)
Signed-off-by: Hui Ruan <hu...@apache.org>
Signed-off-by: Duo Zhang <zh...@apache.org>
---
.../hadoop/hbase/master/locking/LockManager.java | 9 --
.../hbase/master/snapshot/TakeSnapshotHandler.java | 38 ++++-
.../TestSnapshotProcedureWithLockTimeout.java | 159 +++++++++++++++++++++
3 files changed, 190 insertions(+), 16 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java
index 7fab616449c..eae9c7dcc1b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java
@@ -108,15 +108,6 @@ public final class LockManager {
this.description = description;
}
- /**
- * Acquire the lock, waiting indefinitely until the lock is released or the thread is
- * interrupted.
- * @throws InterruptedException If current thread is interrupted while waiting for the lock
- */
- public boolean acquire() throws InterruptedException {
- return tryAcquire(0);
- }
-
/**
* Acquire the lock within a wait time.
* @param timeoutMs The maximum time (in milliseconds) to wait for the lock, 0 to wait
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 58ecaca09ec..f746adf0b89 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
@@ -17,12 +17,16 @@
*/
package org.apache.hadoop.hbase.master.snapshot;
+import static org.apache.hadoop.hbase.HConstants.DEFAULT_HBASE_RPC_TIMEOUT;
+import static org.apache.hadoop.hbase.HConstants.HBASE_RPC_TIMEOUT_KEY;
+
import java.io.IOException;
import java.util.List;
import java.util.concurrent.CancellationException;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.RegionInfo;
@@ -65,6 +69,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos.Snapshot
public abstract class TakeSnapshotHandler extends EventHandler
implements SnapshotSentinel, ForeignExceptionSnare {
private static final Logger LOG = LoggerFactory.getLogger(TakeSnapshotHandler.class);
+ public static final String HBASE_SNAPSHOT_MASTER_LOCK_ACQUIRE_TIMEOUT =
+ "hbase.snapshot.master.lock.acquire.timeout";
private volatile boolean finished;
@@ -85,6 +91,13 @@ public abstract class TakeSnapshotHandler extends EventHandler
protected final TableName snapshotTable;
protected final SnapshotManifest snapshotManifest;
protected final SnapshotManager snapshotManager;
+ /**
+ * Snapshot creation requires table lock. If any region of the table is in transition, table lock
+ * cannot be acquired by LockProcedure and hence snapshot creation could hang for potentially very
+ * long time. This timeout will ensure snapshot creation fails-fast by waiting for only given
+ * timeout.
+ */
+ private final long lockAcquireTimeoutMs;
protected TableDescriptor htd;
@@ -129,6 +142,8 @@ public abstract class TakeSnapshotHandler extends EventHandler
"Taking " + snapshot.getType() + " snapshot on table: " + snapshotTable, false, true);
this.snapshotManifest =
SnapshotManifest.create(conf, rootFs, workingDir, snapshot, monitor, status);
+ this.lockAcquireTimeoutMs = conf.getLong(HBASE_SNAPSHOT_MASTER_LOCK_ACQUIRE_TIMEOUT,
+ conf.getLong(HBASE_RPC_TIMEOUT_KEY, DEFAULT_HBASE_RPC_TIMEOUT));
}
private TableDescriptor loadTableDescriptor() throws IOException {
@@ -147,12 +162,16 @@ public abstract class TakeSnapshotHandler extends EventHandler
public TakeSnapshotHandler prepare() throws Exception {
super.prepare();
// 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
- } catch (Exception e) {
- this.tableLock.release();
- throw e;
+ if (this.tableLock.tryAcquire(this.lockAcquireTimeoutMs)) {
+ try {
+ this.htd = loadTableDescriptor(); // check that .tableinfo is present
+ } catch (Exception e) {
+ this.tableLock.release();
+ throw e;
+ }
+ } else {
+ LOG.error("Master lock could not be acquired in {} ms", lockAcquireTimeoutMs);
+ throw new DoNotRetryIOException("Master lock could not be acquired");
}
return this;
}
@@ -176,7 +195,12 @@ public abstract class TakeSnapshotHandler extends EventHandler
tableLockToRelease = master.getLockManager().createMasterLock(snapshotTable,
LockType.SHARED, this.getClass().getName() + ": take snapshot " + snapshot.getName());
tableLock.release();
- tableLockToRelease.acquire();
+ boolean isTableLockAcquired = tableLockToRelease.tryAcquire(this.lockAcquireTimeoutMs);
+ if (!isTableLockAcquired) {
+ LOG.error("Could not acquire shared lock on table {} in {} ms", snapshotTable,
+ lockAcquireTimeoutMs);
+ throw new IOException("Could not acquire shared lock on table " + snapshotTable);
+ }
}
// If regions move after this meta scan, the region specific snapshot should fail, triggering
// an external exception that gets captured here.
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureWithLockTimeout.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureWithLockTimeout.java
new file mode 100644
index 00000000000..f780d68acf4
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSnapshotProcedureWithLockTimeout.java
@@ -0,0 +1,159 @@
+/*
+ * 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.procedure;
+
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.SnapshotDescription;
+import org.apache.hadoop.hbase.client.SnapshotType;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.master.snapshot.TakeSnapshotHandler;
+import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
+import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.RegionSplitter;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos;
+
+/**
+ * Snapshot creation with master lock timeout test.
+ */
+@Category({ MasterTests.class, MediumTests.class })
+public class TestSnapshotProcedureWithLockTimeout {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(TestSnapshotProcedureWithLockTimeout.class);
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestSnapshotProcedureWithLockTimeout.class);
+
+ private static HBaseTestingUtil TEST_UTIL;
+ private HMaster master;
+ private TableName TABLE_NAME;
+ private byte[] CF;
+ private String SNAPSHOT_NAME;
+
+ @Before
+ public void setup() throws Exception {
+ TEST_UTIL = new HBaseTestingUtil();
+ Configuration config = TEST_UTIL.getConfiguration();
+ config.setInt("hbase.snapshot.remote.verify.threshold", 1);
+ config.setLong(TakeSnapshotHandler.HBASE_SNAPSHOT_MASTER_LOCK_ACQUIRE_TIMEOUT, 1L);
+ TEST_UTIL.startMiniCluster(3);
+ master = TEST_UTIL.getHBaseCluster().getMaster();
+ TABLE_NAME = TableName.valueOf(Bytes.toBytes("TestSnapshotProcedureWithLockTimeout"));
+ CF = Bytes.toBytes("cf");
+ SNAPSHOT_NAME = "SnapshotProcLockTimeout";
+ final byte[][] splitKeys = new RegionSplitter.HexStringSplit().split(10);
+ Table table = TEST_UTIL.createTable(TABLE_NAME, CF, splitKeys);
+ TEST_UTIL.loadTable(table, CF, false);
+ }
+
+ @After
+ public void teardown() throws Exception {
+ if (this.master != null) {
+ ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(master.getMasterProcedureExecutor(),
+ false);
+ }
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @Test
+ public void testTakeZkCoordinatedSnapshot() {
+ for (int i = 0; i < 10; i++) {
+ try {
+ // Verify that snapshot creation is not possible because lock could not be
+ // acquired on time. This can be flaky behavior because even though we provide 1ms
+ // as lock timeout, it could still be fast enough and eventually lead to successful
+ // snapshot creation. If that happens, retry again.
+ testTakeZkCoordinatedSnapshot(i);
+ break;
+ } catch (Exception e) {
+ LOG.error("Error because of faster lock acquisition. retrying....", e);
+ }
+ assertNotEquals("Retries exhausted", 9, i);
+ }
+ }
+
+ private void testTakeZkCoordinatedSnapshot(int i) throws Exception {
+ SnapshotDescription snapshotOnSameTable =
+ new SnapshotDescription(SNAPSHOT_NAME + i, TABLE_NAME, SnapshotType.SKIPFLUSH);
+ SnapshotProtos.SnapshotDescription snapshotOnSameTableProto =
+ ProtobufUtil.createHBaseProtosSnapshotDesc(snapshotOnSameTable);
+ Thread second = new Thread("zk-snapshot") {
+ @Override
+ public void run() {
+ try {
+ master.getSnapshotManager().takeSnapshot(snapshotOnSameTableProto);
+ } catch (IOException e) {
+ LOG.error("zk snapshot failed", e);
+ fail("zk snapshot failed");
+ }
+ }
+ };
+ second.start();
+
+ Thread.sleep(5000);
+ boolean snapshotCreated = false;
+ try {
+ SnapshotTestingUtils.confirmSnapshotValid(TEST_UTIL, snapshotOnSameTableProto, TABLE_NAME,
+ CF);
+ snapshotCreated = true;
+ } catch (AssertionError e) {
+ LOG.error("Assertion error..", e);
+ if (
+ e.getMessage() != null && e.getMessage().contains("target snapshot directory")
+ && e.getMessage().contains("doesn't exist.")
+ ) {
+ LOG.debug("Expected behaviour - snapshot could not be created");
+ } else {
+ throw new IOException(e);
+ }
+ }
+
+ if (snapshotCreated) {
+ throw new IOException("Snapshot created successfully");
+ }
+
+ // ensure all scheduled procedures are successfully completed
+ TEST_UTIL.waitFor(4000, 400,
+ () -> master.getMasterProcedureExecutor().getProcedures().stream()
+ .filter(masterProcedureEnvProcedure -> masterProcedureEnvProcedure.getState()
+ == ProcedureProtos.ProcedureState.SUCCESS)
+ .count() == master.getMasterProcedureExecutor().getProcedures().size());
+ }
+}