[
https://issues.apache.org/jira/browse/HDDS-16911?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16911:
------------------------------
Attachment: CR-2-throw-failed-rotation-commit-to-state-machine.patch
> SCM hangs half stopped instead of exiting when a step of the root CA rotation
> commit fails
> ------------------------------------------------------------------------------------------
>
> Key: HDDS-16911
> URL: https://issues.apache.org/jira/browse/HDDS-16911
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> CR-2-throw-failed-rotation-commit-to-state-machine.patch,
> TestBugCR2ShutdownOnApplyThread.java
>
>
> h3. Mechanism
> {{RootCARotationHandlerImpl.rotationCommit}} is a replicated request, so it
> runs in {{SCMStateMachine.applyTransaction}} on the Ratis
> {{StateMachineUpdater}} thread. When one of its three steps fails (either
> directory move, or the write of the new certificate ID to the VERSION file)
> it calls {{StorageContainerManager.shutDown}}, which is {{stop()}} followed
> by {{ExitUtils.terminate}}. {{stop()}} reaches {{RaftServerProxy.close}}
> through {{SCMHAManagerImpl.stop}} and {{SCMRatisServerImpl.stop}}. The close
> hands the close of the Raft division to its executor and then waits for that
> executor to terminate ({{ConcurrentUtils.shutdownAndWait}}, with a timeout of
> one day). The division close ({{RaftServerImpl.close}},
> {{ServerState.close}}, {{StateMachineUpdater.stopAndJoin}}) joins the
> {{StateMachineUpdater}} thread, which is the thread waiting for the executor.
> Neither can proceed, so the rest of {{stop()}} and the
> {{ExitUtils.terminate}} call are not reached.
> Every other fatal failure in {{applyTransaction}} (any exception that is not
> an {{SCMException}}, and an {{SCMException}} with result {{INTERNAL_ERROR}}
> or {{IO_EXCEPTION}}) reaches the catch block of {{SCMStateMachine}}, which
> calls {{ExitUtils.terminate}} with status 1 and does not stop the SCM on that
> thread. An {{SCMException}} with any other result is a logical rejection, it
> is returned to the caller and the SCM keeps running. {{rotationCommit}}
> declares {{IOException}} but does not use that path. Its three calls are the
> only callers of {{shutDown}} under {{applyTransaction}}. The calls in
> {{RootCARotationManager}} run on the executor of the manager and are not
> affected. HDDS-10643 fixed a related wait for the normal stop, where
> {{SCMStateMachine.close}} called {{shutDown}} on this thread.
> h3. Trigger
> # Secure SCM HA with {{hdds.x509.ca.rotation.enabled=true}} (default false).
> # A root CA rotation reaches {{rotationCommit}} and one of its steps fails on
> an SCM, for example because {{scm/sub-ca-previous}} is left over and not
> empty, because {{scm/sub-ca-next}} is missing, or because of an I/O error on
> the metadata volume.
> Run: the first move fails because the current sub CA directory is missing.
> The other two steps make the same call (code reading, exercised at unit level
> by the test in the patch).
> The same three calls are in the release tags ozone-1.4.0, 1.4.1, 2.0.0,
> 2.1.0, 2.1.1, 2.1.2, 2.2.0 and 2.2.1 (read with git grep, not run). Those
> releases use Ratis 3.0.1 to 3.2.1, and the close path was read only in Ratis
> 3.3.1.
> h3. Impact
> * The SCM process does not exit. Run in a child JVM with {{System.exit}}
> enabled and the shutdown hook of {{StorageContainerManagerStarter}}: the
> process was still alive 200s after the failed commit and was then ended by
> the harness. By code reading it stays until the one day timeout in Ratis
> expires, after which the stop continues and the process exits with status 0.
> * By then the datanode, block and client RPC servers, the HTTP server and the
> RPC service of the Raft server are stopped, and the Raft division has given
> up its role. The SCM serves no RPC or HTTP requests and is an unreachable
> peer for the other SCMs, while it is still a member of the Raft group. It
> still has its RocksDB open (run) and holds the Raft storage directory (code
> reading).
> * A supervisor that restarts the SCM when the process exits does nothing, and
> a check that only looks at the process sees a running SCM. The log has no
> "Terminating with exit status" line. Among the last lines of the applying
> thread are "Stopping SCM HA services." and the Ratis "close".
> * SIGTERM ends the process (run, 0.5s), because the shutdown hook finds the
> SCM already stopped.
> * This only shows after {{rotationCommit}} has already failed on that SCM, so
> it causes no outage by itself. It turns a failed SCM that should exit into
> one that has to be found and killed by hand. If the step fails on every SCM,
> all of them hang.
> h3. Reproduction
> PASS, unmodified source. [^TestBugCR2ShutdownOnApplyThread.java]
> ({{hadoop-hdds/server-scm}}, about 20s) starts one real SCM with its Ratis
> server, registers the handler through its builder and calls
> {{rotationCommit}} through Ratis. The SCM is not secure and has no
> certificate client, so the handler sees the SCM through a delegate that
> supplies one. Every other call, including {{shutDown}}, goes to the real SCM.
> The sub CA directory does not exist, so the first move fails. The test then
> finds the {{StateMachineUpdater}} thread in
> {{ConcurrentUtils.shutdownAndWait}} under {{RaftServerProxy.close}},
> {{StorageContainerManager.shutDown}} and {{rotationCommit}}, and a thread of
> the Raft server in {{Thread.join}} under {{StateMachineUpdater.stopAndJoin}}.
> The SCM reports that it is stopped, {{ExitUtils.terminate}} was not called
> and the SCM RocksDB is still open. A passing test means the defect is
> present. A secure SCM was not run for this report.
> h3. Patch
> [^CR-2-throw-failed-rotation-commit-to-state-machine.patch], against
> b9b33851b20fa11baf4b07982de9afb8ea44f85a. It also applies to master at
> 5108acff480. The patch contains only this change and is written against the
> first commit alone, so other fixes proposed for {{rotationCommit}} will need
> a manual merge with it.
> The three failure branches of {{rotationCommit}} throw an {{IOException}}
> with the same message instead of calling {{scm.shutDown}}.
> {{SCMStateMachine.applyTransaction}} then terminates the SCM with exit status
> 1, as it does for any other fatal apply failure. The status is 1 instead of
> the 0 that {{shutDown}} would have used, which matches the other fatal apply
> failures. Nothing changes for a commit that succeeds, for the replicated
> requests or for what is persisted, and {{StorageContainerManager.shutDown}}
> and its other callers are untouched.
> Run in the same child JVM setup with the patch: "Terminating with exit status
> 1" is logged at once and the process exits with status 1 about 60s later. The
> 60s are the timeout of the shutdown hook
> ({{ozone.service.shutdown.timeout}}), whose {{stop()}} waits in the same
> Ratis close while the applying thread is inside {{System.exit}}. That delay
> exists today for every fatal apply failure and is not changed by the patch.
> Covered by {{testFailedCommitStepIsThrownToStateMachine}} in the existing
> {{TestRootCARotationManager}}, which fails without the change ("Expecting
> code to raise a throwable"). With it {{TestRootCARotationManager}},
> {{TestSCMStateMachine}}, {{TestSCMHAManagerImpl}},
> {{TestReplicationAnnotation}}, {{TestScmInvokerCodeGenerator}} and
> {{TestScmCodecFactoryReplicateCoverage}} (35 tests) pass and checkstyle is
> clean. With the patch the reproduction fails at its first assertion, because
> the SCM reaches {{ExitUtils.terminate}}.
> The patch makes the SCM exit, it does not make the commit succeed. What the
> next start does depends on the step that failed (code reading, a restart was
> not run). After a failed first move nothing on disk has changed and the log
> entry is not recorded as applied, so it is applied again and fails again
> until the cause is repaired. After a failed second move {{scm/sub-ca}} has
> already been moved to {{scm/sub-ca-previous}}, and a secure SCM does not
> start until the directory is put back, because {{DefaultCAServer.init}}
> rejects a subordinate CA that has neither keys nor certificates. After a
> failed write of the certificate ID the VERSION file still names the old
> certificate while {{scm/sub-ca}} holds the new one, and that start was not
> followed. The same holds without the patch once the hung process has been
> killed.
> Found by code review of the SCM root CA rotation paths, as part of the TLA+
> verification effort under HDDS-15926, on commit
> b9b33851b20fa11baf4b07982de9afb8ea44f85a. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
