devmadhuu commented on code in PR #10999:
URL: https://github.com/apache/ozone/pull/10999#discussion_r4135907603
##########
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:
So will this be unbounded retry ? e.g. a follower whose apply rate is below
the leader's append rate, so `isFollowerCaughtUp()` never becomes true — this
loops forever ? Any logs to be added for debuggability purpose ? like attempt
count, elapsed time, logging at WARN on a backoff schedule (e.g. after 30s,
then every 5 min) with the current lastAppliedIndex vs leaderCommitIndexOnStart
so the gap is diagnosable.
##########
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:
This constructor will leave `dnServerStartRetryExecutor` as null ?
--
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]
