Umeshkumar9414 commented on code in PR #8357:
URL: https://github.com/apache/hbase/pull/8357#discussion_r3434416315
##########
hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/ProcedureTestingUtility.java:
##########
@@ -125,14 +127,50 @@ public static <TEnv> void restart(ProcedureExecutor<TEnv>
procExecutor,
stopAction.call();
}
procExecutor.join();
- procExecutor.getScheduler().clear();
+ // HBASE-29555: Callbacks that wake up a procedure after an async
operation (such as updating
+ // meta) run on the asyncTaskExecutor, not on a PEWorker thread.
ProcedureExecutor.stop() shuts
+ // the asyncTaskExecutor down, but ProcedureExecutor.join() only waits a
bounded amount of time
+ // for it to terminate before giving up. If such a wake up task is still
pending, it can call
+ // scheduler.addFront() after we clear the scheduler below and after the
executor has been
+ // restarted, leaving the scheduler queue non-empty and tripping the
Preconditions check in
+ // ProcedureExecutor.load(). Since we reuse the same executor (and
scheduler) instance across
+ // this simulated restart, wait for the already shutdown asyncTaskExecutor
to fully terminate so
+ // any pending wake up task has finished before we clear the scheduler.
+ awaitAsyncTaskExecutorTermination(procExecutor);
// nothing running...
// re-start
LOG.info("RESTART - Start");
- procStore.start(storeThreads);
- procExecutor.init(execThreads, failOnCorrupted);
+ // HBASE-29555: External events (for example a region server reporting a
region state transition
+ // over RPC) can wake a procedure and push it back into the scheduler in
the small window
+ // between clearing the scheduler and ProcedureExecutor.load() checking
that it is empty. Those
+ // producers run on threads we do not own here (RPC handlers etc.), so we
cannot reliably stop
+ // them from this thread. Instead, reload in a retry loop: if load() fails
only because the
+ // scheduler is not empty, stop/drain/clear again and retry.
ProcedureExecutor.stop() is
+ // explicitly safe to call after a failed init(), so this is a clean redo
of the reload.
+ final int maxRestartAttempts = 20;
+ for (int attempt = 1;; attempt++) {
Review Comment:
Emptying scheduler is not out goal here. Check cause of this and try to
resolve that. If we are missing something while simulating the Master restart
lets add that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
