refactor(heal): remove the dead MRF heal-type path - #6275
Merged
Conversation
HealType::MRF (a #1664-era "metadata repair file" task kind) had no production construction site left: its only builder lived in the HealEvent -> HealRequest converter, and the HealEvent/HealEventHandler queue itself had zero production references — both were superseded by the MrfIntent pipeline (mrf_queue.rs), which produces Object/Metadata/ ECDecode requests and never an MRF task. The dead path nevertheless carried ~700 lines: the whole event.rs module, the heal_mrf executor, a dedup-key arm, an overlap arm with the "\u{0}mrf" sentinel bucket hack, per-kind labels, and an empty MrfRuntime::record_accept shell. Deleting the variant is compile-time safe: HealType has no Serialize derive, the protos wire enums carry no heal-type discriminant (the receiver rebuilds it from HealChannelRequest fields), the MRF journal encodes MrfKind (1/2/3), and the scanner pending-heal ledger uses its own kind enum — none of them can name an MRF task. Also resolves the in-crate naming clash where "MRF" denoted both the dead task kind and the live mission-repair-feed loop; the loop stays, the task kind goes. Co-Authored-By: heihutu <heihutu@gmail.com>
Contributor
|
CLA requirements are satisfied for this pull request. |
houseme
enabled auto-merge (squash)
August 19, 2026 12:34
This was referenced Aug 19, 2026
The consumer's flush tick rewrote the full journal snapshot to every
local disk whenever `new_since_flush > 0 || queue.depth() > 0` held —
which is permanently true for a backlog waiting out an admission
backoff (manager full). Because the journal lives in the meta bucket,
each rewrite goes through the strict durability pin and fsyncs the file
plus parent directory on every disk: a steady-state backlog paid a full
re-encode + rewrite + fsync storm twice a second for a snapshot whose
bytes had not changed.
Gate the rewrite on a dirty flag that is set when the pending set
actually changes (channel push, dispatch pop, or an attempts bump that
alters the encoded record) and cleared on a flush that persisted to at
least one disk — a total write failure keeps the flag set so the next
tick retries the persist, preserving the periodic-retry self-healing a
non-empty queue used to provide. The tick decision is extracted into a
pure `tick_action(dirty, depth, journal_on_disk)` table — Flush / Retry
/ DeleteJournal / Idle — so a clean backlog skips the rewrite but keeps
draining (an expired backoff retries on time), and the delete-journal
branch only fires when nothing is pending. After replay arms the queue
at startup the flag is set whenever intents remain, so the
crash-recovery guarantee ("whatever the journal held is re-persisted")
is preserved. The channel-closed final flush still runs for a dirty or
non-empty state (the shutdown retry the unconditional flush used to
provide), and a dropped intent (queue ceiling hit) no longer counts
toward the flush threshold. The journal codec, format, and per-record
CRC are untouched.
Co-authored-by: heihutu <heihutu@gmail.com>
request_for_dedup_key answered "which queued request holds this key"
by linearly scanning the BinaryHeap and rebuilding every candidate's
dedup key with format! — on the submit path, so every heal submission
from the MRF consumer, the scanner, and read repair paid an O(queue)
string-allocation scan, which matters in mass-recovery scenarios where
the queue holds thousands of entries.
The queue already maintained a dedup_keys index but only for existence
checks. Upgrade its value from a bare refcount to DedupKeyEntry
{ refcount, representative_request_id }: the first request that opens a
key names the representative (captured before the request moves into
the heap), and both call sites — which only ever consumed queued.id —
now read it back in O(1) via queued_request_id_for_dedup_key. Push,
pop, displace, and remove keep the index consistent exactly as before.
Representative lifetime: the scheduler pop path deliberately keeps the
opener's name — the popped request surfaces in active_heals under that
same id, and the duplicate pre-check consults active heals before the
queue, so the named id always resolves. When a holder instead leaves
the queue without becoming active (canceled by id, or displaced under
queue pressure) and other holders survive — only reachable through
forced duplicates — refresh_dedup_representative re-elects a still-
queued holder so a merge receipt never names an id that resolves
nowhere. The re-election scan runs only on that rare admin path; the
per-submit lookup stays O(1).
The active/retrying dedup lookups stay linear on purpose: they are
bounded by the concurrency cap and single-digit retry sets, and O(1)
there would need a second cross-lock index for no measurable gain.
Co-authored-by: heihutu <heihutu@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
N/A (follow-up of the backlog#1862 heal/scanner audit review)
Summary of Changes
Removes ~930 lines of dead heal path: the
HealType::MRFtask kind ("metadata repair file", a #1664-era construct) and the wholeevent.rsHealEvent pipeline.HealType::MRF's only production constructor lived in the HealEvent-to-HealRequest converter, and the HealEvent/HealEventHandler queue itself had zero production references — both were superseded by the MrfIntent pipeline (mrf_queue.rs), whose consumer produces Object/Metadata/ECDecode requests and never an MRF task. The dead path nevertheless carried the entire event.rs module (683 lines), theheal_mrfexecutor, a dedup-key arm, an overlap arm with the\u{0}mrfsentinel-bucket hack, per-kind labels, two event-name constants, an emptyMrfRuntime::record_acceptshell, and four event-only tests. This also resolves the in-crate naming clash where "MRF" denoted both the dead task kind and the live mission-repair-feed loop; the loop stays, the task kind goes.Deletion is compile-time-safe:
HealTypehas no serde derive, the protos wire enums carry no heal-type discriminant (the receiver rebuilds the type fromHealChannelRequestfields, which cannot express MRF), the MRF journal encodesMrfKind(1/2/3) whose consumers never construct an MRF task, and the scanner pending-heal ledger uses its own kind enum. All verified by adversarial review (all seven roles, High-risk tier).Verification
cargo fmt --allcargo test -p rustfs-heal— 310 passed lib (was 328; the 18 deleted tests covered only the removed path) + all integration targets, 0 failed, including the untouched mrf_pipeline testscargo check -p rustfs— cleanmake pre-pr— passes (fmt + architecture guards + clippy + tests)Adversarial review (High risk — quorum/heal, all seven roles): correctness verified zero remaining references (grep) and that
HealPriority::Urgentkeeps live users; compatibility verified all four wire/persistence claims by hand; security/concurrency/performance null reports; test-skeptic verified the 18 deleted tests pinned only the dead path andis_per_objectnow pins all seven surviving variants.Impact
No behavior change — the removed code was unreachable in production. Binary size shrinks slightly; the heal-crate public API loses the
eventmodule (zero workspace consumers). Rollback is a clean revert.Additional Notes
Dated audit documents under docs/operations reference the removed symbols with line numbers; they are historical snapshots that already describe this path as dead and are left as-is.