docs: add ordered durability fault testing todo

[GatewayJ]

Summary

• make docs/DURABILITY_FAULT_TESTING_TODO.md the single source of truth for future RustFS durability fault-testing work
• fold the remaining docs/todo.md architecture guardrails into the ordered TODO
• delete the retired docs/todo.md roadmap so future work has one docs entry point
• keep the ordered TODO list with DONE, PARTIAL, TODO, BLOCKED, and DEFERRED status markers

Validation

• cargo fmt --all -- --check
• git diff --check origin/main..HEAD

Notes

• Future work should follow the TODO order unless a new blocker changes the risk ranking.
• The TODO keeps the PR docs: add ordered durability fault testing todo #15 review feedback in the work queue, including calibration, drop_writes, ack-triggering, per-version quorum thresholds, fixed volume targeting, final checker classification precision, heal false-green controls, and deferred physical power work.
• The retired roadmap guardrails preserve the Bash/Rust boundary, suite YAML contract boundary, fault backend port boundary, RustFS reliability coverage boundary, and read-only console/reporting boundary.

[overtrue]

Review: Durability Fault Testing Design

Thanks for writing this down — the framework is genuinely good: Jepsen-style separation, fail-closed readiness gates, a small/stable artifact contract, and disciplined restraint on composition abstraction. I ran a multi-round adversarial review over the design (7 independent reviewers → per-finding refutation → red-team of the synthesis, with the load-bearing claims re-verified against both this repo and rustfs/rustfs source). Full findings + roadmap are tracked in rustfs/backlog#1100.

