You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2022/07/07 00:39:02 UTC
[hbase] branch branch-2 updated: Revert "HBASE-27157 Potential race condition in WorkerAssigner (#4577)"
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 1c90b4344d0 Revert "HBASE-27157 Potential race condition in WorkerAssigner (#4577)"
1c90b4344d0 is described below
commit 1c90b4344d00faca3a73c997a76134f70ae12fd7
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Thu Jul 7 08:38:43 2022 +0800
Revert "HBASE-27157 Potential race condition in WorkerAssigner (#4577)"
This reverts commit d79d812dd01e2b5e6b0bc3ba463fcba7280be394.
---
.../hadoop/hbase/master/SplitWALManager.java | 15 ++++++++---
.../apache/hadoop/hbase/master/WorkerAssigner.java | 31 ++++++++++------------
.../master/procedure/SnapshotVerifyProcedure.java | 3 ++-
.../hbase/master/procedure/SplitWALProcedure.java | 2 +-
.../hbase/master/snapshot/SnapshotManager.java | 15 ++++++++---
.../hadoop/hbase/master/TestSplitWALManager.java | 5 ++--
6 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitWALManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitWALManager.java
index 7497877e67c..18dfc7d493b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitWALManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitWALManager.java
@@ -35,6 +35,7 @@ import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.master.procedure.MasterProcedureScheduler;
import org.apache.hadoop.hbase.master.procedure.SplitWALProcedure;
import org.apache.hadoop.hbase.procedure2.Procedure;
import org.apache.hadoop.hbase.procedure2.ProcedureEvent;
@@ -152,19 +153,25 @@ public class SplitWALManager {
*/
public ServerName acquireSplitWALWorker(Procedure<?> procedure)
throws ProcedureSuspendedException {
- ServerName worker = splitWorkerAssigner.acquire(procedure);
- LOG.debug("Acquired split WAL worker={}", worker);
- return worker;
+ Optional<ServerName> worker = splitWorkerAssigner.acquire();
+ if (worker.isPresent()) {
+ LOG.debug("Acquired split WAL worker={}", worker.get());
+ return worker.get();
+ }
+ splitWorkerAssigner.suspend(procedure);
+ throw new ProcedureSuspendedException();
}
/**
* After the worker finished the split WAL task, it will release the worker, and wake up all the
* suspend procedures in the ProcedureEvent
* @param worker worker which is about to release
+ * @param scheduler scheduler which is to wake up the procedure event
*/
- public void releaseSplitWALWorker(ServerName worker) {
+ public void releaseSplitWALWorker(ServerName worker, MasterProcedureScheduler scheduler) {
LOG.debug("Release split WAL worker={}", worker);
splitWorkerAssigner.release(worker);
+ splitWorkerAssigner.wake(scheduler);
}
/**
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/WorkerAssigner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/WorkerAssigner.java
index b1f2558045d..b6df41acee2 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/WorkerAssigner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/WorkerAssigner.java
@@ -23,9 +23,9 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.master.procedure.MasterProcedureScheduler;
import org.apache.hadoop.hbase.procedure2.Procedure;
import org.apache.hadoop.hbase.procedure2.ProcedureEvent;
-import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException;
import org.apache.yetus.audience.InterfaceAudience;
/**
@@ -51,7 +51,7 @@ public class WorkerAssigner implements ServerListener {
}
}
- public synchronized ServerName acquire(Procedure<?> proc) throws ProcedureSuspendedException {
+ public synchronized Optional<ServerName> acquire() {
List<ServerName> serverList = master.getServerManager().getOnlineServersList();
Collections.shuffle(serverList);
Optional<ServerName> worker = serverList.stream()
@@ -60,30 +60,27 @@ public class WorkerAssigner implements ServerListener {
.findAny();
worker.ifPresent(name -> currentWorkers.compute(name, (serverName,
availableWorker) -> availableWorker == null ? maxTasks - 1 : availableWorker - 1));
- if (worker.isPresent()) {
- ServerName sn = worker.get();
- currentWorkers.compute(sn, (serverName,
- availableWorker) -> availableWorker == null ? maxTasks - 1 : availableWorker - 1);
- return sn;
- } else {
- event.suspend();
- event.suspendIfNotReady(proc);
- throw new ProcedureSuspendedException();
- }
+ return worker;
}
public synchronized void release(ServerName serverName) {
currentWorkers.compute(serverName, (k, v) -> v == null ? null : v + 1);
+ }
+
+ public void suspend(Procedure<?> proc) {
+ event.suspend();
+ event.suspendIfNotReady(proc);
+ }
+
+ public void wake(MasterProcedureScheduler scheduler) {
if (!event.isReady()) {
- event.wake(master.getMasterProcedureExecutor().getEnvironment().getProcedureScheduler());
+ event.wake(scheduler);
}
}
@Override
- public synchronized void serverAdded(ServerName worker) {
- if (!event.isReady()) {
- event.wake(master.getMasterProcedureExecutor().getEnvironment().getProcedureScheduler());
- }
+ public void serverAdded(ServerName worker) {
+ this.wake(master.getMasterProcedureExecutor().getEnvironment().getProcedureScheduler());
}
public synchronized void addUsedWorker(ServerName worker) {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotVerifyProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotVerifyProcedure.java
index 8ec261d768c..651822ff5b2 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotVerifyProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SnapshotVerifyProcedure.java
@@ -109,7 +109,8 @@ public class SnapshotVerifyProcedure extends ServerRemoteProcedure
setFailure("verify-snapshot", e);
} finally {
// release the worker
- env.getMasterServices().getSnapshotManager().releaseSnapshotVerifyWorker(this, targetServer);
+ env.getMasterServices().getSnapshotManager().releaseSnapshotVerifyWorker(this, targetServer,
+ env.getProcedureScheduler());
}
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitWALProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitWALProcedure.java
index 98c2c0ec693..699834f9c1d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitWALProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitWALProcedure.java
@@ -90,7 +90,7 @@ public class SplitWALProcedure
skipPersistence();
throw new ProcedureSuspendedException();
}
- splitWALManager.releaseSplitWALWorker(worker);
+ splitWALManager.releaseSplitWALWorker(worker, env.getProcedureScheduler());
if (!finished) {
LOG.warn("Failed to split wal {} by server {}, retry...", walPath, worker);
setNextState(MasterProcedureProtos.SplitWALState.ACQUIRE_SPLIT_WAL_WORKER);
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
index c881142b5e8..259987ca66d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
@@ -63,6 +63,7 @@ import org.apache.hadoop.hbase.master.cleaner.HFileCleaner;
import org.apache.hadoop.hbase.master.cleaner.HFileLinkCleaner;
import org.apache.hadoop.hbase.master.procedure.CloneSnapshotProcedure;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
+import org.apache.hadoop.hbase.master.procedure.MasterProcedureScheduler;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureUtil;
import org.apache.hadoop.hbase.master.procedure.RestoreSnapshotProcedure;
import org.apache.hadoop.hbase.master.procedure.SnapshotProcedure;
@@ -1418,14 +1419,20 @@ public class SnapshotManager extends MasterProcedureManager implements Stoppable
public ServerName acquireSnapshotVerifyWorker(SnapshotVerifyProcedure procedure)
throws ProcedureSuspendedException {
- ServerName worker = verifyWorkerAssigner.acquire(procedure);
- LOG.debug("{} Acquired verify snapshot worker={}", procedure, worker);
- return worker;
+ Optional<ServerName> worker = verifyWorkerAssigner.acquire();
+ if (worker.isPresent()) {
+ LOG.debug("{} Acquired verify snapshot worker={}", procedure, worker.get());
+ return worker.get();
+ }
+ verifyWorkerAssigner.suspend(procedure);
+ throw new ProcedureSuspendedException();
}
- public void releaseSnapshotVerifyWorker(SnapshotVerifyProcedure procedure, ServerName worker) {
+ public void releaseSnapshotVerifyWorker(SnapshotVerifyProcedure procedure, ServerName worker,
+ MasterProcedureScheduler scheduler) {
LOG.debug("{} Release verify snapshot worker={}", procedure, worker);
verifyWorkerAssigner.release(worker);
+ verifyWorkerAssigner.wake(scheduler);
}
private void restoreWorkers() {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestSplitWALManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestSplitWALManager.java
index 309de7b6e81..8818609c731 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestSplitWALManager.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestSplitWALManager.java
@@ -115,7 +115,8 @@ public class TestSplitWALManager {
Assert.assertNotNull(e);
Assert.assertTrue(e instanceof ProcedureSuspendedException);
- splitWALManager.releaseSplitWALWorker(server);
+ splitWALManager.releaseSplitWALWorker(server, TEST_UTIL.getHBaseCluster().getMaster()
+ .getMasterProcedureExecutor().getEnvironment().getProcedureScheduler());
Assert.assertNotNull(splitWALManager.acquireSplitWALWorker(testProcedures.get(3)));
}
@@ -347,7 +348,7 @@ public class TestSplitWALManager {
setNextState(MasterProcedureProtos.SplitWALState.RELEASE_SPLIT_WORKER);
return Flow.HAS_MORE_STATE;
case RELEASE_SPLIT_WORKER:
- splitWALManager.releaseSplitWALWorker(worker);
+ splitWALManager.releaseSplitWALWorker(worker, env.getProcedureScheduler());
return Flow.NO_MORE_STATE;
default:
throw new UnsupportedOperationException("unhandled state=" + state);