Force recovery test failover with SIGSTOP

[Amaury Chamayou (achamayou)]

Motivation

Fix the recovery-election race shown in the failing Coverage Virtual recovery-test log. A SIGTERM stop notice nominates a successor but leaves the original primary eligible for re-election. In this failure, the nominated backup's committable log was behind, both peers correctly rejected it, and the original primary won again. The test then timed out waiting for a different primary despite successful recovery.

Implementation summary

• Suspend the original primary with SIGSTOP once the final recovery share has committed, using the existing Node.suspend() helper.
• Preserve the separate after-backups-recovered variant, its recovery-state ordering, and the different-primary/higher-view, healthy-survivor, exactly-one-opening, and new-primary-opening assertions.
• Remove the obsolete ignore_first_sigterm setup and SIGTERM-specific step-down diagnostic. Report the verified single opening and elected primary instead.
• Retain SIGKILL and bounded process reaping on both success and failure, including suspended or not-yet-joined recovered nodes.

The immediate variant does not wait for the backups to catch up or for the service to open before suspension. It still exercises interrupted recovery, but now a lagging backup can lose an election without allowing the original primary to win again.

Why suspension removes the race

This illustrates the observed stale-candidate ordering. The winning backup can vary; the invariant is that the suspended original primary cannot campaign or send heartbeats, while the two surviving backups retain a quorum.

sequenceDiagram
    participant T as Recovery test
    participant P as Original primary
    participant B as Lagging backup
    participant U as Up-to-date backup

    T->>P: Submit final recovery share
    P-->>T: Recovery initiated, final share committed

    alt Before: SIGTERM only requests a handover
        T->>P: SIGTERM (ignore_first_sigterm=true)
        P->>B: Nominate successor
        B->>P: Request vote (view 5, log 4.94)
        P-->>B: Reject: local committable log is 4.96
        B->>U: Request vote (view 5, log 4.94)
        U-->>B: Reject: local committable log is 4.96
        P->>U: Request vote (view 6, log 4.96)
        U-->>P: Vote granted
        Note over P: Original primary wins again
        P-->>T: Same primary, higher view
        Note over T: Waiting for a different primary times out
    else After: SIGSTOP forces primary failure
        T->>P: SIGSTOP
        Note over P: Paused until SIGKILL, cannot campaign or send heartbeats
        Note over B,U: Two surviving backups retain a quorum
        B->>U: Request vote with stale committable log
        U-->>B: Reject stale candidate
        U->>B: Request vote with up-to-date committable log
        B-->>U: Vote granted
        U-->>T: Different primary in a higher view
        Note over B,U: Recovery completes, exactly one service opening per ledger
        T->>P: SIGKILL and reap
    end

Loading

Validation

Local build: Clang 21.1.8, Debug, COVERAGE=ON, LONG_TESTS=ON, WORKER_THREADS=1, on a 10-core-affinity Ubuntu host.

• The changed immediate-failover case passed 50 consecutive repetitions. Parsed the actual output to confirm 50 different-from-original primaries and 50 exactly-one-opening checks.
• Ten controlled slow-backup scenarios passed: verified kernel SIGSTOP on one backup before share submission, then on the original primary before resuming that backup. A surviving backup was elected and all three recovered processes were reaped in every run. Either backup may legitimately win after catching up from buffered messages.
• Both recovery-election variants passed an initial coverage-instrumented run.
• The complete coverage-instrumented unit suite passed: 62/62 tests.
• open_service_test, raft_test, and raft_enclave_test passed 10 consecutive repetitions each.
• Injected an exception immediately after primary suspension in both variants: all six recovered processes were killed and reaped, including the suspended primaries.
• All checks in scripts/ci-checks.sh passed without auto-fix, including ASCII, formatting, lint, types, and the fresh CMake CI-bucket inventory check.
• The Mermaid sequence diagram passed Mermaid 11 syntax parsing.
• All 808 collected LLVM raw profiles merged successfully.

The primary repeated test and full unit/check commands were:

(cd build && CR_FILTER=recovery_with_election ./tests.sh -VV --timeout 360 --repeat until-fail:50 -R '^recovery_test$' --no-tests=error)
ctest --test-dir build -L '^unit$' --output-on-failure --no-tests=error --timeout 360 -j2
scripts/ci-checks.sh

Stress limitations, not hidden or treated as passes:

• The combined 50-repeat command stopped on iteration 11 in the unchanged after-backups-recovered scenario. The original primary committed its opening before the test could suspend it, missing that scenario's 1-second signature window. The immediate failover case passed all 11 iterations. The unmodified baseline at 8a152f602 reproduces the same late-opening assertion with a controlled 1.25-second scheduling delay before suspension.
• A three-repeat full recovery-suite command stopped on its first run: both election variants passed, but the unchanged recovery_corrupt_ledger case hit Historical range for idx 9 not available after 3s. The same corrupt-ledger case from the unmodified baseline at 8a152f602 passed in isolation.

These separate timing-sensitive cases are not changed by this PR. Their timeouts and assertions remain intact; this is not a full recovery-suite stress pass.

Safety and compatibility

Test-harness-only change: no production code, API, ledger format, consensus voting rule, or mixed-version behavior changes. The final recovery-share transaction is already committed before the original primary is suspended. The survivors still have to elect a different primary in a higher view, complete recovery, remain healthy, and commit exactly one service opening per ledger. The after-backups-recovered variant still requires that opening to belong to the new primary's view.

The test now waits for the ordinary election timeout rather than requesting an immediate nomination. No timeout or assertion was relaxed.

[Copilot]

Copilot review overview

🟢 Approval recommended

The focused test-harness change resolves the observed race while preserving recovery invariants and cleanup.

Review effort: Balanced
Findings: None

What changed in this PR

Updates the recovery election test to eliminate re-election of the original primary.

Changes:

• Uses SIGSTOP to force failover.
• Removes obsolete SIGTERM diagnostics/configuration.
• Preserves recovery assertions and bounded cleanup.

Custom instructions used

• .github/copilot-instructions.md
• .github/instructions/reviewing.instructions.md
• .github/skills/testing/SKILL.md

File	Description
tests/​recovery.py	Forces deterministic recovery failover and updates diagnostics and cleanup comments.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.