[ 
https://issues.apache.org/jira/browse/HDDS-16911?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16911:
------------------------------
    Attachment: TestBugCR2ShutdownOnApplyThread.java

> 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]

Reply via email to