[
https://issues.apache.org/jira/browse/RATIS-2656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Tsz-wo Sze updated RATIS-2656:
------------------------------
Component/s: StateMachine
(was: server)
> BaseStateMachine.pause() does not pause the lifecycle, so a streamed snapshot
> install closes the division
> ---------------------------------------------------------------------------------------------------------
>
> Key: RATIS-2656
> URL: https://issues.apache.org/jira/browse/RATIS-2656
> Project: Ratis
> Issue Type: Bug
> Components: StateMachine
> Affects Versions: 3.2.2
> Reporter: Anony Mouse
> Priority: Major
>
> h3. Summary
> A state machine that extends {{BaseStateMachine}} without overriding
> {{pause()}} has its division closed by every streamed snapshot install it
> receives. The default {{pause()}} is an empty method, so the lifecycle never
> reaches {{PAUSED}}; the install path then calls
> {{StateMachineUpdater.reload()}}, whose precondition requires {{PAUSED}}; the
> resulting {{IllegalStateException}} reaches the updater's catch-all, which
> closes the division.
> Observed at 3.2.2 and again against the 3.3.0 RC2 artifacts.
> h3. Chain
> # {{BaseStateMachine.pause()}} - empty body; nothing transitions the
> lifecycle.
> # Follower receives a streamed install; the server pauses the state machine
> and calls {{StateMachineUpdater.reload()}}.
> # {{reload()}} asserts the state machine is {{PAUSED}} (frame observed:
> {{StateMachineUpdater.reload(StateMachineUpdater.java:230)}} at the 3.2.2
> tag) -> {{IllegalStateException}}.
> # The updater's catch-all logs {{StateMachineUpdater caught a Throwable}} and
> closes the division.
> h3. Observed timeline (one follower, one install)
> {noformat}
> 06:49:26.353 SnapshotManager - Installed snapshot, renaming temporary dir
> ... to .../sm
> 06:49:26.356 StateMachineUpdater caught a Throwable.
> java.lang.IllegalStateException
> at StateMachineUpdater.reload(StateMachineUpdater.java:230)
> 06:49:26.357 RaftServer$Division: shutdown
> {noformat}
> Four milliseconds from a successful install to a dead division. The install
> itself is durable on disk; only the division dies. A later crash-restart of
> that node recovers normally from the installed snapshot, which is what makes
> this easy to miss: the cluster keeps working, and the node only dies again
> the next time it is sent an install.
> h3. Impact
> Any integrator whose state machine inherits the default {{pause()}} - which
> is the natural thing to do, since it is a concrete no-op rather than an
> abstract method - loses a follower's division on each streamed install, and
> gains it back only on process restart. Nothing in the signature or javadoc of
> {{pause()}} indicates that an override is required for correctness.
> A secondary effect is filed separately: while the division is closed, the
> leader continues to re-initiate the install with no backoff.
> h3. Reproduction
> An in-JVM probe with a deliberately naive {{BaseStateMachine}} subclass (no
> {{pause()}} override) reproduces it directly, and a fixed subclass that
> honors the lifecycle survives the same path:
> https://github.com/hooji/ratis-jepsen - see
> {{harness/probe/ratis_jepsen/probe/lifecycle.clj}}, and
> {{jobs/08-membership-churn/}} plus {{reviews/08-membership-churn/}} for the
> original discovery, exact frames, and preserved run stores.
> The probe was run against 3.2.2 (reproduces) and the 3.3.0 RC2 artifacts
> (reproduces identically); it is not sensitive to the surrounding harness.
> h3. Possible directions
> Any one of these would close it; the choice is yours:
> * Give {{BaseStateMachine.pause()}} a default implementation that actually
> transitions the lifecycle to {{PAUSED}} (and {{reinitialize()}} back), so the
> shipped base class is correct by default.
> * Have {{StateMachineUpdater.reload()}} tolerate a non-{{PAUSED}} state
> machine rather than treating it as a precondition violation.
> * If the override is intended to be the integrator's responsibility, make
> that explicit - javadoc on {{pause()}}/{{reinitialize()}} stating that a
> snapshot install will close the division otherwise, and ideally a louder
> failure than an assertion swallowed by a catch-all.
> Happy to test a patch - the probe above gives a fast yes/no.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
