smengcl commented on code in PR #10999:
URL: https://github.com/apache/ozone/pull/10999#discussion_r4212560775
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java:
##########
@@ -414,18 +443,77 @@ public void notifyTermIndexUpdated(long term, long index)
{
* container reports are processed against the up-to-date container/pipeline
* state rather than a stale, mid-replay snapshot.
*/
- private void tryStartDNServerAndRefreshSafeMode() {
- if (isStateMachineReady.get()) {
+ private synchronized void tryStartDNServerAndRefreshSafeMode() {
+ if (scm.isStopped() || isStateMachineReady.get()) {
return;
}
if (scm.getScmContext().isLeader() || isFollowerCaughtUp()) {
if (isStateMachineReady.compareAndSet(false, true)) {
+ // No further retry is needed. shutdown() lets the current retry, if
+ // this method was called by it, finish normally without cancellation.
+ dnServerStartRetryExecutor.shutdown();
scm.getDatanodeProtocolServer().start();
scm.getScmSafeModeManager().refreshAndValidate();
}
}
}
+ /**
+ * Periodic fallback for the deferred datanode-server start on a follower.
The
+ * Ratis callbacks ({@code applyTransaction}, {@code notifyTermIndexUpdated},
+ * {@code notifyLeaderChanged}) only fire on new activity, so on an idle
+ * cluster a restarted follower can miss the moment the leader's committed
+ * index becomes observable (it is not yet known when {@code
notifyLeaderChanged}
+ * fires) and never start its datanode server, leaving it stuck in safe mode.
+ * This re-checks until the server starts. Starting stays
+ * gated by the same {@link #tryStartDNServerAndRefreshSafeMode()}
predicate, so
+ * it cannot start the server before genuine catch-up.
+ */
+ private void retryStartDNServerUntilReady() {
+ try {
+ if (!scm.isStopped() && !isStateMachineReady.get()) {
+ tryStartDNServerAndRefreshSafeMode();
+ }
+ } catch (Exception e) {
+ // Do not let a failed readiness check terminate the fallback. The
+ // finally block only schedules another attempt if startup is pending.
+ LOG.warn("Deferred datanode-server start retry failed", e);
+ } finally {
+ synchronized (this) {
+ dnServerStartRetryFuture = null;
+ scheduleDNServerStartRetry();
+ }
+ }
+ }
+
+ private synchronized void scheduleDNServerStartRetry() {
+ if (scm.isStopped() || isStateMachineReady.get() ||
+ dnServerStartRetryExecutor.isShutdown() || dnServerStartRetryFuture !=
null) {
+ return;
+ }
+ dnServerStartRetryFuture = dnServerStartRetryExecutor.schedule(
+ this::retryStartDNServerUntilReady,
Review Comment:
Addressed in a3eeb68d8ba.
The readiness check compares against a captured commit index, not the
leader's continuously advancing index, so a busy leader does not create a
moving catch-up target. Added
`testRetryUsesCapturedCommitIndexWhenLeaderAdvances` to verify this.
A genuinely stalled follower can still wait indefinitely. Continuing retries
lets it recover when progress resumes; a retry cutoff would leave the datanode
server permanently deferred. Added WARN logging after 30 seconds and then every
five minutes, including attempt count, elapsed time, `lastAppliedIndex` and
`leaderCommitIndexOnStart`. Tests cover warning throttling, unavailable commit
information, and stopping after readiness or shutdown.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java:
##########
@@ -111,12 +133,18 @@ public SCMStateMachine(final StorageContainerManager scm,
.setNameFormat(scm.threadNamePrefix() + "SCMInstallSnapshot-%d")
.build()
);
+ this.dnServerStartRetryExecutor = HadoopExecutors.newScheduledThreadPool(1,
+ new ThreadFactoryBuilder()
+ .setNameFormat(scm.threadNamePrefix() + "SCMDNServerStartRetry-%d")
+ .setDaemon(true)
+ .build());
isInitialized = true;
}
public SCMStateMachine() {
Review Comment:
Addressed in a3eeb68d8ba.
Yes, intentionally: the no-argument constructor is used only by
`SCMRatisServerImpl.initialize()` to bootstrap Ratis storage, without an SCM
instance or background executors. Added a comment explaining this and made
`stopDNServerStartRetry()` safely skip bootstrap instances, consistent with the
existing callback and `close()` guards.
Added `testBootstrapStateMachineSkipsSCMCallbacksAndRetryStop`, covering
bootstrap callbacks, repeated stop calls and `close()`, without allocating
retry threads.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