Recommendation: request changes — but the blocking reason is not the doc-vs-code drift (that's fast-follow); it's a fault-model gap that makes the flagship Phase 4 scenarios green-pass the exact bug class this design exists to catch (#4221 ack-then-lose). Details below, most severe first.

Blocker 1 — The durability oracle is masked three ways; several P0 rows can't detect ack-then-lose data loss

The target failure chain (ACK → not persisted → lost after recovery) is defeated before the checker ever runs, on any build, independent of the durability switch:

1. The injection step destroys the signal. The dm backend loads its fault table via dmsetup suspend without --nolockfs (src/fault/backends/host.rs:430-436 only knows --noflush). Default suspend does lockfs → freeze_bdev → sync_filesystem, which flushes every ack'd-but-unsynced dirty page to the still-healthy device at inject time. (xfstests' common/dmflakey uses --nolockfs precisely to avoid this.)
2. Page cache masks the rest. Even with (1) fixed, ack'd-unsynced writes live in the host kernel page cache; the recovery path requires the pod to stay Ready with zero restarts (runner.rs), and page cache survives a pod restart anyway. The design contains zero cache-eviction / remount / crash steps — grep 'page cache|drop_caches|fsync|writeback|remount|drop_writes|nolockfs' over the doc returns nothing. Note the default dm fault table is EIO, and drop_writes appears 0 times in the repo — so dm-flakey-versioned-hot is today an EIO error-path test, not a durability test.
3. EC redundancy masks single-volume loss. The reference topology (4 nodes × 4 disks, backlog#834) resolves to a single 16-disk set, EC 12+4 by default (possible_set_counts affinity → 16; default_parity_count(16)=4). A single node losing power drops exactly 4 shards = P; the remaining 12 satisfy both read and write quorum — no signal, buggy and correct builds both PASS. single-node-power-cycle-after-ack, delete-marker-hard-poweroff, and multipart-complete-hard-poweroff all inherit this.

The design never states the key derivation: the minimum fault size that surfaces no-fsync at the S3 layer is "> P volumes in the same erasure set simultaneously lose their unpersisted writes." This should be a gating step in Core Semantics that constrains the Phase 4 roster — not an afterthought.

Correction the red-team surfaced (this one is subtle and important): the "> P volumes" rule is wrong for delete markers and size-0 objects. list_object_parities (rustfs/rustfs crates/ecstore/src/set_disk/metadata.rs:253-254) forces parity = total_shards / 2 for deleted || size == 0. On EC 12+4 that's parity 8 → a delete marker's read quorum tolerates losing 8 volumes (2 whole nodes); you need ≥9 lost for committed_object_unavailable and ≥12 (≈3 nodes) before a delete marker could be rolled back. So delete-marker-hard-poweroff bound to ≥P+1=5 volumes is still a green false-pass — the real threshold is ≈ n/2 + 1 (≈3 nodes), off by an entire node from the naive P+1. The Phase 4 roster needs a per-version-type quorum table, not a single P+1 rule.

Blocker 2 — There is no detector-calibration gate, so the false-greens above are unfalsifiable

Nothing in the design proves a scenario can fail when it should. Since rustfs/rustfs now ships RUSTFS_DURABILITY_MODE=strict|relaxed|none (default strict; crates/ecstore/src/disk/local.rs:257 — note this corrects the "sync default off" premise), calibration is nearly free as a 3-tier ladder that also validates Phase 2's classification granularity:

• strict → must PASS (false-positive control).
• relaxed (data shards fdatasync'd; xl.meta/rename dir entries left in page cache — local.rs:3534-3549) → power-class scenarios must FAIL with a metadata-loss-family classification (committed_version_missing / delete_marker_missing / committed_object_unavailable). This directly checks whether Phase 2's "committed object vs committed version vs delete marker" split is actually distinguishable. (Confirmed the physics: relaxed genuinely skips the xl.meta tmp-fdatasync and the post-rename dir fsync, and ext4 data=ordered gives an atomic rollback = clean version loss, not torn corruption — so it lands in the metadata-loss family, not data_corruption.)
• none / legacy-off / the pinned beta.8 image (predates all fsync fixes) → must FAIL, widest loss surface.

Make a passing calibration pair (vulnerable→FAIL, strict→PASS) a hard Phase 4 acceptance gate and a catalog metadata field (detects=[commit-metadata-loss, data-shard-loss]). A scenario that can't produce its calibration evidence may exist only as a diagnostic, never as a green gate.

Red-team caveat on the calibration mechanism (please fold into the doc):

• The crash sequence in the doc is physically wrong. suspend --noflush freezes the device → umount/sync block; drop_caches evicts only clean pages (never the dirty ack'd-unsynced ones); sync -f would persist them → false PASS. The correct primitive is dm-flakey drop_writes (writes "succeed" but are discarded → pages go clean → evictable/re-readable on remount), armed ack-triggered so the strict build's fsync (which completed before ack) is already on platter and survives — preserving the strict-PASS invariant. You already reference drop_writes in the WS2.2 discussion; it just needs to replace the suspend+drop_caches recipe in the scenario spec. host.rs has neither drop_writes nor --nolockfs today.
• FAIL is a timing race, not deterministic. ext4 data=ordered commits ordered data pages on every jbd2 tick (~5s) with no fsync; a hot workload's next-PUT fdatasync (relaxed still fdatasyncs data shards) will flush the previous PUT's unsynced xl.meta via the shared jbd2 transaction. So calibration needs a quiet single-write workload, a tight maxAckToFaultMs (≤ ~2s) or an enlarged-commit fixture, run-spec recording of the fs commit/writeback params, a non-empty crash-window cohort assertion (else no_signal, not PASS), and a bounded retry budget. Hot-workload + ack-trigger self-defeats.

Blocker 3 — committed_version_missing can't be emitted on the path your P0 fix depends on (C3-3)

The post-return oracle wants "final checker sees a committed version gone → product FAIL." But runner.rs:1477-1482 hardcodes the final checker verdict to classification="product_or_environment", which projects to s3_model_classification=None, responsibility_domain=Unknown (reporting.rs:430 Unknown arm; not in the is_s3_model_classification allowlist) — while the same failure caught at pre-recommit gets a proper Product classification. So the #4221 signal is diluted in responsibility-domain routing exactly at the final gate. Wire the checker-final verdict to the s3-model mapper (derive committed_version_missing/committed_object_unavailable from checker-report evidence) before the post-return oracle can be trusted.

Blocker 4 — Heal-family scenarios have the same masking bug and aren't covered by the credibility thesis (D4-9)

fresh-volume-replacement-heal and on-disk-bitrot-heal can pass with heal completely broken: after replacing/corrupting one volume, GET reconstructs from the other 15 (≫ data=12) and bypasses the tested volume — even if version-aware heal never restored the old versions to the new volume, the S3-visible re-read is green. This is the same failure law as page-cache masking: any oracle that can satisfy reads from a non-tested replica is structurally blind. These scenarios need a "force reads through the healed/repaired volume by injecting io-error on the other P volumes" step, and must pass the same calibration gate.

Should-fix before merge (doc-vs-code consistency; individually fast-follow)

The doc is pinned to a baseline 10 merges behind main (c213a3d), while implementation (PR#18/#21/#23) has run ahead to PR-order step 5. As an authoritative design doc this misleads contributors in several places:

• D3-2: schema_version=2 is already burned by #23 with a narrower field set. The v2 table's observed_at_ms is the field that would retro-invalidate existing v2 artifacts if made required (run_id/attempt_index are Nullable and reserialize as null). → mark new v2 fields optional, defer strict-required to v3, note attempt context already lives in suite-summary.json.
• D3-3: The legacy-classification projection table contradicts merged code + tests — 3 legacy keys (checker_or_environment, environment_or_workload, workload_or_product) are real v2-writer output, not "compat inputs only." → doc follows code; promote them to first-class run_failure_reason in the 444-456 table.
• D3-8: The fall-through the doc forbids exists in the current v2 writer — unregistered classifications silently degrade to needs_investigation/unknown (reporting.rs). → exhaustive allowlist + a compile/test-time guard when the target table is wired, or a correctness failure will misproject.
• D3-1: console/ already exists (console.rs + console_server.rs; #23 edits console/app.js) and consumes v2 fields — the "future surface" and "LIST still promotes data_corruption" present-tense claims are false against merge target main. (This is the one genuine near-term blocker in the D3 set — it causes immediate duplicate work.)
• D3-4: primary_evidence_refs contract is three-way inconsistent — the schema section forbids self-reference and roots at the suite root, but the implementation always leads with failure-summary.json self-reference and validates same-dir.
• D3-7: the merged actuator port (#21) violates the doc's stated dependency direction (domain depends on backends::host concrete types).

One over-framing to correct in the review's own findings (fairness)

D4-1 (network faults absent from the durability matrix) was over-weighted as P1 "quorum boundary narrowed to power." Network partition is a consistency/availability axis, not the static-data-loss-after-power axis; RustFS single-master EC means a minority partition simply can't get write quorum (an availability event), and a rejoining node carries its latest accepted data, not an old disk generation — so "stale-node-rejoin = stale-disk-return dual" is wrong. Its legitimate core is "a cheaper executable path" (P2), and network scenarios' absence from the durability matrix should be an explicit non-goal, not a coverage gap. (Multi-target/asymmetric partition are still worth a catalog entry on the availability axis.)

Suggested execution order (the doc has none; this is the highest-leverage path)

1. Calibration ladder first — meta-gate, near-zero cost, and it immediately exposes the false-greens above.
2. Corrected soft-power-loss detection path (drop_writes recipe, ack-triggered, quiet workload, ≥n/2+1 or ≥P+1 by version type) — the only executable ack-then-lose detector without power hardware (none of minikube / dedicated-K3s / bare-metal has a power control plane; the bare-metal #834 env has documented root SSH but is excluded by non-goal — reconsider).
3. Volume-kind FixedTargets(N) targeting — shared base for both "≥P+1 volumes" and the heal force-read step. (Note: fault_kind_accepts_selection already accepts FixedTargets(count>0) for pod/network/flakey kinds at plan.rs:650; only the volume family rejects it at plan.rs:638. So same-kind multi-target pod kill already type-checks today — the blocker is the hardcoded mode: one render + FixedTargets(1), not faults.len()==1/CompositionPolicy. Quorum is same-kind-multi-target, not heterogeneous composition — worth distinguishing in the Composition Boundary section so implementers don't reach for the wrong mechanism.)

Happy to pair on the corrected crash sequence and the per-version-type quorum table — both are small once the physics is pinned. None of this detracts from the doc's structure; it mostly needs the fault-model section to close the ACK → S3-visible-loss causal chain before the Phase 4 roster is locked.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d31e9057a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Require PV node before accepting volume proof

For RustFS volume targets backed by PVs without a hostname node affinity (for example CSI volumes where pv_device_or_path still returns a volumeHandle), this predicate marks the generated target proof as satisfied because it only checks the PV name and device/path. The same artifact is then rejected by validate_target_proof, whose target_pod_has_bound_volume also requires pv.node, so an otherwise successful run can fail artifact validation after the fact. Keep the preflight satisfied criteria in sync with the validator, or stop requiring the PV node during validation.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Avoid PV lookups for pod-only faults

When running PodChaos, NetworkChaos, StressChaos, or dm-flakey scenarios under RBAC that permits pod access but not PVC/PV listing, this inventory path now fails before target proof even though those scenarios do not need volume-binding evidence; the runner calls rustfs_target_inventory for every scenario. This can turn otherwise valid pod-only fault runs into target-preflight failures because of unrelated storage permissions, so defer the PVC/PV queries until a volume target is being proven or make them best-effort for non-volume targets.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Prove the PVC mounted at the faulted volume path

For IOChaos volume scenarios where a RustFS pod has multiple PVC-backed volumes, this records every PVC from spec.volumes and later validation only checks that the recorded PVCs are bound; it never follows volumeMounts to prove which PVC is actually mounted at the configured volumePath such as /data/rustfs0. That lets target-proof.json pass while proving unrelated PVC/PV bindings rather than the volume the fault backend will inject into, which weakens the new target-aware safety gate.

Useful? React with 👍 / 👎.