From 5b2fe374fccaea88f6a3c146ff2470936a6a051a Mon Sep 17 00:00:00 2001 From: Eliah Rusin Date: Sun, 13 Sep 2026 00:19:18 +0300 Subject: [PATCH] [Volume] Scrub every disk's EC shards for a volume id, not just the first (#11258) * storage: add Store::find_all_ec_volumes for split-disk EC lookups Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: add merge_ec_runtimes to resolve a vid's per-disk shard set Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: replace dead slots.get(14) assertion with a width-14 pin Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: build the checksum scrub plan from every per-disk runtime Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: build the local scrub plan from every per-disk runtime Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: prove the local scrub plan reaches every runtime's slots Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: make the scrub plan tests falsifiable Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: report unverifiable protection when the sidecar predates the scrubbed encode Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: commit sidecar provenance with the sidecar it describes Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * volume server: scrub every disk's EC shards for CHECKSUM and LOCAL Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: run the FULL/READS parity check across split-disk shards Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * volume server: report fenced-out runtimes in FULL/READS scrubs Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: tighten verify_ec_shards ordering and missing-shard coverage Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * volume server: visit each EC volume id once in node-wide scrubs Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: cover split-disk scrub aggregation end to end Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * volume server: pin fenced-out disks and sibling-disk shards in EC scrubs Three scrub behaviors shipped without a test at the RPC seam. Task 8 showed the seam exists, so close them here. FULL/READS (mode 2|5) now marks a volume broken when the identity fence excludes a runtime, where it previously reported clean. Pinned against a control fixture whose two disks AGREE and scrub clean, so the test fails on the clean->broken transition, not only on the message text. That needs a structurally valid, tombstone-only .ecx (so the needle walk finds nothing to complain about) and a seeded shard-location cache (so the absent master does not short-circuit the scrub with an error of its own). LOCAL (mode 3) and CHECKSUM (mode 4) now build their plans from every per-disk runtime. Made observable by moving shard 0 -- the shard the volume's single needle spans and the one the checksum sidecar is checked against -- to the SIBLING disk, leaving shard 5 on the disk the singular find_ec_volume lookup returns. Built from that disk alone, neither scrub ever looks at shard 0. The split-disk fixture grows a config struct rather than more positional arguments; its defaults reproduce the existing layout byte for byte, so the node-wide dedupe test is unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: report fenced-out disks on a malformed sidecar too `errors.extend(self.skipped)` sat below the whole status match, so only `(Some(p), On)` ever reached it. The Invalid arm already returns a non-empty error vector of its own, so the Go-parity contract that silences the Off arm (`case BitrotOff: return 0, nil, nil`) does not reach it -- appending the fence lines there costs nothing that contract protects. A volume with BOTH a malformed sidecar and a disk the identity fence excluded reported only the sidecar, hiding the unscanned disk behind an unrelated integrity error. Off stays byte-identical, and so does the `(None, On)` arm that is documented as treating a missing payload defensively as protection off. Off is now the ONLY status that drops the report, and the comment at the On-path copy says so: that is the one place the parity constraint costs us coverage. Also corrects a false claim in the FULL/READS test's doc comment. It said a fenced-out disk "is a disk this scrub did NOT read", which is true only of the merge-driven parity half. The per-needle walk still resolves `store.find_ec_volume` (store_ec.rs:281) and binds `expected_encode_ts_ns` to that runtime (:311) -- position 0, the EXCLUDED one on that fixture -- so `read_local_intervals`' generation filter (:1204) makes it read the excluded disk and treat the anchor's shards as non-local, the inverse of what `skipped` reports. The fixture's tombstone-only .ecx walks nothing, so the test cannot tell the two apart; the comment now says that rather than implying coverage it does not have. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: take CHECKSUM's bitrot protection from the disk that has the sidecar `EcChecksumScrubPlan::for_volumes` read `(prot, status)` off the ANCHOR. The anchor is the first shard-bearing runtime at the maximum `encode_ts_ns`, chosen with no regard for which disk holds the `.ecsum`. That sidecar is deliberately NOT mirrored across disks -- `ec_metadata_dirs()` exists so one authoritative copy stays reachable rather than being duplicated -- and at mount `EcVolume::new` resolves it via `load_active_bitrot_sidecar(&[])` with no sibling directories at all; only the `VolumeEcShardsMount` RPC ever passes `ec_metadata_dirs()`. So after EVERY volume-server restart, the split-disk runtime that does not physically hold the sidecar mounts `BitrotStatus::Off`. When the one copy lives on disk 1 and the anchor is disk 0, `run()` hit `case BitrotOff` and returned `(0, [], [])`: the whole volume scrubbed clean, silently. That is the steady state for roughly half of all mirrored split-disk layouts, and it is the exact failure this branch exists to remove. Source protection from the first MERGED runtime that has any -- `On` if one does, else `Invalid`, else the anchor's `Off`. Two facts make that safe, and both are load-bearing: - Every runtime that mounted `On` already passed the `geometry_matches` gate in `load_bitrot_for_generation`, so its manifest agrees with the volume's layout. A sidecar that contradicted it would have failed the mount. - All merged runtimes share the same `encode_ts_ns` by construction of the identity fence, so a sidecar from any of them describes the same encode run. The `unverifiable_sidecar` provenance rule four lines down read `anchor.bitrot_source_dir`; it now reads the SAME runtime `prot` came from. Otherwise the two would describe different sidecars and the rule would vouch for a manifest nobody is scanning against. One consequence worth naming: that source dir is now non-empty by construction (a runtime with protection found a file), where the anchor's was often "" and short-circuited the rule -- so on a fenced volume whose anchor had no sidecar, an unverifiable-protection note now surfaces where previously nothing was reported at all. `run()` is untouched, and the `BitrotStatus::Off` arm still returns `(0, [], [])` exactly, for Go parity with `case BitrotOff: return 0, nil, nil`. `parity_shards` still comes from the anchor while `prot` may come from a sibling; the geometry gate above makes them agree, and slot-width agreement is handled separately. The test drives mode 4 through the real RPC against a split-disk volume whose sidecar exists only on dir1, and asserts up front that the anchor mounted `Off` and the sibling `On` -- otherwise it would prove nothing. Reverting this commit's one-line source change makes it report `[]` instead of `[0, 5]`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: pin the slot width, contain the shard-size fallback, and cover multi-disk FULL Five findings from the whole-branch review, none of which changes what a healthy volume reports. Slot width was undefined and the two consumers disagreed (ec_volume.rs). `merge_ec_runtimes` sizes `slots` to the WIDEST merged runtime, but the identity fence keys on `encode_ts_ns` alone and never on geometry -- so two same-generation runtimes whose `.vif`s disagree do merge. The mode 2|5 arm truncates to the anchor's `data+parity` and silently drops the surplus slots, while `EcChecksumScrubPlan::for_volumes` iterated the full width and emitted "present but missing from sidecar manifest" for exactly those ids. Nothing in the volume describes them -- the sidecar manifest and the Reed-Solomon matrix are both the anchor's -- so that message was the width disagreement talking, not a finding. The `slots` field doc now states the contract (the range is the anchor's geometry; every consumer truncates to it) and CHECKSUM truncates. The LOCAL `shard_size` fallback had grown a node-wide blast radius (ec_volume.rs). `anchor.shard_file_size()` returns the anchor's FIRST held shard, not a maximum. Before aggregation the plan read only that runtime's own shards, so a truncated shard was contained to its disk; now that one value sizes every merged sibling's shards, mis-offsetting `locate_data` and manufacturing needle corruption across the node. Take the max over the merged slots, which is how `verify_ec_shards` already answers the same question (`if size > shard_size { shard_size = size }`). Only on the legacy `dat_file_size == 0` path. Multi-disk `all_local` had no end-to-end test (grpc_server.rs). The parity check is gated on every shard being present, and the one all-local fixture keeps them in a single directory, so every entry of `dirs` is the same string and a permutation or off-by-one in the `slots` -> `dirs` mapping is invisible; `test_verify_ec_shards_reads_shards_from_multiple_dirs` builds its `dirs` by hand and never goes through `merge_ec_runtimes`. The new fixture is a real 10+4 encode split 0..=6 / 7..=13 across two store locations (the `.dat`/`.idx` stay outside both, so `prune_incomplete_ec_with_sibling_dat` has nothing to act on), driven through the real RPC: clean first, then a corrupted PARITY shard on the SECOND disk -- which only the parity half can see, and only through a correct mapping. Shifting that mapping by one, or computing `all_local` from the anchor alone, both make it report `[]` instead of `[13]`. Deleted `test_ec_volume_enumeration_is_deduped` (store_ec_reconcile.rs). It built `raw` from `store.locations` and then applied its OWN inline `filter(|v| seen.insert(*v))`, asserting on that -- a property of `HashSet::insert`, never reaching the production dedupe. That path is covered by `test_scrub_ec_volume_node_wide_dedupes_a_split_disk_volume`, which does fail (2 != 1) when the dedupe is removed. Corrected `test_verify_ec_shards_treats_a_none_dir_as_missing`'s docstring (ec_encoder.rs). It claimed the unmounted shard "must not drag the shards that ARE mounted down with it", but `dirs[5] = None` puts shard 5 in `broken_shards` before the block loop, so every iteration takes the `read_failed` arm and the parity comparison never runs: corrupting a mounted shard in that fixture changes nothing about the result. The assertions are unchanged; the docstring now states what they actually establish. Also refreshed two comments that cited `shard_file_size() - 1` as the reason `merge_ec_runtimes` prefers a shard-bearing anchor -- true before this commit, stale after it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: correct the Fix 1 rationale and truncate the shard-size scan The safety argument attached to `EcChecksumScrubPlan::for_volumes`'s protection selection was false as written, and it is the argument a reviewer reads first. `geometry_matches` compares a sidecar against the MOUNTING runtime's own data/parity/block size, not the anchor's, and returns true vacuously when `ec_shard_config` is `None` -- so it establishes agreement only when all merged runtimes share one geometry, which an `encode_ts_ns`-only fence does not guarantee and which `test_checksum_scrub_truncates_slots_to_the_anchors_geometry` constructs a counterexample to. The second clause was weaker than stated too: a `.ecsum` records no encode identity at all, so merged runtimes agreeing on `encode_ts_ns` does not transfer to the sidecar. Replace it with the property that is true, checkable from the selection itself, and stronger for what actually matters. `anchor` is an element of `merged`, so the `.unwrap_or(anchor)` fallback is reached only when no merged runtime is `On` and none is `Invalid` -- in which case the anchor is necessarily `Off`. The status can therefore only move `Off -> On`, `Off -> Invalid` or `Invalid -> On`; never `On -> Off`, never `Invalid -> Off`. This selection cannot stop a volume that was being scanned from being scanned, and cannot turn a reported integrity error into silence: every change it makes is toward more verification. The comment now also states what it does NOT establish -- geometry agreement is not guaranteed -- and names geometry fencing as the follow-up that would close it. Second, `EcLocalScrubPlan::for_volumes`'s `shard_size` max scanned the FULL slot width, violating the `slots` contract documented in the same commit that introduced the max: the volume's shard-id range is the anchor's geometry and every consumer must truncate to it. Pre-fix that input could not exist, because `anchor.shard_file_size()` read only the anchor's own anchor-sized vector -- so the max opened a new, narrow path to the same node-wide mis-sizing it exists to close (same-generation runtimes with disagreeing `.vif`s, the wider one holding an out-of-geometry shard larger than the in-geometry ones, `dat_file_size == 0`). `.take(anchor.data_shards + anchor.parity_shards)` mirrors the truncation already applied to the CHECKSUM shard scan. The sibling `shards:` vector is left untruncated on purpose: every access in `EcLocalScrubPlan::run` is `shards.get(sid)` with `sid < data_shards`, so the surplus entries are inert. No behavior change for any healthy volume, and no test added -- the suite is unchanged at 575 passing, 0 failing, 0 warnings. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: aggregate split-disk runtimes in Go scrubs, mirroring Rust Go volume scrubs previously used FindEcVolume (first runtime only), so a volume whose EC shards are split across multiple disks was scrubbed against just one disk's shards and the others were silently skipped. Node-wide ScrubEcVolume also appended each disk's EcVolumeIds without deduplication, scrubbing a split-disk volume once per disk. Add MergedEcRuntimes/MergeEcRuntimes (Go counterpart to Rust's merge_ec_runtimes): select the maximum EncodeTsNs as the anchor generation, fence out runtimes whose encode generation or geometry (DataShards, ParityShards, BlockSize) disagrees with the anchor, merge shard handles by shard ID, and report excluded runtimes rather than dropping them. Wire it into every scrub mode: - INDEX: scrub the anchor's index, report skipped runtimes. - LOCAL: aggregate local shards across all merged runtimes via a synthetic EcVolume built from the merged shard slots. - FULL/READS: resolve the runtime matching the anchor's encode generation (not the first match) so the needle walk and parity phase inspect one encode run; report skipped runtimes. - CHECKSUM: take bitrot protection from the first merged runtime that has a valid sidecar (On, else Invalid, else anchor's Off), preserve invalid sidecar errors from every other merged runtime, and report skipped runtimes. Deduplicate EC volume IDs in node-wide ScrubEcVolume so each volume is scrubbed exactly once. Refactor ScrubEcVolume to share the per-needle walk via scrubEcVolumeWalk, called by both the legacy first-runtime path and the new merged path. Add Go regression tests covering split-disk deduplication, encode-generation fencing, geometry fencing, sibling-disk LOCAL reach, and merge anchor selection. Rust: keep the previously-landed merge/fence/checksum changes intact; revert incidental cargo-fmt drift from unrelated files so the diff stays focused. * ec: fence merged CHECKSUM on sidecar encode generation and fix legacy shard size Address two review findings on the Go merged-runtime scrub: 1. Sidecar provenance: a merged runtime can load a bitrot sidecar from a sibling metadata directory (ReloadBitrotSidecar), and the merge fence may then exclude the runtime owning that directory. Generation-0 sidecars do not identify the encode run, so geometry validation alone cannot prove the borrowed manifest describes the anchor shards. If the sidecar records a non-zero EncodeTsNs that disagrees with the anchor, refuse the scan instead of applying stale checksums to current shards and reporting false corruption. 2. Legacy shard size: for volumes without datFileSize in .vif, LocateEcShardNeedleInterval derives the shard size from Shards[0].ecdFileSize. The merged shard set is compacted in shard-ID order, so a truncated lowest-ID shard would shrink every interval and misread intact sibling shards. Synthesize a datFileSize from the maximum mounted shard size when the anchor lacks one, so the datFileSize>0 path uses the largest shard size across all merged runtimes. * ec: fix copylocks, legacy shard boundary, and encode-aware Rust lookups Address review findings from CodeRabbit and Devin: Go (ec_volume_merge.go): - Remove bitrotLock copy from the synthetic EcVolume: copying a sync.RWMutex is a go vet copylocks error. The synthetic volume uses its own zero-value mutex; bitrot/bitrotStatus are set directly before ChecksumScrub reads them via BitrotProtection(), so no concurrent access occurs. - Fix legacy shard-size boundary: synthesize datFileSize from (maxShardSize - 1) * DataShards, not maxShardSize * DataShards, to match the legacy fallback in LocateEcShardNeedleInterval (ecdFileSize - 1). An exact large-block boundary is ambiguous; the unadjusted size would select an extra large row and misread intact sibling shards. Rust (store_ec.rs): - Add find_ec_volume_for_scrub helper that resolves by encode generation (not first-match find_ec_volume) and use it in scrub_snapshot_under_lock, write_back_shard_locations, and the post-refresh shard-location read. Previously the encode-aware lookup was only used for the initial runtime selection; the cache write-back and per-needle snapshot still used first-match, so a split-disk volume whose first runtime was from an older encode run would write to and read from the wrong runtime's shard-location cache and falsely abort with 'remounted as a different encode run'. --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Chris Lu --- seaweed-volume/src/server/grpc_server.rs | 1031 +++++++++++-- seaweed-volume/src/server/store_ec.rs | 228 +-- .../src/storage/erasure_coding/ec_encoder.rs | 170 ++- .../src/storage/erasure_coding/ec_volume.rs | 1312 +++++++++++++++-- seaweed-volume/src/storage/store.rs | 14 + .../src/storage/store_ec_reconcile.rs | 70 + weed/server/volume_grpc_scrub.go | 43 +- .../server/volume_grpc_scrub_ec_merge_test.go | 178 +++ .../storage/erasure_coding/ec_volume_merge.go | 301 ++++ weed/storage/store_ec_scrub.go | 43 + 10 files changed, 3037 insertions(+), 353 deletions(-) create mode 100644 weed/server/volume_grpc_scrub_ec_merge_test.go create mode 100644 weed/storage/erasure_coding/ec_volume_merge.go diff --git a/seaweed-volume/src/server/grpc_server.rs b/seaweed-volume/src/server/grpc_server.rs index bf201098e..051a13b5d 100644 --- a/seaweed-volume/src/server/grpc_server.rs +++ b/seaweed-volume/src/server/grpc_server.rs @@ -65,7 +65,10 @@ fn unix_now_seconds() -> f64 { /// caller mistake, not a concurrent change, and callers match on the message. fn scrub_vanished_volume(explicit: bool, kind: &str, vid: VolumeId) -> Option { if explicit { - return Some(Status::not_found(format!("{} id {} not found", kind, vid.0))); + return Some(Status::not_found(format!( + "{} id {} not found", + kind, vid.0 + ))); } tracing::info!( volume_id = vid.0, @@ -146,9 +149,7 @@ fn save_state_file( } /// Load VolumeServerState from a state.pb file (matches Go's State.Load). -pub fn load_state_file( - path: &str, -) -> Option { +pub fn load_state_file(path: &str) -> Option { if path.is_empty() || !std::path::Path::new(path).exists() { return None; } @@ -184,8 +185,7 @@ fn select_rebuild_location(loc_infos: &[LocInfo]) -> Option<(usize, Vec) for (i, info) in loc_infos.iter().enumerate() { let better = info.has_ecx - && rebuild_loc_idx - .is_none_or(|prev| info.shard_count > loc_infos[prev].shard_count); + && rebuild_loc_idx.is_none_or(|prev| info.shard_count > loc_infos[prev].shard_count); if better { if let Some(prev) = rebuild_loc_idx { other_dirs.push(loc_infos[prev].dir.clone()); @@ -294,9 +294,7 @@ impl VolumeGrpcService { host, remote, ); - return Err(Status::permission_denied(format!( - "not authorized: {host}" - ))); + return Err(Status::permission_denied(format!("not authorized: {host}"))); } Ok(()) } @@ -451,7 +449,11 @@ impl VolumeGrpcService { let store = self.state.store.read().unwrap(); store.find_volume(*vid).map(|(_, v)| { // INDEX mode (1) calls scrub_index; FULL (2) and LOCAL (3) call scrub - let r = if mode == 1 { v.scrub_index() } else { v.scrub() }; + let r = if mode == 1 { + v.scrub_index() + } else { + v.scrub() + }; (r, v.file_count()) }) }) else { @@ -599,24 +601,51 @@ impl VolumeGrpcService { // deliberate divergence from Go FULL to preserve coverage. Drop // verify_ec_shards from this arm once mode 4 (CHECKSUM) lands. // - // The RS recompute needs every shard co-located, so only run it - // when this node holds all data+parity shards (single-node EC); - // on a distributed layout it would report every non-local shard - // as missing. Snapshot under a brief lock; release before await. - let Some((dir, collection, data_shards, parity_shards, all_local)) = ({ + // The RS recompute needs every shard, so only run it when this + // node holds all data+parity shards (single-node EC); on a + // distributed layout it would report every non-local shard as + // missing. The shards need not share one disk: `verify_ec_shards` + // takes a directory per shard id, so a reconciled volume split + // across this node's disks still qualifies. + // Snapshot under a brief lock; release before await. + let Some(( + dirs, + collection, + data_shards, + parity_shards, + all_local, + skipped, + encode_ts_ns, + )) = ({ let store = self.state.store.read().unwrap(); - store.find_ec_volume(vid).map(|ecv| { - let total = (ecv.data_shards + ecv.parity_shards) as usize; - let local = ecv.shards.iter().filter(|s| s.is_some()).count(); - ( - ecv.dir.clone(), - ecv.collection.clone(), - ecv.data_shards as usize, - ecv.parity_shards as usize, - local == total, - ) - }) - }) else { + let runtimes = store.find_all_ec_volumes(vid); + crate::storage::erasure_coding::ec_volume::merge_ec_runtimes(&runtimes).map( + |m| { + let total = + (m.anchor.data_shards + m.anchor.parity_shards) as usize; + let dirs: Vec> = (0..total) + .map(|id| { + m.slots + .get(id) + .copied() + .flatten() + .map(|(owner, _)| owner.dir.clone()) + }) + .collect(); + let all_local = dirs.iter().all(|d| d.is_some()); + ( + dirs, + m.anchor.collection.clone(), + m.anchor.data_shards as usize, + m.anchor.parity_shards as usize, + all_local, + m.skipped, + m.anchor.encode_ts_ns, + ) + }, + ) + }) + else { if let Some(status) = scrub_vanished_volume(explicit, "EC volume", vid) { return Err(status); } @@ -629,19 +658,23 @@ impl VolumeGrpcService { crate::server::store_ec::scrub_ec_volume_distributed( &self.state, vid, + encode_ts_ns, force_deleted_needles_check, mode == 5, ) .await; - total_files += files as u64; // count comes from the needle walk only + total_files += files as u64; + + errs.extend(skipped); // (2) Local parity check, gated on all-shards-local. Blocking RS // verify -> spawn_blocking; inputs are owned, no lock held. - if all_local && !dir.is_empty() { + if all_local { let collection_pc = collection.clone(); + let dirs_pc = dirs; let join = tokio::task::spawn_blocking(move || { crate::storage::erasure_coding::ec_encoder::verify_ec_shards( - &dir, + &dirs_pc, &collection_pc, vid, data_shards, @@ -706,7 +739,11 @@ impl VolumeGrpcService { // under the guard stalls the node. See EcChecksumScrubPlan. let Some(plan) = ({ let store = self.state.store.read().unwrap(); - store.find_ec_volume(vid).map(|ecv| ecv.scrub_local_plan()) + // Every disk holding this vid, not just the first: a + // reconciled volume's shards are split across runtimes. + crate::storage::erasure_coding::ec_volume::EcLocalScrubPlan::for_volumes( + &store.find_all_ec_volumes(vid), + ) }) else { if let Some(status) = scrub_vanished_volume(explicit, "EC volume", vid) { return Err(status); @@ -747,9 +784,12 @@ impl VolumeGrpcService { // under the guard stalls the node. See EcChecksumScrubPlan. let Some((plan, collection)) = ({ let store = self.state.store.read().unwrap(); - store - .find_ec_volume(vid) - .map(|ecv| (ecv.checksum_scrub_plan(), ecv.collection.clone())) + let runtimes = store.find_all_ec_volumes(vid); + let collection = runtimes.first().map(|ecv| ecv.collection.clone()); + crate::storage::erasure_coding::ec_volume::EcChecksumScrubPlan::for_volumes( + &runtimes, + ) + .zip(collection) }) else { if let Some(status) = scrub_vanished_volume(explicit, "EC volume", vid) { return Err(status); @@ -1717,8 +1757,12 @@ impl VolumeServer for VolumeGrpcService { }; if let Err(e) = save_state_file(&self.state.state_file_path, &pb) { // Rollback in-memory state on save failure (matches Go) - self.state.maintenance.store(prev_maintenance, Ordering::Relaxed); - self.state.state_version.store(prev_version, Ordering::Relaxed); + self.state + .maintenance + .store(prev_maintenance, Ordering::Relaxed); + self.state + .state_version + .store(prev_version, Ordering::Relaxed); return Err(Status::internal(format!("failed to save state: {}", e))); } @@ -1854,9 +1898,8 @@ impl VolumeServer for VolumeGrpcService { // Write a .note file to indicate copy in progress. A leftover note // fails the volume load on restart, so a write failure must abort. let note_path = format!("{}.note", data_base_name); - std::fs::write(¬e_path, format!("copying from {}", source)).map_err(|e| { - Status::internal(format!("write .note for volume {}: {}", vid, e)) - })?; + std::fs::write(¬e_path, format!("copying from {}", source)) + .map_err(|e| Status::internal(format!("write .note for volume {}: {}", vid, e)))?; let (tx, rx) = tokio::sync::mpsc::channel::>(16); @@ -2959,7 +3002,7 @@ impl VolumeServer for VolumeGrpcService { // (V3) or checksum only (V2) + padding. Validate minimum // footer length for the protocol version. use crate::storage::types::{ - NEEDLE_CHECKSUM_SIZE, TIMESTAMP_SIZE, VERSION_3, Version, + Version, NEEDLE_CHECKSUM_SIZE, TIMESTAMP_SIZE, VERSION_3, }; let version = Version(resp_version as u8); let min_footer = if version >= VERSION_3 { @@ -3031,10 +3074,7 @@ impl VolumeServer for VolumeGrpcService { &dir, &idx_dir, collection, vid, ) .map_err(|e| { - tonic::Status::internal(format!( - "read ec shard config for volume {}: {}", - vid.0, e - )) + tonic::Status::internal(format!("read ec shard config for volume {}: {}", vid.0, e)) })?; let block_size = match crate::storage::erasure_coding::ec_encoder::write_ec_files( @@ -3198,10 +3238,7 @@ impl VolumeServer for VolumeGrpcService { vid, ) .map_err(|e| { - tonic::Status::internal(format!( - "read ec shard config for volume {}: {}", - vid.0, e - )) + tonic::Status::internal(format!("read ec shard config for volume {}: {}", vid.0, e)) })?; let total_shards = data_shards + parity_shards; @@ -3630,7 +3667,8 @@ impl VolumeServer for VolumeGrpcService { // shards. Unload and remove only the strictly-older disks, never node-wide. let mut store = self.state.store.write().unwrap(); for disk_id in 0..store.locations.len() { - let disk_gen = store.locations[disk_id].ec_generation_ts_ns(&req.collection, vid); + let disk_gen = + store.locations[disk_id].ec_generation_ts_ns(&req.collection, vid); let older = matches!(disk_gen, Some(g) if g > 0 && g < req.encode_ts_ns); if !older { tracing::info!( @@ -4379,10 +4417,7 @@ impl VolumeServer for VolumeGrpcService { // The error otherwise goes to a channel nobody is // reading, leaving an abandoned tier move with no trace. if e.code() == tonic::Code::Cancelled { - tracing::info!( - "volume {} tier move to remote abandoned by its caller", - vid - ); + tracing::info!("volume {} tier move to remote abandoned by its caller", vid); } else { tracing::warn!("volume {} tier move to remote failed: {}", vid, e); } @@ -4544,14 +4579,25 @@ impl VolumeServer for VolumeGrpcService { // Restore the .dat mtime so a reload computes TTL from real data age, // not download time (matches Go VolumeTierMoveDatFromRemote). if remote_modified_secs > 0 { - let modified_ts_ns = (remote_modified_secs as i64).saturating_mul(1_000_000_000); + let modified_ts_ns = + (remote_modified_secs as i64).saturating_mul(1_000_000_000); if let Err(e) = set_file_mtime(&dat_path, modified_ts_ns) { - tracing::warn!("volume {} restore data file {} modified time: {}", vid, dat_path, e); + tracing::warn!( + "volume {} restore data file {} modified time: {}", + vid, + dat_path, + e + ); } else if let Ok(dat_file) = tokio::fs::File::open(&dat_path).await { // Persist the restored mtime past the download's content fsync; // best-effort, only TTL accuracy depends on it surviving a crash. if let Err(e) = dat_file.sync_all().await { - tracing::warn!("volume {} fsync data file {} after mtime restore: {}", vid, dat_path, e); + tracing::warn!( + "volume {} fsync data file {} after mtime restore: {}", + vid, + dat_path, + e + ); } } } @@ -4983,10 +5029,15 @@ impl VolumeServer for VolumeGrpcService { req.volume_ids.iter().map(|&id| VolumeId(id)).collect() } else { let store = self.state.store.read().unwrap(); + // A vid mounted on N disks appears N times here. Left as-is, each + // duplicate rescans the same runtime and inflates total_volumes, + // while the sibling disks' shards are still never reached. + let mut seen = std::collections::HashSet::new(); store .locations .iter() .flat_map(|loc| loc.ec_volumes().map(|(vid, _)| *vid)) + .filter(|vid| seen.insert(*vid)) .collect() }; @@ -5970,7 +6021,9 @@ mod tests { (format!("http://{}", addr), shutdown_tx, delete_count) } - fn make_remote_only_service(backend_id: &str) -> ( + fn make_remote_only_service( + backend_id: &str, + ) -> ( VolumeGrpcService, TempDir, tokio::sync::oneshot::Sender<()>, @@ -6027,7 +6080,10 @@ mod tests { }; { let mut registry = global_s3_tier_registry().write().unwrap(); - registry.register(format!("s3.{}", backend_id), S3TierBackend::new(&tier_config)); + registry.register( + format!("s3.{}", backend_id), + S3TierBackend::new(&tier_config), + ); } let vif = crate::storage::volume::VifVolumeInfo { @@ -6096,7 +6152,10 @@ mod tests { // The tier-down handler resolves the backend from the per-server // registry, so register it here too (reads use the global one). let mut reg = crate::remote_storage::s3_tier::S3TierRegistry::new(); - reg.register(format!("s3.{}", backend_id), S3TierBackend::new(&tier_config)); + reg.register( + format!("s3.{}", backend_id), + S3TierBackend::new(&tier_config), + ); reg }), read_mode: crate::config::ReadMode::Local, @@ -6236,13 +6295,14 @@ mod tests { // Carry a peer address so the admin gate sees a caller; the test guard // has an empty whitelist, so any peer is accepted. - let mut request = Request::new(volume_server_pb::VolumeConsolidateIndexRequest { - volume_id: 1, - }); - request.extensions_mut().insert(tonic::transport::server::TcpConnectInfo { - local_addr: None, - remote_addr: Some("127.0.0.1:65000".parse().unwrap()), - }); + let mut request = + Request::new(volume_server_pb::VolumeConsolidateIndexRequest { volume_id: 1 }); + request + .extensions_mut() + .insert(tonic::transport::server::TcpConnectInfo { + local_addr: None, + remote_addr: Some("127.0.0.1:65000".parse().unwrap()), + }); service.volume_consolidate_index(request).await.unwrap(); @@ -6344,9 +6404,7 @@ mod tests { let dest = format!("{}/aborted.dat", tmp.path().to_str().unwrap()); let err = backend - .download_file(&dest, "remote-key", |_, _| { - Err("caller gone".to_string()) - }) + .download_file(&dest, "remote-key", |_, _| Err("caller gone".to_string())) .await .expect_err("a failing progress callback must abort the download"); assert!(err.contains("caller gone"), "unexpected error: {}", err); @@ -6496,9 +6554,7 @@ mod tests { // Serve a VolumeGrpcService over a real gRPC endpoint. VolumeCopy dials its // source_data_node rather than calling in process, so a cancellation test // needs a source that is reachable over the wire. - async fn serve_source( - service: VolumeGrpcService, - ) -> (u16, tokio::sync::oneshot::Sender<()>) { + async fn serve_source(service: VolumeGrpcService) -> (u16, tokio::sync::oneshot::Sender<()>) { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let port = listener.local_addr().unwrap().port(); let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel::<()>(); @@ -6538,17 +6594,15 @@ mod tests { .connect() .await .unwrap(); - let mut client = - volume_server_pb::volume_server_client::VolumeServerClient::new(channel) - .max_decoding_message_size(GRPC_MAX_MESSAGE_SIZE) - .max_encoding_message_size(GRPC_MAX_MESSAGE_SIZE); + let mut client = volume_server_pb::volume_server_client::VolumeServerClient::new(channel) + .max_decoding_message_size(GRPC_MAX_MESSAGE_SIZE) + .max_encoding_message_size(GRPC_MAX_MESSAGE_SIZE); let dest_tmp = TempDir::new().unwrap(); let dest_path = format!("{}/copied.dat", dest_tmp.path().to_str().unwrap()); - let (tx, rx) = tokio::sync::mpsc::channel::< - Result, - >(16); + let (tx, rx) = + tokio::sync::mpsc::channel::>(16); drop(rx); // exactly what a client hanging up does to the sender let mut next_report_target: i64 = 128 * 1024 * 1024; @@ -6781,7 +6835,10 @@ mod tests { assert_ne!(messages[0].modified_ts_ns, 0); assert!(messages[1..].iter().all(|m| m.modified_ts_ns == 0)); - let copied: Vec = messages.iter().flat_map(|m| m.file_content.clone()).collect(); + let copied: Vec = messages + .iter() + .flat_map(|m| m.file_content.clone()) + .collect(); assert_eq!(copied, dat_bytes); } @@ -6846,7 +6903,10 @@ mod tests { "expected multiple streamed chunks, got {}", messages.len() ); - let copied: Vec = messages.iter().flat_map(|m| m.file_content.clone()).collect(); + let copied: Vec = messages + .iter() + .flat_map(|m| m.file_content.clone()) + .collect(); assert_eq!(copied, dat_bytes[super_block_size as usize..]); } @@ -7074,7 +7134,11 @@ mod tests { v.compact_by_index(0, 0, |_| true).unwrap(); v.commit_compact().unwrap(); - (before, v.super_block.compaction_revision, v.last_compact_revision()) + ( + before, + v.super_block.compaction_revision, + v.last_compact_revision(), + ) }; assert_eq!(revision_before, 0, "fresh volume starts at revision 0"); @@ -7250,15 +7314,13 @@ mod tests { .await .unwrap(); service - .volume_ec_shards_mount(Request::new( - volume_server_pb::VolumeEcShardsMountRequest { - volume_id: 1, - collection: String::new(), - shard_ids: (0..14).collect(), - source_disk_type: String::new(), - recover_missing_index: false, - }, - )) + .volume_ec_shards_mount(Request::new(volume_server_pb::VolumeEcShardsMountRequest { + volume_id: 1, + collection: String::new(), + shard_ids: (0..14).collect(), + source_disk_type: String::new(), + recover_missing_index: false, + })) .await .unwrap(); @@ -7335,15 +7397,13 @@ mod tests { .await .unwrap(); service - .volume_ec_shards_mount(Request::new( - volume_server_pb::VolumeEcShardsMountRequest { - volume_id: 1, - collection: String::new(), - shard_ids: (0..14).collect(), - source_disk_type: String::new(), - recover_missing_index: false, - }, - )) + .volume_ec_shards_mount(Request::new(volume_server_pb::VolumeEcShardsMountRequest { + volume_id: 1, + collection: String::new(), + shard_ids: (0..14).collect(), + source_disk_type: String::new(), + recover_missing_index: false, + })) .await .unwrap(); @@ -7357,8 +7417,7 @@ mod tests { for sid in 0u8..14 { locs.insert(sid, vec!["127.0.0.1:255.1".to_string()]); } - *ecv.shard_locations_refresh_time.lock().unwrap() = - Some(std::time::Instant::now()); + *ecv.shard_locations_refresh_time.lock().unwrap() = Some(std::time::Instant::now()); } // All shards local: FULL is clean. @@ -7392,6 +7451,766 @@ mod tests { assert_eq!(resp.total_files, 1); } + /// Two locations, one vid: the split-disk / cross-disk-reconcile layout + /// `build_split_disk_store` exercises in store_ec_reconcile.rs. Neither + /// disk holds a `.dat`, so each disk's orphan shard survives + /// `check_orphaned_shards` and `Store::add_location`'s automatic + /// `reconcile_ec_shards_across_disks()` mounts it against its own disk, + /// pointing at the sibling's `.ecx` -- the same layout a real + /// seaweedfs/seaweedfs#9212 report produces. + fn make_local_service_with_split_disk_ec_volume(vid_raw: u32) -> (VolumeGrpcService, TempDir) { + SplitDiskEcFixture::new(vid_raw).build() + } + + /// Knobs on the two-location layout above, so one fixture can produce the + /// shapes the scrub tests need. `new()` reproduces the layout exactly as + /// `make_local_service_with_split_disk_ec_volume` has always built it; every + /// field is a deliberate departure from it. + struct SplitDiskEcFixture { + vid_raw: u32, + /// Which shard id each disk gets. Which disk holds the shard a needle + /// actually spans is the whole difference between reaching one disk and + /// reaching both, so the LOCAL/CHECKSUM tests move shard 0 to disk 1. + dir0_shard_id: u8, + dir1_shard_id: u8, + /// dir0's own `.vif` encode identity. `None` writes NO `.vif` in dir0 at + /// all -- the original layout, where `locate_vif_path` falls through to + /// dir1's copy and both runtimes therefore agree. `Some(ts)` writes one, + /// which is what lets the two disks disagree and drives the identity + /// fence in `merge_ec_runtimes`. + dir0_encode_ts_ns: Option, + dir1_encode_ts_ns: i64, + /// `false` writes the original 20 zero bytes: a structurally invalid + /// index, so every scrub that walks it reports something. `true` writes + /// one well-formed TOMBSTONE entry instead -- a deleted needle, which the + /// walk skips -- so a FULL/READS scrub of this volume is CLEAN and a + /// test can pin a clean-to-broken transition rather than a message. + clean_index: bool, + /// Where the generation-0 `.ecsum` is written. Its per-shard checksums + /// do not describe the placeholder bytes, so a CHECKSUM scrub reports + /// every shard it actually reads. With `Nowhere`, `bitrot_status` is + /// `Off` on both disks and `EcChecksumScrubPlan::run` returns clean + /// before touching a handle. + bitrot_sidecar: SidecarPlacement, + } + + /// Which disks get the `.ecsum`. The sidecar is deliberately NOT mirrored + /// in production (`Store::ec_metadata_dirs` exists so one copy stays + /// reachable rather than being duplicated), and at mount `EcVolume::new` + /// resolves it with no sibling directories -- so `Dir1Only` is not an + /// artificial shape, it is what a volume server looks like after any + /// restart when the one copy happens to live on the non-anchor disk. + #[derive(Clone, Copy, PartialEq, Eq)] + enum SidecarPlacement { + Nowhere, + BothDisks, + Dir1Only, + } + + impl SplitDiskEcFixture { + fn new(vid_raw: u32) -> Self { + SplitDiskEcFixture { + vid_raw, + dir0_shard_id: 0, + dir1_shard_id: 1, + dir0_encode_ts_ns: None, + dir1_encode_ts_ns: 0, + clean_index: false, + bitrot_sidecar: SidecarPlacement::Nowhere, + } + } + + fn build(self) -> (VolumeGrpcService, TempDir) { + make_local_service_with_split_disk_ec_volume_from(self) + } + } + + fn make_local_service_with_split_disk_ec_volume_from( + cfg: SplitDiskEcFixture, + ) -> (VolumeGrpcService, TempDir) { + let SplitDiskEcFixture { + vid_raw, + dir0_shard_id, + dir1_shard_id, + dir0_encode_ts_ns, + dir1_encode_ts_ns, + clean_index, + bitrot_sidecar, + } = cfg; + + let tmp = TempDir::new().unwrap(); + let dir0 = tmp.path().join("data0"); + let dir1 = tmp.path().join("data1"); + std::fs::create_dir_all(&dir0).unwrap(); + std::fs::create_dir_all(&dir1).unwrap(); + + let ec_vif = |encode_ts_ns: i64| crate::storage::volume::VifVolumeInfo { + version: 3, + ec_shard_config: Some(crate::storage::volume::VifEcShardConfig { + data_shards: 10, + parity_shards: 4, + encode_ts_ns, + ..Default::default() + }), + ..Default::default() + }; + + // dir0: one shard, no .ecx. + std::fs::write( + dir0.join(format!("{}.ec{:02}", vid_raw, dir0_shard_id)), + b"shard data nonempty", + ) + .unwrap(); + if let Some(ts) = dir0_encode_ts_ns { + std::fs::write( + dir0.join(format!("{}.vif", vid_raw)), + serde_json::to_string(&ec_vif(ts)).unwrap(), + ) + .unwrap(); + } + // dir1: one shard plus the index files. + std::fs::write( + dir1.join(format!("{}.ec{:02}", vid_raw, dir1_shard_id)), + b"shard data nonempty", + ) + .unwrap(); + let ecx = if clean_index { + let mut buf: Vec = Vec::new(); + crate::storage::idx::write_index_entry( + &mut buf, + crate::storage::types::NeedleId(1), + crate::storage::types::Offset::from_actual_offset(0), + crate::storage::types::TOMBSTONE_FILE_SIZE, + ) + .unwrap(); + buf + } else { + vec![0u8; 20] + }; + std::fs::write(dir1.join(format!("{}.ecx", vid_raw)), ecx).unwrap(); + std::fs::write(dir1.join(format!("{}.ecj", vid_raw)), b"").unwrap(); + std::fs::write( + dir1.join(format!("{}.vif", vid_raw)), + serde_json::to_string(&ec_vif(dir1_encode_ts_ns)).unwrap(), + ) + .unwrap(); + + if bitrot_sidecar != SidecarPlacement::Nowhere { + use crate::pb::volume_server_pb::{ + ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums, + }; + use crate::storage::erasure_coding::ec_bitrot; + let prot = EcBitrotProtection { + algorithm: ChecksumAlgorithm::ChecksumCrc32c as i32, + block_size: ec_bitrot::DEFAULT_BITROT_BLOCK_SIZE as u32, + generation: 0, + ec_shard_config: Some(ec_bitrot::ec_shard_config(10, 4, 0)), + shards: (0..14u32) + .map(|shard_id| EcShardChecksums { + shard_id, + covered_size: 4, + block_crc32c: vec![0u8; 4], + }) + .collect(), + encode_uuid: vec![0u8; 16], + }; + // Each runtime resolves its sidecar from its own directory at mount + // (`load_bitrot_for_generation` gets no sibling dirs there), so a + // disk that gets no copy here mounts `BitrotStatus::Off`. + let sidecar_dirs: Vec<&std::path::Path> = match bitrot_sidecar { + SidecarPlacement::Nowhere => Vec::new(), + SidecarPlacement::BothDisks => vec![dir0.as_path(), dir1.as_path()], + SidecarPlacement::Dir1Only => vec![dir1.as_path()], + }; + for d in sidecar_dirs { + let base = crate::storage::volume::volume_file_name( + d.to_str().unwrap(), + "", + VolumeId(vid_raw), + ); + ec_bitrot::save_bitrot_sidecar(&ec_bitrot::bitrot_sidecar_path(&base, 0), &prot) + .unwrap(); + } + } + + let mut store = Store::new(NeedleMapKind::InMemory); + for d in [&dir0, &dir1] { + store + .add_location( + d.to_str().unwrap(), + d.to_str().unwrap(), + 100, + DiskType::HardDrive, + MinFreeSpace::Percent(0.0), + Vec::new(), + ) + .unwrap(); + } + + (split_disk_grpc_service(store), tmp) + } + + /// Wrap a hand-built two-location `Store` in the full + /// `VolumeServerState` the gRPC handlers need. Shared by the split-disk + /// fixtures so a second one costs a store, not another copy of this + /// 50-line literal. + fn split_disk_grpc_service(store: Store) -> VolumeGrpcService { + let state = Arc::new(VolumeServerState { + store: RwLock::new(store), + guard: RwLock::new(Guard::new( + &[], + SigningKey(vec![]), + 0, + SigningKey(vec![]), + 0, + )), + is_stopping: RwLock::new(false), + maintenance: std::sync::atomic::AtomicBool::new(false), + state_version: std::sync::atomic::AtomicU32::new(0), + concurrent_upload_limit: 0, + concurrent_download_limit: 0, + inflight_upload_data_timeout: std::time::Duration::from_secs(60), + inflight_download_data_timeout: std::time::Duration::from_secs(60), + inflight_upload_bytes: std::sync::atomic::AtomicI64::new(0), + inflight_download_bytes: std::sync::atomic::AtomicI64::new(0), + upload_notify: tokio::sync::Notify::new(), + download_notify: tokio::sync::Notify::new(), + data_center: String::new(), + rack: String::new(), + file_size_limit_bytes: 0, + maintenance_byte_per_second: 0, + is_heartbeating: std::sync::atomic::AtomicBool::new(true), + has_master: false, + pre_stop_seconds: 0, + volume_state_notify: tokio::sync::Notify::new(), + write_queue: std::sync::OnceLock::new(), + s3_tier_registry: std::sync::RwLock::new( + crate::remote_storage::s3_tier::S3TierRegistry::new(), + ), + read_mode: crate::config::ReadMode::Local, + allow_untrusted_remote_endpoints: false, + master_url: String::new(), + master_urls: Vec::new(), + seed_master_set: std::collections::HashSet::new(), + current_master_url: tokio::sync::RwLock::new(String::new()), + self_url: String::new(), + http_client: reqwest::Client::new(), + outgoing_http_scheme: "http".to_string(), + outgoing_grpc_tls: None, + metrics_runtime: std::sync::RwLock::new( + crate::server::volume_server::RuntimeMetricsConfig::default(), + ), + metrics_notify: tokio::sync::Notify::new(), + fix_jpg_orientation: false, + has_slow_read: false, + read_buffer_size_bytes: 1024 * 1024, + security_file: String::new(), + cli_white_list: vec![], + state_file_path: String::new(), + }); + + VolumeGrpcService { state } + } + + /// The dedupe lives on the implicit (empty `volume_ids`) path of + /// `scrub_ec_volume` itself -- `scrub_ec_volumes` is handed an + /// already-deduped list by every other caller in this test module, so it + /// cannot exercise this regression. Before the fix, flat-mapping every + /// location's `ec_volumes()` visited this split-disk vid twice and + /// `total_volumes` counted it twice. + #[tokio::test] + async fn test_scrub_ec_volume_node_wide_dedupes_a_split_disk_volume() { + let (service, _tmp) = make_local_service_with_split_disk_ec_volume(7040); + + let resp = service + .scrub_ec_volume(Request::new(volume_server_pb::ScrubEcVolumeRequest { + mode: 1, + volume_ids: vec![], + force_deleted_needles_check: false, + })) + .await + .unwrap() + .into_inner(); + + assert_eq!( + resp.total_volumes, 1, + "a vid split across two disks must be scrubbed once, not once per disk" + ); + } + + /// Issue one scrub mode against a single vid on a split-disk service. + async fn scrub_split_disk( + service: &VolumeGrpcService, + vid_raw: u32, + mode: i32, + ) -> volume_server_pb::ScrubEcVolumeResponse { + service + .scrub_ec_volume(Request::new(volume_server_pb::ScrubEcVolumeRequest { + mode, + volume_ids: vec![vid_raw], + force_deleted_needles_check: false, + })) + .await + .unwrap() + .into_inner() + } + + /// FULL/READS resolves shard locations up front, and with no master + /// configured that lookup fails and short-circuits the whole scrub with an + /// error of its own. Seed the cache so the needle walk actually runs and the + /// response reflects the volume rather than the missing master. + fn seed_all_shard_locations(service: &VolumeGrpcService, vid_raw: u32) { + let store = service.state.store.read().unwrap(); + for ecv in store.find_all_ec_volumes(VolumeId(vid_raw)) { + let mut locs = ecv.shard_locations.write().unwrap(); + for sid in 0u8..14 { + locs.insert(sid, vec!["127.0.0.1:255.1".to_string()]); + } + *ecv.shard_locations_refresh_time.lock().unwrap() = Some(std::time::Instant::now()); + } + } + + /// A disk the identity fence excluded is a disk the scrub did not read, + /// and the mode 2|5 arm must say so. `merge_ec_runtimes` hands the + /// exclusions back as `skipped`; the arm folds them into `errs`, which + /// flags the volume broken and prints the line. Before that, a volume + /// whose disks disagreed on encode identity scrubbed the anchor and + /// reported CLEAN -- the sibling disk went unmentioned. + /// + /// Both halves of the TRANSITION are asserted here: the same layout with + /// the disks AGREEING is clean, so the clean-to-broken change is pinned, + /// not just the message text. + /// + /// Across modes, `BitrotStatus::Off` is now the ONLY place an exclusion + /// goes unreported: modes 2|5 and 3 always report it, and mode 4 reports it + /// on `On` and (since `ec: report fenced-out disks on a malformed sidecar + /// too`) on `Invalid`. `Off` returns `(0, [], [])` before reporting + /// `skipped` because that is a hard Go-parity contract + /// (`case BitrotOff: return 0, nil, nil`), so on an unprotected generation + /// a CHECKSUM scrub of a fenced volume is clean and the excluded disk goes + /// unmentioned. That single exception is deliberate, not an oversight. + #[tokio::test] + async fn test_scrub_ec_volume_full_reports_a_fenced_out_disk() { + // Control: identical layout, both disks on encode run 200. + let (agreeing, _tmp_a) = SplitDiskEcFixture { + dir0_encode_ts_ns: None, // no dir0 .vif => falls through to dir1's + dir1_encode_ts_ns: 200, + clean_index: true, + ..SplitDiskEcFixture::new(7050) + } + .build(); + seed_all_shard_locations(&agreeing, 7050); + { + let store = agreeing.state.store.read().unwrap(); + let gens: Vec = store + .find_all_ec_volumes(VolumeId(7050)) + .iter() + .map(|v| v.encode_ts_ns) + .collect(); + assert_eq!(gens, vec![200, 200], "control fixture must NOT be fenced"); + } + + // The fenced volume: dir0 carries its own, older encode identity. + let (fenced, _tmp_f) = SplitDiskEcFixture { + dir0_encode_ts_ns: Some(100), + dir1_encode_ts_ns: 200, + clean_index: true, + ..SplitDiskEcFixture::new(7051) + } + .build(); + seed_all_shard_locations(&fenced, 7051); + { + let store = fenced.state.store.read().unwrap(); + let gens: Vec = store + .find_all_ec_volumes(VolumeId(7051)) + .iter() + .map(|v| v.encode_ts_ns) + .collect(); + assert_eq!( + gens, + vec![100, 200], + "fixture must mount both disks with DISAGREEING encode identities \ + for this to be meaningful" + ); + } + + // Both FULL (2) and READS (5) share the arm, so both must report it. + for mode in [2, 5] { + let clean = scrub_split_disk(&agreeing, 7050, mode).await; + assert!( + clean.broken_volume_ids.is_empty() && clean.details.is_empty(), + "mode {mode}: agreeing disks must scrub clean, else the fenced \ + case below proves nothing: {:?} {:?}", + clean.broken_volume_ids, + clean.details + ); + + let resp = scrub_split_disk(&fenced, 7051, mode).await; + assert_eq!( + resp.broken_volume_ids, + vec![7051], + "mode {mode}: a fenced-out disk must flag the volume broken, not \ + pass silently: {:?}", + resp.details + ); + assert_eq!( + resp.details.len(), + 1, + "mode {mode}: the exclusion is the only finding: {:?}", + resp.details + ); + assert!( + resp.details[0].starts_with("ecvol 7051: ") + && resp.details[0].contains("belong to encode run 100") + && resp.details[0].contains("they were not verified"), + "mode {mode}: {:?}", + resp.details + ); + } + } + + /// LOCAL and CHECKSUM must verify the shards on EVERY disk holding the vid, + /// not the ones on whichever disk `find_ec_volume` happened to return. The + /// layout makes that observable: disk 0 -- the disk the singular lookup + /// returns -- holds only shard 5, while shard 0 (the one the volume's single + /// needle spans, and the one the checksum sidecar is checked against) lives + /// on disk 1. Built from disk 0 alone, both scrubs simply never look at + /// shard 0. + #[tokio::test] + async fn test_scrub_ec_volume_local_and_checksum_reach_the_sibling_disk() { + // LOCAL reads only the shards the volume's one needle spans, so shard 5 + // is never touched; CHECKSUM reads every shard it holds, so both appear. + for (vid_raw, mode, bitrot_sidecar, want_broken) in [ + (7052u32, 3i32, SidecarPlacement::Nowhere, &[0u32][..]), + (7053, 4, SidecarPlacement::BothDisks, &[0, 5][..]), + ] { + let (service, _tmp) = SplitDiskEcFixture { + dir0_shard_id: 5, + dir1_shard_id: 0, + bitrot_sidecar, + ..SplitDiskEcFixture::new(vid_raw) + } + .build(); + + { + let store = service.state.store.read().unwrap(); + let first = store + .find_ec_volume(VolumeId(vid_raw)) + .expect("the singular lookup must still resolve the vid"); + assert!( + !first.has_shard(0) && first.has_shard(5), + "mode {mode}: the first-disk runtime must NOT hold shard 0, or a \ + first-disk-only scrub would reach it anyway and this proves nothing" + ); + assert!( + store + .find_all_ec_volumes(VolumeId(vid_raw)) + .iter() + .any(|v| v.has_shard(0)), + "mode {mode}: fixture must mount shard 0 on the sibling disk" + ); + } + + let resp = scrub_split_disk(&service, vid_raw, mode).await; + let mut broken: Vec = resp.broken_shard_infos.iter().map(|s| s.shard_id).collect(); + broken.sort_unstable(); + assert!( + broken.contains(&0), + "mode {mode}: shard 0 lives on the sibling disk and was never \ + verified: shards={broken:?} details={:?}", + resp.details + ); + assert_eq!( + broken, want_broken, + "mode {mode}: details={:?}", + resp.details + ); + assert_eq!(resp.broken_volume_ids, vec![vid_raw]); + } + } + + /// CHECKSUM must not source bitrot protection from the ANCHOR alone. + /// + /// The `.ecsum` sidecar is deliberately not mirrored across disks, and at + /// mount `EcVolume::new` resolves it with no sibling directories -- only + /// `VolumeEcShardsMount` ever passes `ec_metadata_dirs()`. So after any + /// volume-server restart the split-disk runtime that does not hold the + /// sidecar mounts `BitrotStatus::Off`. The anchor is the first + /// shard-bearing runtime at the maximum `encode_ts_ns`, picked with no + /// regard for which disk holds the sidecar -- so whenever the one copy + /// lands on the non-anchor disk (about half of all mirrored split-disk + /// layouts), taking `(prot, status)` from the anchor made `run()` return + /// `(0, [], [])` and the WHOLE volume scrubbed clean, silently. That is the + /// exact failure this branch exists to remove. + /// + /// The fixture is the same one the sibling-disk test uses, with the sidecar + /// written to dir1 ONLY. Both disks agree on encode identity, so nothing is + /// fenced and the plan's `unverifiable_sidecar` early return cannot fire -- + /// the volume must actually be SCANNED, and the placeholder shard bytes do + /// not match the sidecar, so every shard the merge reaches is reported. + #[tokio::test] + async fn test_checksum_scrub_takes_protection_from_the_disk_that_has_the_sidecar() { + let (service, _tmp) = SplitDiskEcFixture { + dir0_shard_id: 5, + dir1_shard_id: 0, + bitrot_sidecar: SidecarPlacement::Dir1Only, + ..SplitDiskEcFixture::new(7054) + } + .build(); + + // Without this the test could pass for the wrong reason: if the anchor + // ever resolved the sibling's sidecar itself, sourcing from the anchor + // would be fine and this would prove nothing. + { + use crate::storage::erasure_coding::ec_bitrot::BitrotStatus; + let store = service.state.store.read().unwrap(); + let runtimes = store.find_all_ec_volumes(VolumeId(7054)); + assert_eq!( + runtimes.len(), + 2, + "fixture must mount the vid on both disks" + ); + assert_eq!( + runtimes[0].bitrot_status, + BitrotStatus::Off, + "the ANCHOR disk must have NO sidecar, or this proves nothing" + ); + assert_eq!( + runtimes[1].bitrot_status, + BitrotStatus::On, + "the sibling disk must be the one carrying protection" + ); + assert_eq!( + runtimes[0].encode_ts_ns, runtimes[1].encode_ts_ns, + "the disks must AGREE on encode identity, so nothing is fenced \ + and the unverifiable-sidecar early return cannot mask the scan" + ); + } + + let resp = scrub_split_disk(&service, 7054, 4).await; + let mut broken: Vec = resp.broken_shard_infos.iter().map(|s| s.shard_id).collect(); + broken.sort_unstable(); + assert_eq!( + broken, + vec![0, 5], + "the volume must be SCANNED against the sibling's sidecar, not \ + returned clean-empty because the anchor mounted BitrotStatus::Off: \ + details={:?}", + resp.details + ); + assert_eq!( + resp.broken_volume_ids, + vec![7054], + "details={:?}", + resp.details + ); + } + + /// A two-location store holding a REAL 10+4 encoded volume with EVERY data + /// and parity shard present, scattered across both disks: shards 0..=6 on + /// disk 0, shards 7..=13 plus the `.ecx`/`.ecj`/`.vif` on disk 1. Disk 0 + /// gets no index files, so its shards mount only through the cross-disk + /// reconcile -- the real split-disk shape. + /// + /// This is the only layout that reaches mode 2's local Reed-Solomon parity + /// check over a genuinely MULTI-DIRECTORY `dirs`. The check is gated on + /// `all_local`, so every shard has to be present; the existing all-local + /// fixture keeps them in one directory, where every entry of `dirs` is the + /// same string and a permutation or off-by-one in the `slots` -> `dirs` + /// mapping is invisible. + /// + /// The `.dat`/`.idx` are left in a third directory that is NOT a store + /// location, so `prune_incomplete_ec_with_sibling_dat` finds no sibling + /// `.dat` and cannot delete the EC artefacts out from under the fixture. + fn make_service_with_two_disk_complete_ec_volume(vid_raw: u32) -> (VolumeGrpcService, TempDir) { + let tmp = TempDir::new().unwrap(); + let src = tmp.path().join("src"); + let dir0 = tmp.path().join("data0"); + let dir1 = tmp.path().join("data1"); + for d in [&src, &dir0, &dir1] { + std::fs::create_dir_all(d).unwrap(); + } + let src_s = src.to_str().unwrap(); + let vid = VolumeId(vid_raw); + + // A real volume, really encoded: a parity check needs genuine + // Reed-Solomon parity to agree with, or "no corruption" means nothing. + { + let mut v = crate::storage::volume::Volume::new( + src_s, + src_s, + "", + vid, + NeedleMapKind::InMemory, + None, + None, + 0, + crate::storage::types::Version::current(), + ) + .unwrap(); + for i in 1..=8u64 { + let data = format!("test data for needle {} with a bit more length", i); + let mut n = crate::storage::needle::Needle { + id: crate::storage::types::NeedleId(i), + cookie: crate::storage::types::Cookie(i as u32), + data: data.as_bytes().to_vec(), + data_size: data.len() as u32, + ..Default::default() + }; + v.write_needle(&mut n, true, false).unwrap(); + } + v.sync_to_disk().unwrap(); + v.close(); + } + crate::storage::erasure_coding::ec_encoder::write_ec_files(src_s, src_s, "", vid, 10, 4) + .unwrap(); + + for id in 0..14u8 { + let target = if id < 7 { &dir0 } else { &dir1 }; + std::fs::rename( + format!("{}/{}.ec{:02}", src_s, vid_raw, id), + format!("{}/{}.ec{:02}", target.to_str().unwrap(), vid_raw, id), + ) + .unwrap(); + } + let idx_dir = dir1.to_str().unwrap(); + std::fs::rename( + format!("{}/{}.ecx", src_s, vid_raw), + format!("{}/{}.ecx", idx_dir, vid_raw), + ) + .unwrap(); + std::fs::write(format!("{}/{}.ecj", idx_dir, vid_raw), b"").unwrap(); + std::fs::write( + format!("{}/{}.vif", idx_dir, vid_raw), + serde_json::to_string(&crate::storage::volume::VifVolumeInfo { + version: 3, + ec_shard_config: Some(crate::storage::volume::VifEcShardConfig { + data_shards: 10, + parity_shards: 4, + ..Default::default() + }), + ..Default::default() + }) + .unwrap(), + ) + .unwrap(); + + let mut store = Store::new(NeedleMapKind::InMemory); + for d in [&dir0, &dir1] { + store + .add_location( + d.to_str().unwrap(), + d.to_str().unwrap(), + 100, + DiskType::HardDrive, + MinFreeSpace::Percent(0.0), + Vec::new(), + ) + .unwrap(); + } + + (split_disk_grpc_service(store), tmp) + } + + /// The branch's newest production behavior, end to end: a mode 2 scrub of a + /// SPLIT-DISK volume whose shards are all local but spread over two + /// directories. + /// + /// `all_local` is now computed from the merged view, so this layout reaches + /// the local Reed-Solomon parity check for the first time -- and + /// `verify_ec_shards` is handed a `dirs` vector whose entries differ. + /// `test_scrub_ec_volume_reads_mode_reconstructs_missing_shard` also reaches + /// `all_local == true`, but every entry of its `dirs` is the same string, so + /// a permutation or off-by-one in the `slots` -> `dirs` mapping (the mode + /// 2|5 arm) cannot be observed there; and + /// `test_verify_ec_shards_reads_shards_from_multiple_dirs` builds its `dirs` + /// by hand and never goes through `merge_ec_runtimes` at all. + #[tokio::test] + async fn test_scrub_ec_volume_full_parity_checks_a_two_disk_all_local_volume() { + let (service, tmp) = make_service_with_two_disk_complete_ec_volume(7060); + seed_all_shard_locations(&service, 7060); + let dir0 = tmp.path().join("data0").to_str().unwrap().to_string(); + let dir1 = tmp.path().join("data1").to_str().unwrap().to_string(); + + // The fixture must actually produce the shape under test: every shard + // mounted, and the mapping the mode 2|5 arm builds `dirs` from spanning + // BOTH directories. Without this the test could pass over a one-disk + // layout and prove nothing about the mapping. + { + let store = service.state.store.read().unwrap(); + let runtimes = store.find_all_ec_volumes(VolumeId(7060)); + assert_eq!(runtimes.len(), 2, "the vid must mount on both disks"); + let merged = crate::storage::erasure_coding::ec_volume::merge_ec_runtimes(&runtimes) + .expect("two runtimes merge"); + assert!(merged.skipped.is_empty(), "{:?}", merged.skipped); + let dirs: Vec = (0..14) + .map(|id| { + merged.slots[id] + .unwrap_or_else(|| { + panic!( + "shard {id} is not mounted; all_local would be false \ + and the parity check would never run" + ) + }) + .0 + .dir + .clone() + }) + .collect(); + assert_eq!(dirs[0], dir0); + assert_eq!(dirs[6], dir0); + assert_eq!(dirs[7], dir1); + assert_eq!(dirs[13], dir1); + } + + // Real parity over a real encode, read through two directories: nothing + // may be fabricated as corrupt. + let resp = scrub_split_disk(&service, 7060, 2).await; + assert!( + resp.broken_volume_ids.is_empty() && resp.broken_shard_infos.is_empty(), + "an intact two-disk volume must not be reported broken: {:?}", + resp.details + ); + assert!(resp.details.is_empty(), "{:?}", resp.details); + assert_eq!(resp.total_files, 8, "the needle walk must have run"); + + // ...and the parity check really RAN, over the SECOND disk's shards. + // Shard 13 is a PARITY shard: the per-needle walk reads only live + // data-shard intervals, so nothing but `verify_ec_shards` can see this, + // and nothing but a correct `slots` -> `dirs` mapping can find the file. + // + // `assert_eq!` rather than `contains`, and the reason is specific: + // `dirs` is indexed BY SHARD ID and carries only directories, so an + // off-by-one or a permutation that lands the wrong DIRECTORY at some + // index makes `verify_ec_shards` look for a shard file that is not + // there -- the reported set is then not exactly `[13]`, which + // `contains(&13)` would still accept. A permutation that stays WITHIN + // one disk is not caught here: both entries name the same directory, so + // it is unobservable through `dirs` at all. That is a limit of this + // fixture, stated rather than papered over. + let shard13 = format!("{}/7060.ec13", dir1); + let mut bytes = std::fs::read(&shard13).unwrap(); + bytes[0] ^= 0xFF; + std::fs::write(&shard13, &bytes).unwrap(); + + let resp = scrub_split_disk(&service, 7060, 2).await; + let broken: Vec = resp.broken_shard_infos.iter().map(|s| s.shard_id).collect(); + assert_eq!( + broken, + vec![13], + "the parity check must reach disk 1 and blame exactly shard 13: {:?}", + resp.details + ); + assert_eq!(resp.broken_volume_ids, vec![7060]); + assert!( + resp.details + .iter() + .any(|d| d.contains("parity mismatch on shard 13")), + "the finding must come from the parity check, not the needle walk: {:?}", + resp.details + ); + } + #[tokio::test] async fn volume_delete_reports_absent_volume_as_not_found() { let (service, _tmp) = make_service_with_seed_masters(&[]); @@ -7421,7 +8240,13 @@ mod tests { assert_eq!(err.code(), tonic::Code::FailedPrecondition, "{err:?}"); assert!(err.message().contains("volume not empty"), "{err:?}"); assert!( - service.state.store.read().unwrap().find_volume(VolumeId(1)).is_some(), + service + .state + .store + .read() + .unwrap() + .find_volume(VolumeId(1)) + .is_some(), "refused delete must leave the volume mounted" ); } diff --git a/seaweed-volume/src/server/store_ec.rs b/seaweed-volume/src/server/store_ec.rs index 7bf123470..83f8330e5 100644 --- a/seaweed-volume/src/server/store_ec.rs +++ b/seaweed-volume/src/server/store_ec.rs @@ -134,8 +134,13 @@ pub async fn read_ec_shard_needle_distributed( Ok(fresh) => { // A complete reply merges into the cache; an incomplete one // (< data_shards) is left unwritten — keep the prior cache. - match write_back_shard_locations(state, vid, fresh, snapshot.data_shards as usize) - { + match write_back_shard_locations( + state, + vid, + fresh, + snapshot.data_shards as usize, + snapshot.encode_ts_ns, + ) { Some(merged) => shard_locations = merged, // An incomplete reply leaves the cache unwritten and its refresh // time unadvanced, so the mark this refresh consumed goes back. @@ -168,36 +173,37 @@ pub async fn read_ec_shard_needle_distributed( let parity_shards = snapshot.parity_shards as usize; let encode_ts_ns = snapshot.encode_ts_ns; let intervals = std::mem::take(&mut snapshot.intervals); - let fetched: Vec, bool)>> = stream::iter(intervals.into_iter().map(|res| { - let shard_locations = &shard_locations; - async move { - match res { - IntervalResult::Local(buf) => Ok((buf, false)), - IntervalResult::NeedRemote { - shard_id, - shard_offset, - size, - } => { - fetch_one_interval( - state, - vid, - needle_id, + let fetched: Vec, bool)>> = + stream::iter(intervals.into_iter().map(|res| { + let shard_locations = &shard_locations; + async move { + match res { + IntervalResult::Local(buf) => Ok((buf, false)), + IntervalResult::NeedRemote { shard_id, shard_offset, size, - shard_locations, - data_shards, - parity_shards, - encode_ts_ns, - ) - .await + } => { + fetch_one_interval( + state, + vid, + needle_id, + shard_id, + shard_offset, + size, + shard_locations, + data_shards, + parity_shards, + encode_ts_ns, + ) + .await + } } } - } - })) - .buffered(INTERVAL_READ_CONCURRENCY) - .collect() - .await; + })) + .buffered(INTERVAL_READ_CONCURRENCY) + .collect() + .await; let mut assembled: Vec> = Vec::with_capacity(fetched.len()); for res in fetched { @@ -256,16 +262,16 @@ pub async fn read_ec_shard_needle_distributed( pub async fn scrub_ec_volume_distributed( state: &Arc, vid: VolumeId, + expected_encode_ts_ns: i64, force_deleted_needles_check: bool, recover_unreadable: bool, -) -> (i64, Vec, Vec) { +) -> ( + i64, + Vec, + Vec, +) { // Phase A — under the Store read lock, snapshot the index scrub and grab the // paths/scalars + shard-location staleness; release the lock before any await. - // - // The index walk itself runs AFTER the guard: scrub_index() reads the whole - // .ecx, and doing that under store.read() parks the periodic heartbeat's - // store.write(), which a write-preferring RwLock then makes every later - // reader queue behind. See EcChecksumScrubPlan. let ( ecx_path, collection, @@ -278,7 +284,10 @@ pub async fn scrub_ec_volume_distributed( total_shards, ) = { let store = state.store.read().unwrap(); - let ecv = match store.find_ec_volume(vid) { + // Resolve the runtime matching the anchor's encode generation, not the + // first-match find_ec_volume — otherwise the needle walk can scan an + // older run while the parity half scans the newest. + let ecv = match find_ec_volume_for_scrub(&store, vid, expected_encode_ts_ns) { Some(v) => v, None => { return ( @@ -350,7 +359,7 @@ pub async fn scrub_ec_volume_distributed( "EC volume {} index scrub task panicked: {}", vid.0, e )], - ) + ); } // Cancellation: the runtime is shutting down, so this response is // unlikely to reach anyone. Return clean rather than inventing a @@ -374,7 +383,9 @@ pub async fn scrub_ec_volume_distributed( ) { match cached_lookup_ec_shard_locations(state, vid).await { Ok(fresh) => { - if write_back_shard_locations(state, vid, fresh, data_shards).is_none() { + if write_back_shard_locations(state, vid, fresh, data_shards, expected_encode_ts_ns) + .is_none() + { mark_shard_locations_stale(state, vid); return ( 0, @@ -401,7 +412,7 @@ pub async fn scrub_ec_volume_distributed( // walk, so per-needle snapshots no longer clone it. let locations: HashMap> = { let store = state.store.read().unwrap(); - let ecv = match store.find_ec_volume(vid) { + let ecv = match find_ec_volume_for_scrub(&store, vid, expected_encode_ts_ns) { Some(v) => v, None => { return ( @@ -424,54 +435,57 @@ pub async fn scrub_ec_volume_distributed( // `walk_index_file` reads the full .ecx synchronously, so run it in the // blocking pool rather than on this async worker — same reason as // `index_plan.run()` above. - let (count, needles, walk_errs) = - match tokio::task::spawn_blocking(move || -> (i64, Vec<(NeedleId, Offset, Size)>, Vec) { + let (count, needles, walk_errs) = match tokio::task::spawn_blocking( + move || -> (i64, Vec<(NeedleId, Offset, Size)>, Vec) { let mut count: i64 = 0; let mut needles: Vec<(NeedleId, Offset, Size)> = Vec::new(); let mut walk_errs: Vec = Vec::new(); match ecx_walk { Ok(mut f) => { - if let Err(e) = crate::storage::idx::walk_index_file(&mut f, 0, |id, offset, size| { - count += 1; - // Skip ALL deleted entries: -1 tombstones (runtime delete folded - // into .ecx) and -originalSize entries (a needle deleted on the - // regular volume before EC encode). get_actual_size uses the raw - // signed size, so a negative would yield empty intervals - // (false-positive) or an under-16-byte buffer (parse panic). - if !size.is_deleted() { - needles.push((id, offset, size)); - } - Ok(()) - }) { + if let Err(e) = + crate::storage::idx::walk_index_file(&mut f, 0, |id, offset, size| { + count += 1; + // Skip ALL deleted entries: -1 tombstones (runtime delete folded + // into .ecx) and -originalSize entries (a needle deleted on the + // regular volume before EC encode). get_actual_size uses the raw + // signed size, so a negative would yield empty intervals + // (false-positive) or an under-16-byte buffer (parse panic). + if !size.is_deleted() { + needles.push((id, offset, size)); + } + Ok(()) + }) + { walk_errs.push(format!("walk ECX file {}: {}", ecx_path, e)); } } Err(e) => walk_errs.push(format!("open ECX file {}: {}", ecx_path, e)), } (count, needles, walk_errs) - }).await { - Ok(v) => v, - Err(e) => { - // A panic is evidence about the volume and counts as broken; a - // cancellation is not — see the index_plan join above for the - // same reasoning. - if e.is_panic() { - return ( - 0, - Vec::new(), - vec![format!( - "EC volume {} ecx walk task panicked: {}", - vid.0, e - )], - ) - } - return (0, Vec::new(), Vec::new()); + }, + ) + .await + { + Ok(v) => v, + Err(e) => { + // A panic is evidence about the volume and counts as broken; a + // cancellation is not — see the index_plan join above for the + // same reasoning. + if e.is_panic() { + return ( + 0, + Vec::new(), + vec![format!("EC volume {} ecx walk task panicked: {}", vid.0, e)], + ); } - }; + return (0, Vec::new(), Vec::new()); + } + }; errs.extend(walk_errs); // reads for EC chunks can hit the same shard repeatedly, so dedupe broken shards - let mut broken_shards: HashMap = HashMap::new(); + let mut broken_shards: HashMap = + HashMap::new(); for (id, offset, size) in needles { // Per-needle snapshot under the lock from the RAW .ecx (offset, size) so @@ -606,7 +620,11 @@ pub async fn scrub_ec_volume_distributed( // Mirror Go CmpEcShardInfo: sort by (volume_id, shard_id). let mut broken: Vec = broken_shards.into_values().collect(); - broken.sort_by(|a, b| a.volume_id.cmp(&b.volume_id).then(a.shard_id.cmp(&b.shard_id))); + broken.sort_by(|a, b| { + a.volume_id + .cmp(&b.volume_id) + .then(a.shard_id.cmp(&b.shard_id)) + }); (count, broken, errs) } @@ -647,7 +665,7 @@ fn scrub_snapshot_under_lock( expected_encode_ts: i64, ) -> io::Result { let store = state.store.read().unwrap(); - let ecv = match store.find_ec_volume(vid) { + let ecv = match find_ec_volume_for_scrub(&store, vid, expected_encode_ts) { Some(v) => v, // Volume unmounted mid-scan: a distinct NotFound so the caller aborts // with an error rather than silently skipping (which would false-CLEAN). @@ -856,8 +874,8 @@ async fn cached_lookup_ec_shard_locations( )); } - let grpc_addr = parse_grpc_address(&master) - .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; + let grpc_addr = + parse_grpc_address(&master).map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; let endpoint = build_grpc_endpoint(&grpc_addr, state.outgoing_grpc_tls.as_ref()) .map_err(|e| io::Error::new(io::ErrorKind::Other, e.to_string()))?; let channel = endpoint @@ -905,15 +923,34 @@ fn write_back_shard_locations( vid: VolumeId, locations: HashMap>, data_shards: usize, + expected_encode_ts_ns: i64, ) -> Option>> { if locations.len() < data_shards { return None; } let store = state.store.read().unwrap(); - let ecv = store.find_ec_volume(vid)?; + let ecv = find_ec_volume_for_scrub(&store, vid, expected_encode_ts_ns)?; Some(ecv.merge_shard_locations(locations)) } +/// Resolve the runtime matching the scrub's anchor encode generation, not the +/// first-match `find_ec_volume`. When `expected_encode_ts_ns` is 0 (legacy or +/// pre-feature), falls back to first-match so existing behavior is preserved. +fn find_ec_volume_for_scrub<'a>( + store: &'a crate::storage::store::Store, + vid: VolumeId, + expected_encode_ts_ns: i64, +) -> Option<&'a crate::storage::erasure_coding::EcVolume> { + if expected_encode_ts_ns != 0 { + store + .find_all_ec_volumes(vid) + .into_iter() + .find(|v| v.encode_ts_ns == expected_encode_ts_ns) + } else { + store.find_ec_volume(vid) + } +} + /// Build a SeaweedFS-style `host:httpPort.grpcPort` address from a /// master `Location` so the result is what `parse_grpc_address` (and /// the heartbeat path) already understand. @@ -1083,7 +1120,10 @@ async fn do_read_remote_ec_shard_interval( .map_err(|e| { io::Error::new( io::ErrorKind::Other, - format!("volume_ec_shard_read {}.{} from {}: {}", vid.0, shard_id, source, e), + format!( + "volume_ec_shard_read {}.{} from {}: {}", + vid.0, shard_id, source, e + ), ) })?; let mut stream = resp.into_inner(); @@ -1154,12 +1194,8 @@ async fn recover_one_remote_ec_shard_interval( expected_encode_ts_ns: i64, ) -> io::Result<(Vec, bool)> { let total_shards = data_shards + parity_shards; - let rs = ReedSolomon::new(data_shards, parity_shards).map_err(|e| { - io::Error::new( - io::ErrorKind::Other, - format!("reed-solomon init: {:?}", e), - ) - })?; + let rs = ReedSolomon::new(data_shards, parity_shards) + .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("reed-solomon init: {:?}", e)))?; // Charge the buffers this recovery is about to hold against the budget, so a // burst of them queues here rather than on the heap. An interval whose @@ -1201,12 +1237,20 @@ async fn recover_one_remote_ec_shard_interval( // lenient only when the caller carries no identity (pre-upgrade). // Mirrors Go's `readLocalEcShardInterval`. let owner = match store.find_ec_volume_with_shard(vid, sid as u32) { - Some(ecv) if expected_encode_ts_ns == 0 || ecv.encode_ts_ns == expected_encode_ts_ns => ecv, + Some(ecv) + if expected_encode_ts_ns == 0 || ecv.encode_ts_ns == expected_encode_ts_ns => + { + ecv + } _ => continue, }; if let Some(Some(shard)) = owner.shards.get(sid) { let mut buf = vec![0u8; size]; - if shard.read_at(&mut buf, shard_offset as u64).map(|n| n == size).unwrap_or(false) { + if shard + .read_at(&mut buf, shard_offset as u64) + .map(|n| n == size) + .unwrap_or(false) + { bufs[sid] = Some(buf); available += 1; } @@ -1496,7 +1540,11 @@ async fn fetch_ec_index_from_one_peer( let _ = fs::remove_file(ecx_path); return Err(io::Error::new( io::ErrorKind::Other, - format!("peer {} served an unusable .ecx (size {})", peer, meta.len()), + format!( + "peer {} served an unusable .ecx (size {})", + peer, + meta.len() + ), )); } @@ -1534,7 +1582,10 @@ async fn drain_copy_stream( ) -> io::Result<()> { use std::io::Write; let mut file = if append { - fs::OpenOptions::new().create(true).append(true).open(dest_path) + fs::OpenOptions::new() + .create(true) + .append(true) + .open(dest_path) } else { fs::File::create(dest_path) } @@ -1544,8 +1595,9 @@ async fn drain_copy_stream( .await .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("recv {}: {}", dest_path, e)))? { - file.write_all(&chunk.file_content) - .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("write {}: {}", dest_path, e)))?; + file.write_all(&chunk.file_content).map_err(|e| { + io::Error::new(io::ErrorKind::Other, format!("write {}: {}", dest_path, e)) + })?; } Ok(()) } diff --git a/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs b/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs index 1470e85de..6a5f15e8f 100644 --- a/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs +++ b/seaweed-volume/src/storage/erasure_coding/ec_encoder.rs @@ -284,8 +284,12 @@ pub fn rebuild_ec_files( /// FULL walk only reads live data-shard intervals, so on its own it can't catch /// bitrot in a parity shard or an unwalked region. Move to mode 4 (CHECKSUM) and /// drop it from mode 2 once the `.ecsum` subsystem lands. +/// +/// `dirs` is indexed BY SHARD ID: each entry is the directory holding that +/// shard, or `None` when no disk mounts it. A reconciled volume's shards can be +/// split across disks, so a single directory cannot address them all. pub fn verify_ec_shards( - dir: &str, + dirs: &[Option], collection: &str, volume_id: VolumeId, data_shards: usize, @@ -295,29 +299,48 @@ pub fn verify_ec_shards( .map_err(|e| io::Error::new(io::ErrorKind::Other, format!("reed-solomon init: {:?}", e)))?; let total_shards = data_shards + parity_shards; - let mut shards: Vec = (0..total_shards as u8) - .map(|i| EcVolumeShard::new(dir, collection, volume_id, i)) + let mut shards: Vec> = (0..total_shards) + .map(|i| { + dirs.get(i) + .and_then(|d| d.as_ref()) + .map(|d| EcVolumeShard::new(d, collection, volume_id, i as u8)) + }) .collect(); let mut shard_size = 0; let mut broken_shards = std::collections::HashSet::new(); let mut details = Vec::new(); - for (i, shard) in shards.iter_mut().enumerate() { - if let Ok(_) = shard.open() { - let size = shard.file_size(); - if size > shard_size { - shard_size = size; + for (i, slot) in shards.iter_mut().enumerate() { + match slot.as_mut() { + // Not a match guard: a binding is immutable until the guard ends, + // and `open()` needs `&mut self`. + Some(shard) => { + if shard.open().is_ok() { + let size = shard.file_size(); + if size > shard_size { + shard_size = size; + } + } else { + broken_shards.insert(i as u32); + details.push(format!("failed to open or missing shard {}", i)); + } + } + None => { + broken_shards.insert(i as u32); + details.push(format!("shard {} is not mounted on any disk", i)); } - } else { - broken_shards.insert(i as u32); - details.push(format!("failed to open or missing shard {}", i)); } } if shard_size == 0 || broken_shards.len() >= parity_shards { - // Can't do much if we don't know the size or have too many missing - return Ok((broken_shards.into_iter().collect(), details)); + // Can't do much if we don't know the size or have too many missing. + // Sort like the normal path below: a `HashSet` iteration order would + // make this return shard ids in an arbitrary order, and enough `None` + // entries in `dirs` now reach this branch for a caller to notice. + let mut broken_vec: Vec = broken_shards.into_iter().collect(); + broken_vec.sort_unstable(); + return Ok((broken_vec, details)); } let block_size = ERASURE_CODING_SMALL_BLOCK_SIZE; @@ -331,7 +354,17 @@ pub fn verify_ec_shards( let mut read_failed = false; for i in 0..total_shards { if !broken_shards.contains(&(i as u32)) { - if let Err(e) = shards[i].read_at(&mut buffers[i], offset) { + // The `None` arm is defensive and unreachable: the open loop + // put every unmounted slot in `broken_shards`, which this + // branch already skipped. Kept because the `Option` forces + // some handling here, and an error is the only shape that + // cannot quietly feed an unread buffer into the parity + // comparison below. Nothing needs to cover it. + let read = match shards[i].as_mut() { + Some(shard) => shard.read_at(&mut buffers[i], offset), + None => Err(io::Error::new(io::ErrorKind::NotFound, "shard not mounted")), + }; + if let Err(e) = read { broken_shards.insert(i as u32); details.push(format!("read error shard {}: {}", i, e)); read_failed = true; @@ -377,7 +410,7 @@ pub fn verify_ec_shards( } // Close all shards - for shard in &mut shards { + for shard in shards.iter_mut().flatten() { shard.close(); } @@ -1457,4 +1490,111 @@ mod tests { "should fail when idx_dir doesn't contain .idx" ); } + + /// Write a real 10+4 encoded volume into `dir`. + /// + /// Unlike `make_volume_with_needles` and `encode_sample_volume` this seeds + /// a caller-chosen directory, which is what a split-disk test needs: the + /// shards have to be scattered out of the directory they were encoded into. + fn seed_encoded_volume(dir: &str, vid: VolumeId) { + let mut v = Volume::new( + dir, + dir, + "", + vid, + NeedleMapKind::InMemory, + None, + None, + 0, + Version::current(), + ) + .unwrap(); + for i in 1..=8 { + let data = format!("test data for needle {} with a bit more length", i); + let mut n = Needle { + id: NeedleId(i), + cookie: Cookie(i as u32), + data: data.as_bytes().to_vec(), + data_size: data.len() as u32, + ..Needle::default() + }; + v.write_needle(&mut n, true, false).unwrap(); + } + v.sync_to_disk().unwrap(); + v.close(); + write_ec_files(dir, dir, "", vid, 10, 4).unwrap(); + } + + /// Shards split across two directories must all be found. Passing one dir + /// per shard is what lets a reconciled volume's parity be checked at all. + #[test] + fn test_verify_ec_shards_reads_shards_from_multiple_dirs() { + let tmp = TempDir::new().unwrap(); + let src = tmp.path().join("src"); + let d0 = tmp.path().join("d0"); + let d1 = tmp.path().join("d1"); + for d in [&src, &d0, &d1] { + std::fs::create_dir_all(d).unwrap(); + } + let src_s = src.to_str().unwrap(); + seed_encoded_volume(src_s, VolumeId(1)); + + // Move shards 0..=6 to d0 and 7..=13 to d1. + let mut dirs: Vec> = Vec::new(); + for id in 0..14u8 { + let target = if id < 7 { &d0 } else { &d1 }; + std::fs::rename( + format!("{}/1.ec{:02}", src_s, id), + format!("{}/1.ec{:02}", target.to_str().unwrap(), id), + ) + .unwrap(); + dirs.push(Some(target.to_str().unwrap().to_string())); + } + + let (broken, details) = verify_ec_shards(&dirs, "", VolumeId(1), 10, 4).unwrap(); + assert!( + broken.is_empty(), + "split-dir shards reported broken: {:?}", + details + ); + } + + /// A shard no disk holds is a missing shard, not a panic and not a silent + /// pass: it is REPORTED, by id, with a message that distinguishes "no disk + /// holds this shard" from "the disk holds it but it won't open". + /// + /// Read the scope literally. This does NOT show that the mounted shards + /// verify clean. `dirs[5] = None` puts shard 5 in `broken_shards` before + /// the block loop starts, so every iteration takes the + /// `else { read_failed = true; }` arm and the Reed-Solomon comparison never + /// runs at all. `broken == vec![5]` therefore holds because the other 13 + /// were never verified, not because they verified clean -- a parity check + /// over intact shards is what + /// `test_verify_ec_shards_reads_shards_from_multiple_dirs` and the + /// end-to-end split-disk FULL scrub establish. + #[test] + fn test_verify_ec_shards_treats_a_none_dir_as_missing() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + seed_encoded_volume(dir, VolumeId(1)); + + let mut dirs: Vec> = (0..14).map(|_| Some(dir.to_string())).collect(); + dirs[5] = None; + + let (broken, details) = verify_ec_shards(&dirs, "", VolumeId(1), 10, 4).unwrap(); + assert_eq!( + broken, + vec![5], + "an unmounted shard must be reported, and only it: {:?}", + details + ); + // "no disk holds this shard" and "the disk holds it but it won't open" + // are different operator problems, which is why they carry different + // messages. Asserting only the id would let one masquerade as the other. + assert!( + details.iter().any(|d| d.contains("not mounted")), + "an unmounted shard must be distinguished from an unopenable one, got {:?}", + details + ); + } } diff --git a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs index 19853caec..595ae2328 100644 --- a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs +++ b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs @@ -81,6 +81,15 @@ pub struct EcVolume { /// so `bitrot_protection()` can return the `Off`/`Invalid` distinction without /// re-reading, mirroring Go's `EcVolume.bitrotStatus`. pub(crate) bitrot_status: crate::storage::erasure_coding::ec_bitrot::BitrotStatus, + /// Directory the active sidecar was actually resolved from. The search in + /// `load_bitrot_for_generation` spans sibling disks, and a `.ecsum` records + /// no encode identity, so provenance is the only signal that a loaded + /// manifest may describe a different encode run. Same purpose as + /// `ecx_actual_dir`. Empty when no sidecar was found. Always committed in + /// the same step as `bitrot`/`bitrot_status`, so a reload whose `Err` gets + /// swallowed by `reload_bitrot_sidecar` cannot leave this describing a + /// sidecar other than the one those two fields actually hold. + pub(crate) bitrot_source_dir: String, io_error_count: std::sync::atomic::AtomicI32, io_error_quarantined: std::sync::atomic::AtomicBool, @@ -249,9 +258,8 @@ pub fn ec_shard_config_from( let default_ds = crate::storage::erasure_coding::ec_shard::DATA_SHARDS_COUNT as u32; let default_ps = crate::storage::erasure_coding::ec_shard::PARITY_SHARDS_COUNT as u32; // Sum as u64: counts near the u32 ceiling would wrap and pass the bound. - let usable = |ds: u32, ps: u32| { - ds > 0 && ps > 0 && (ds as u64 + ps as u64) <= MAX_SHARD_COUNT as u64 - }; + let usable = + |ds: u32, ps: u32| ds > 0 && ps > 0 && (ds as u64 + ps as u64) <= MAX_SHARD_COUNT as u64; if let Some(ec) = vif.and_then(|v| v.ec_shard_config.as_ref()) { // A config that is PRESENT but records an impossible ratio is not a @@ -308,7 +316,10 @@ pub fn ec_shard_config_from( if prot.generation != 0 { return Err(io::Error::new( io::ErrorKind::InvalidData, - format!("{} records generation {}, not generation 0", path, prot.generation), + format!( + "{} records generation {}, not generation 0", + path, prot.generation + ), )); } let ec = prot.ec_shard_config.as_ref().ok_or_else(|| { @@ -436,6 +447,7 @@ impl EcVolume { encode_ts_ns, bitrot: None, bitrot_status: crate::storage::erasure_coding::ec_bitrot::BitrotStatus::Off, + bitrot_source_dir: String::new(), io_error_count: std::sync::atomic::AtomicI32::new(0), io_error_quarantined: std::sync::atomic::AtomicBool::new(false), last_io_error: std::sync::Mutex::new(None), @@ -522,7 +534,11 @@ impl EcVolume { /// => `Off` (protection off, not corruption); self-integrity or manifest /// failure => `Invalid` with a warning (protection off pending repair); usable /// => `On`. Mirrors Go's `loadBitrotForGeneration`. - fn load_bitrot_for_generation(&mut self, generation: u32, additional_dirs: &[String]) -> io::Result<()> { + fn load_bitrot_for_generation( + &mut self, + generation: u32, + additional_dirs: &[String], + ) -> io::Result<()> { use crate::storage::erasure_coding::ec_bitrot; // Data base then index base, matching Go's findBitrotSidecar. A split // -dir/-dir.idx location keeps the sidecar with the INDEX, and on a @@ -582,6 +598,22 @@ impl EcVolume { self.data_shards as usize, self.parity_shards as usize, ); + // Provenance is committed together with the sidecar it describes, so a + // swallowed reload (`reload_bitrot_sidecar` logs and discards the `Err` + // above) cannot leave them disagreeing: the geometry-mismatch return + // happens before this point, so `bitrot`/`bitrot_status` below and + // `bitrot_source_dir` here always describe the same load attempt. + // Only a path that actually exists is a real source: `path` falls back + // to the data path when nothing was found, and recording that would + // claim a provenance the volume does not have. + self.bitrot_source_dir = if std::path::Path::new(&path).exists() { + std::path::Path::new(&path) + .parent() + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_default() + } else { + String::new() + }; self.bitrot = None; self.bitrot_status = status; match status { @@ -629,31 +661,19 @@ impl EcVolume { /// already unrecoverable, so treating it as a sidecar-integrity issue avoids /// raising false shard-corruption alarms. /// + /// A third outcome sits ahead of both of the above: if the identity fence + /// excluded a runtime AND the sidecar that supplied the protection was + /// resolved from that excluded runtime's directory, the checksums cannot be + /// trusted against + /// the merged shards at all. That case returns `(0, [], [errors])` with an + /// "unverifiable" note and never touches a shard handle — no scan, no + /// wholesale-mismatch classification, no blamed shard. + /// /// This method NEVER deletes or mutates anything — it is purely diagnostic. /// Duplicates the mounted shard handles so `run()` can scan with the store /// guard released; see `EcChecksumScrubPlan` for why that matters. pub fn checksum_scrub_plan(&self) -> EcChecksumScrubPlan { - let (prot, status) = self.bitrot_protection(); - - // Only BitrotOn reaches the shard loop in `run()`; the other statuses - // return before touching a handle, so cloning for them buys nothing. - let shards = match status { - crate::storage::erasure_coding::ec_bitrot::BitrotStatus::On => self - .shards - .iter() - .enumerate() - .filter_map(|(i, slot)| slot.as_ref().map(|s| (i as u32, s.try_clone_file()))) - .collect(), - _ => Vec::new(), - }; - - EcChecksumScrubPlan { - volume_id: self.volume_id, - prot, - status, - parity_shards: self.parity_shards, - shards, - } + EcChecksumScrubPlan::for_volumes(&[self]).expect("a single runtime is never an empty slice") } /// Convenience wrapper preserving the original call shape. Callers that @@ -1311,37 +1331,7 @@ impl EcVolume { /// distinction to decide whether a needle can be reassembled locally at /// all, so compacting it would silently change which needles get verified. pub fn scrub_local_plan(&self) -> EcLocalScrubPlan { - EcLocalScrubPlan { - volume_id: self.volume_id, - version: self.version, - data_shards: self.data_shards, - // locate_data wants shardSize = datFileSize / DataShards when known, - // else ecdFileSize - 1 (shards are padded to the small block size; - // the -1 avoids an off-by-one in the large-block row count). - shard_size: if self.dat_file_size > 0 { - self.dat_file_size / self.data_shards as i64 - } else { - self.shard_file_size() - 1 - }, - large_block_size: self.large_block_size(), - small_block_size: self.small_block_size(), - index: self.scrub_index_plan(), - ecx_path: self.ecx_file_name(), - // A second descriptor: the index plan's is consumed by its own walk, - // and both seek. - ecx_walk: File::open(self.ecx_file_name()), - shards: self - .shards - .iter() - .map(|slot| { - slot.as_ref().map(|s| EcLocalShard { - file: s.try_clone_file(), - file_size: s.file_size(), - info: s.to_ec_shard_info(), - }) - }) - .collect(), - } + EcLocalScrubPlan::for_volumes(&[self]).expect("a single runtime is never an empty slice") } /// ScrubLocal verifies each needle against the LOCAL shards only; it cannot @@ -1352,7 +1342,11 @@ impl EcVolume { /// hold the store lock MUST use `scrub_local_plan()` + `run()` instead. pub fn scrub_local( &self, - ) -> (u64, Vec, Vec) { + ) -> ( + u64, + Vec, + Vec, + ) { self.scrub_local_plan().run() } @@ -1516,9 +1510,7 @@ impl EcVolume { .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "ecj file not open"))?; let mut buf = [0u8; NEEDLE_ID_SIZE]; needle_id.to_bytes(&mut buf); - ecj_file - .write_all(&buf) - .and_then(|_| ecj_file.sync_all()) + ecj_file.write_all(&buf).and_then(|_| ecj_file.sync_all()) }; match append_result { @@ -1551,10 +1543,7 @@ impl EcVolume { /// Internal: binary search .ecx without masking by `deleted_needles`. /// Used by `journal_delete` so a repeat delete can still see the raw /// pre-existing .ecx tombstone from a prior rebuild. - fn find_needle_from_ecx_raw( - &self, - needle_id: NeedleId, - ) -> io::Result> { + fn find_needle_from_ecx_raw(&self, needle_id: NeedleId) -> io::Result> { let ecx_file = self .ecx_file .as_ref() @@ -1711,9 +1700,8 @@ impl EcVolume { // volume-id reuse cannot load stale protection, and so // collection.delete does not leave orphaned .ecsum files. // ecx_actual_dir is always one of these two dirs. - let _ = crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars( - &self.base_name(), - ); + let _ = + crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars(&self.base_name()); if self.dir_idx != self.dir { let idx_base = crate::storage::volume::volume_file_name( &self.dir_idx, @@ -1843,8 +1831,15 @@ mod tests { } v.sync_to_disk().unwrap(); v.close(); - crate::storage::erasure_coding::ec_encoder::write_ec_files(dir, dir, "", VolumeId(1), 10, 4) - .unwrap(); + crate::storage::erasure_coding::ec_encoder::write_ec_files( + dir, + dir, + "", + VolumeId(1), + 10, + 4, + ) + .unwrap(); let vol = EcVolume::new(dir, dir, "", VolumeId(1)).unwrap(); assert!( @@ -1894,8 +1889,15 @@ mod tests { } v.sync_to_disk().unwrap(); v.close(); - crate::storage::erasure_coding::ec_encoder::write_ec_files(dir, dir, "", VolumeId(1), 10, 4) - .unwrap(); + crate::storage::erasure_coding::ec_encoder::write_ec_files( + dir, + dir, + "", + VolumeId(1), + 10, + 4, + ) + .unwrap(); let mut vol = EcVolume::new(dir, dir, "", VolumeId(1)).unwrap(); for id in 0..14u8 { @@ -1980,8 +1982,15 @@ mod tests { } v.sync_to_disk().unwrap(); v.close(); - crate::storage::erasure_coding::ec_encoder::write_ec_files(dir, dir, "", VolumeId(1), 10, 4) - .unwrap(); + crate::storage::erasure_coding::ec_encoder::write_ec_files( + dir, + dir, + "", + VolumeId(1), + 10, + 4, + ) + .unwrap(); // Rewrite the first .ecx row's size as -1000: a negative that is NOT // the -1 tombstone the walk skips. A scrub is what you point at an @@ -2065,8 +2074,15 @@ mod tests { } v.sync_to_disk().unwrap(); v.close(); - crate::storage::erasure_coding::ec_encoder::write_ec_files(dir, dir, "", VolumeId(1), 10, 4) - .unwrap(); + crate::storage::erasure_coding::ec_encoder::write_ec_files( + dir, + dir, + "", + VolumeId(1), + 10, + 4, + ) + .unwrap(); let mut vol = EcVolume::new(dir, dir, "", VolumeId(1)).unwrap(); for id in 0..14u8 { @@ -2157,8 +2173,15 @@ mod tests { } v.sync_to_disk().unwrap(); v.close(); - crate::storage::erasure_coding::ec_encoder::write_ec_files(dir, dir, "", VolumeId(1), 10, 4) - .unwrap(); + crate::storage::erasure_coding::ec_encoder::write_ec_files( + dir, + dir, + "", + VolumeId(1), + 10, + 4, + ) + .unwrap(); let mut vol = EcVolume::new(dir, dir, "", VolumeId(1)).unwrap(); for id in 0..14u8 { @@ -2565,6 +2588,724 @@ mod tests { "a non-zero size mismatch is genuine corruption and must be reported" ); } + + /// Build one real encoded EC volume in `dir`, then hand back N runtimes over + /// it, each holding the shard ids it was given. Models the reconciled + /// split-disk mount without needing N directories. + fn split_runtimes(dir: &str, vid: VolumeId, subsets: &[&[u8]]) -> Vec { + use crate::storage::needle_map::NeedleMapKind; + use crate::storage::volume::Volume; + + let mut v = Volume::new( + dir, + dir, + "", + vid, + NeedleMapKind::InMemory, + None, + None, + 0, + Version::current(), + ) + .unwrap(); + for i in 1..=8 { + let data = format!("test data for needle {} with a bit more length", i); + let mut n = Needle { + id: NeedleId(i), + cookie: Cookie(i as u32), + data: data.as_bytes().to_vec(), + data_size: data.len() as u32, + ..Needle::default() + }; + v.write_needle(&mut n, true, false).unwrap(); + } + v.sync_to_disk().unwrap(); + v.close(); + crate::storage::erasure_coding::ec_encoder::write_ec_files(dir, dir, "", vid, 10, 4) + .unwrap(); + + subsets + .iter() + .map(|ids| { + let mut ev = EcVolume::new(dir, dir, "", vid).unwrap(); + for &id in ids.iter() { + ev.add_shard(EcVolumeShard::new(dir, "", vid, id)).unwrap(); + } + ev + }) + .collect() + } + + /// The union must reach every disk's shards, and first-disk-wins must + /// resolve a shard mounted on two disks — the same rule + /// `collect_ec_shard_dirs` and Go's `CollectEcShards` already use. + #[test] + fn test_merge_ec_runtimes_unions_slots_first_disk_wins() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + // Disk 0: 0..=6 plus 9 (9 is also on disk 1 — a duplicate mount). + // Disk 1: 7..=13. + let runtimes = split_runtimes( + dir, + VolumeId(1), + &[&[0, 1, 2, 3, 4, 5, 6, 9], &[7, 8, 9, 10, 11, 12, 13]], + ); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let merged = merge_ec_runtimes(&refs).expect("non-empty input merges"); + assert_eq!(merged.merged.len(), 2, "both runtimes share generation 0"); + assert!( + merged.skipped.is_empty(), + "nothing to skip: {:?}", + merged.skipped + ); + + // Every shard 0..=13 is reachable from the merged slots. + for id in 0..14usize { + assert!( + merged.slots[id].is_some(), + "shard {} missing from the union", + id + ); + } + // Shard 9 is held by both; the first disk wins. + let (owner, _) = merged.slots[9].unwrap(); + assert!( + std::ptr::eq(owner, refs[0]), + "duplicate shard must resolve to the first disk" + ); + // Slots are shard-id indexed across the volume's whole shard space, + // never compacted: index N is shard N or nothing. + assert_eq!( + merged.slots.len(), + 14, + "slots must span the full 10+4 shard space" + ); + } + + /// Leniency is keyed on the ANCHOR, never the holder: a known identity must + /// not accept an unstamped holder (store_ec.rs:667-673, + /// grpc_server.rs:3743-3748). Merging leftover legacy shards beside a + /// current encode is what produces false corruption reports. + #[test] + fn test_merge_ec_runtimes_excludes_incompatible_identities() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut runtimes = split_runtimes( + dir, + VolumeId(1), + &[&[0, 1, 2, 3, 4, 5, 6], &[7, 8, 9], &[10, 11, 12, 13]], + ); + runtimes[0].encode_ts_ns = 500; // stale, but stamped + runtimes[1].encode_ts_ns = 0; // legacy, unstamped + runtimes[2].encode_ts_ns = 900; // the live encode run + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let merged = merge_ec_runtimes(&refs).unwrap(); + + // Anchor is the newest known identity; only exact matches merge. + assert_eq!(merged.anchor.encode_ts_ns, 900); + assert_eq!(merged.merged.len(), 1); + assert!( + merged.slots[10].is_some(), + "the anchor's own shards must merge" + ); + assert!( + merged.slots[0].is_none(), + "an older known generation must not merge" + ); + assert!( + merged.slots[7].is_none(), + "an unstamped holder must not merge" + ); + + // Exclusions are REPORTED, never silent — that is what keeps this from + // regressing to the silent-clean bug this whole change fixes. + assert_eq!(merged.skipped.len(), 2, "got {:?}", merged.skipped); + assert!(merged.skipped.iter().all(|s| s.contains("not verified"))); + } + + /// When no runtime carries an identity there is nothing to fence on, so + /// behavior stays exactly as it is today: everything merges. + #[test] + fn test_merge_ec_runtimes_is_lenient_when_no_identity_is_known() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let runtimes = split_runtimes(dir, VolumeId(1), &[&[0, 1, 2], &[3, 4, 5]]); + assert!(runtimes.iter().all(|r| r.encode_ts_ns == 0)); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let merged = merge_ec_runtimes(&refs).unwrap(); + assert_eq!(merged.merged.len(), 2); + assert!(merged.skipped.is_empty()); + assert!(merged.slots[3].is_some()); + } + + /// The anchor supplies the volume-level metadata, so prefer a shard-bearing + /// runtime at the anchor generation over an empty one. + #[test] + fn test_merge_ec_runtimes_anchor_prefers_a_shard_bearing_runtime() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let runtimes = split_runtimes(dir, VolumeId(1), &[&[], &[0, 1, 2]]); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let merged = merge_ec_runtimes(&refs).unwrap(); + assert!( + std::ptr::eq(merged.anchor, refs[1]), + "anchor must hold shards" + ); + assert!( + merged.skipped.is_empty(), + "same generation: nothing is excluded" + ); + } + + /// Empty input is the vanished-volume case and the ONLY None. + #[test] + fn test_merge_ec_runtimes_none_only_when_empty() { + assert!(merge_ec_runtimes(&[]).is_none()); + } + + /// The vanished-volume case: callers' `let ... else` guards depend on this + /// being the ONLY None. + #[test] + fn test_for_volumes_is_none_only_for_an_empty_slice() { + assert!(EcChecksumScrubPlan::for_volumes(&[]).is_none()); + assert!(EcLocalScrubPlan::for_volumes(&[]).is_none()); + } + + /// The whole point: corruption on a shard that only a sibling runtime holds + /// must be reported. Before this change the scrub saw disk 0 alone and + /// returned clean. + #[test] + fn test_checksum_scrub_for_volumes_catches_sibling_disk_corruption() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + // Shard 11 is held ONLY by the second runtime. + let runtimes = split_runtimes( + dir, + VolumeId(1), + &[&[0, 1, 2, 3, 4, 5, 6], &[7, 8, 9, 10, 11, 12, 13]], + ); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + // Clean to start. + let (scanned, broken, errs) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert!(errs.is_empty(), "unexpected errors: {:?}", errs); + assert!(broken.is_empty(), "unexpected mismatches: {:?}", broken); + assert!(scanned > 0, "merged plan scanned nothing"); + + // Corrupt shard 11 — reachable only through the second runtime. + let shard11 = format!("{}/1.ec11", dir); + let mut bytes = std::fs::read(&shard11).unwrap(); + bytes[0] ^= 0xFF; + std::fs::write(&shard11, &bytes).unwrap(); + + let (_, broken2, _) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert!( + broken2.contains(&11), + "sibling-disk corruption must be flagged, got {:?}", + broken2 + ); + + // The single-runtime path is exactly the old blind spot: disk 0 alone + // still reports clean, which is why aggregation was needed. + let (_, broken_disk0, _) = refs[0].checksum_scrub(); + assert!(!broken_disk0.contains(&11)); + } + + /// An excluded runtime is reported through the plan's errors, but ONLY on + /// the On path — the Off arm's clean-empty contract is Go parity + /// (`case BitrotOff: return 0, nil, nil`) and must not grow errors. + #[test] + fn test_checksum_scrub_skips_are_reported_but_never_break_the_off_arm() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut runtimes = split_runtimes(dir, VolumeId(1), &[&[0, 1, 2], &[3, 4, 5]]); + runtimes[0].encode_ts_ns = 100; // excluded: older known identity + runtimes[1].encode_ts_ns = 200; // anchor + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + // With protection ON, the skip surfaces as an error line, and the + // surviving (anchor) runtime is still actually scanned. + let (scanned, _, errs) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert!( + scanned > 0, + "excluding one runtime must not stop the rest from being scanned" + ); + assert!( + errs.iter().any(|e| e.contains("not verified")), + "excluded runtime must be reported, got {:?}", + errs + ); + + // With protection OFF, the arm stays byte-for-byte clean. + let mut off = runtimes; + for r in off.iter_mut() { + r.bitrot_status = crate::storage::erasure_coding::ec_bitrot::BitrotStatus::Off; + r.bitrot = None; + } + let off_refs: Vec<&EcVolume> = off.iter().collect(); + assert_eq!( + EcChecksumScrubPlan::for_volumes(&off_refs).unwrap().run(), + (0, Vec::new(), Vec::new()), + "BitrotOff must stay clean-empty for Go parity" + ); + } + + /// The Invalid arm returns a non-empty error vector of its own, so the + /// Go-parity contract that silences the Off arm does not reach it. A volume + /// with BOTH a malformed sidecar and a fenced-out disk must report BOTH: + /// reporting only the sidecar hides an unscanned disk behind an unrelated + /// integrity error, which is the failure mode this whole change exists to + /// close. + #[test] + fn test_checksum_scrub_reports_skips_on_a_malformed_sidecar() { + use crate::storage::erasure_coding::ec_bitrot::BitrotStatus; + + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut runtimes = split_runtimes(dir, VolumeId(1), &[&[0, 1, 2], &[3, 4, 5]]); + runtimes[0].encode_ts_ns = 100; // excluded: older known identity + runtimes[1].encode_ts_ns = 200; // anchor + for r in runtimes.iter_mut() { + r.bitrot_status = BitrotStatus::Invalid; + } + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let (scanned, broken, errs) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert_eq!( + scanned, 0, + "a malformed sidecar must not be scanned against" + ); + assert!(broken.is_empty(), "must never blame shards: {:?}", broken); + assert_eq!( + errs.len(), + 2, + "expected the sidecar error AND the skip: {:?}", + errs + ); + assert!( + errs[0].contains("malformed/unverifiable"), + "the sidecar error stays first: {:?}", + errs + ); + assert!( + errs[1].contains("belong to encode run 100") && errs[1].contains("not verified"), + "the excluded disk must still be named: {:?}", + errs + ); + } + + /// When the anchor's sidecar came from a directory belonging to a runtime + /// the fence excluded, the checksums cannot be trusted against the anchor's + /// shards. Report that, never "shard N is corrupt". + #[test] + fn test_checksum_scrub_reports_unverifiable_sidecar_from_excluded_runtime() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut runtimes = split_runtimes(dir, VolumeId(1), &[&[0, 1, 2], &[3, 4, 5]]); + + // Mount actually resolved the real sidecar from `dir` (both subsets + // share one physical directory in this fixture), confirming + // `load_bitrot_for_generation` populates the field before the test + // overwrites it below to simulate a cross-disk borrow. + assert_eq!( + runtimes[1].bitrot_source_dir, dir, + "mount must record the dir the sidecar was actually resolved from" + ); + + // Runtime 0 is an older encode run, and the anchor resolved its sidecar + // from runtime 0's directory. + runtimes[0].encode_ts_ns = 100; + runtimes[0].dir = "/disk-old".to_string(); + runtimes[0].dir_idx = "/disk-old".to_string(); + runtimes[1].encode_ts_ns = 200; + runtimes[1].dir = "/disk-new".to_string(); + runtimes[1].dir_idx = "/disk-new".to_string(); + runtimes[1].bitrot_source_dir = "/disk-old".to_string(); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let (scanned, broken, errs) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert_eq!( + scanned, 0, + "an unverifiable sidecar must not be scanned against" + ); + assert!(broken.is_empty(), "must never blame shards: {:?}", broken); + assert!( + errs.iter().any(|e| e.contains("unverifiable")), + "expected an unverifiable-protection note, got {:?}", + errs + ); + assert!( + errs.iter().any(|e| e.contains("were not verified")), + "the fenced-out runtime must still be reported alongside the unverifiable note: {:?}", + errs + ); + } + + /// The provenance rule fires only when a runtime was actually excluded. A + /// healthy single-encode volume whose sidecar lives on a sibling disk is the + /// normal mirrored case and must scrub normally. + #[test] + fn test_checksum_scrub_provenance_rule_does_not_fire_without_exclusions() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut runtimes = split_runtimes( + dir, + VolumeId(1), + &[&[0, 1, 2, 3, 4, 5, 6], &[7, 8, 9, 10, 11, 12, 13]], + ); + // Same generation: nothing is excluded, so provenance is irrelevant even + // though the anchor's sidecar appears to have come from somewhere + // neither runtime owns. (The anchor is runtimes[0]: `merge_ec_runtimes` + // picks the first shard-bearing runtime at the anchor generation.) + runtimes[0].bitrot_source_dir = "/somewhere-else".to_string(); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let (scanned, broken, errs) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert!(scanned > 0, "a normal volume must still be scanned"); + assert!(broken.is_empty(), "{:?}", broken); + assert!( + !errs.iter().any(|e| e.contains("unverifiable")), + "{:?}", + errs + ); + } + + /// The identity fence keys on `encode_ts_ns` ALONE, never on geometry, so + /// two same-generation runtimes whose `.vif`s disagree about the layout do + /// merge -- and `slots` is then sized to the WIDER of them while the + /// volume's actual shard-id range is the ANCHOR's `data + parity`. + /// + /// The two consumers disagreed about exactly that range: the mode 2|5 arm + /// builds `dirs` over `0..anchor.data_shards + anchor.parity_shards` and + /// silently drops the surplus slots, while `EcChecksumScrubPlan` iterated + /// the full width and reported each one "present but missing from sidecar + /// manifest". Nothing in this volume describes those ids -- the sidecar + /// manifest and the Reed-Solomon matrix are both the anchor's -- so that + /// message is the width disagreement talking, not a finding. + #[test] + fn test_checksum_scrub_truncates_slots_to_the_anchors_geometry() { + use crate::pb::volume_server_pb::{ + ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums, + }; + use crate::storage::erasure_coding::ec_bitrot; + use crate::storage::volume::{VifEcShardConfig, VifVolumeInfo}; + + let tmp = TempDir::new().unwrap(); + let vid = VolumeId(1); + + // One dir per geometry: `EcVolume::new` reads `data_shards`/ + // `parity_shards` from the `.vif` beside the volume, so two runtimes can + // only disagree if they mount from different directories. + let seed = |name: &str, ds: u32, ps: u32, shard_id: u8| -> String { + let dir = tmp.path().join(name); + std::fs::create_dir_all(&dir).unwrap(); + let d = dir.to_str().unwrap().to_string(); + let base = crate::storage::volume::volume_file_name(&d, "", vid); + std::fs::write(format!("{}.ecx", base), b"").unwrap(); + std::fs::write(format!("{}.ecj", base), b"").unwrap(); + std::fs::write( + format!("{}.vif", base), + serde_json::to_string(&VifVolumeInfo { + version: 3, + ec_shard_config: Some(VifEcShardConfig { + data_shards: ds, + parity_shards: ps, + encode_ts_ns: 500, + ..Default::default() + }), + ..Default::default() + }) + .unwrap(), + ) + .unwrap(); + std::fs::write( + format!("{}.ec{:02}", base, shard_id), + b"shard data nonempty", + ) + .unwrap(); + d + }; + + // 10+4, and the only disk with a sidecar -- so it supplies `prot`, and + // its manifest covers shard ids 0..=13 and nothing else. + let narrow = seed("narrow", 10, 4, 0); + let prot = EcBitrotProtection { + algorithm: ChecksumAlgorithm::ChecksumCrc32c as i32, + block_size: ec_bitrot::DEFAULT_BITROT_BLOCK_SIZE as u32, + generation: 0, + ec_shard_config: Some(ec_bitrot::ec_shard_config(10, 4, 0)), + shards: (0..14u32) + .map(|shard_id| EcShardChecksums { + shard_id, + covered_size: 4, + block_crc32c: vec![0u8; 4], + }) + .collect(), + encode_uuid: vec![0u8; 16], + }; + ec_bitrot::save_bitrot_sidecar( + &ec_bitrot::bitrot_sidecar_path( + &crate::storage::volume::volume_file_name(&narrow, "", vid), + 0, + ), + &prot, + ) + .unwrap(); + + // 12+4: 16 slots, and it holds shard 14 -- an id the anchor's layout + // does not contain at all. Same encode_ts_ns, but the geometry fence + // now excludes it rather than merging incompatible shards. + let wide = seed("wide", 12, 4, 14); + + let mut narrow_v = EcVolume::new(&narrow, &narrow, "", vid).unwrap(); + narrow_v + .add_shard(EcVolumeShard::new(&narrow, "", vid, 0)) + .unwrap(); + let mut wide_v = EcVolume::new(&wide, &wide, "", vid).unwrap(); + wide_v + .add_shard(EcVolumeShard::new(&wide, "", vid, 14)) + .unwrap(); + + assert_eq!(narrow_v.shards.len(), 14); + assert_eq!(wide_v.shards.len(), 16); + assert_eq!( + narrow_v.encode_ts_ns, wide_v.encode_ts_ns, + "the two runtimes must share a generation" + ); + assert_eq!( + narrow_v.bitrot_status, + crate::storage::erasure_coding::ec_bitrot::BitrotStatus::On, + "the narrow disk must carry usable protection" + ); + + let refs: Vec<&EcVolume> = vec![&narrow_v, &wide_v]; + let merged = merge_ec_runtimes(&refs).unwrap(); + assert!( + std::ptr::eq(merged.anchor, refs[0]), + "the NARROW runtime must anchor" + ); + assert!( + !merged.skipped.is_empty(), + "the wide runtime must be excluded by the geometry fence: {:?}", + merged.skipped + ); + assert_eq!( + merged.slots.len(), + 14, + "slots are sized to the anchor's geometry, not the excluded wide runtime" + ); + assert!( + merged.slots.get(14).is_none() || merged.slots.get(14).unwrap().is_none(), + "shard 14 from the excluded runtime must not appear in slots" + ); + + let (scanned, broken, errs) = EcChecksumScrubPlan::for_volumes(&refs).unwrap().run(); + assert!( + !errs.iter().any(|e| e.contains("shard 14")), + "shard 14 belongs to an excluded runtime and must not be scanned: {:?}", + errs + ); + assert!( + scanned > 0, + "the anchor's shard 0 must still be scanned: {:?}", + errs + ); + assert_eq!( + broken, + vec![0], + "shard 0 is inside the anchor's geometry and its bytes do not match \ + the manifest, so it must still be reported: errs={:?}", + errs + ); + } + + /// `for_volumes` must reach every runtime's shards, not just the first's. + /// A needle walk alone can't prove that with this fixture: ~500 bytes of + /// data under a legacy 1GiB large block puts every needle interval on + /// shard 0, so the slot vector is asserted directly. The walk-level checks + /// below additionally show the merged plan verifies cleanly and then + /// detects corruption reachable through the first runtime, while a + /// single-runtime plan stays blind to it. + #[test] + fn test_scrub_local_for_volumes_merges_slots_across_runtimes() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let runtimes = split_runtimes(dir, VolumeId(1), &[&[0, 1, 2], &[7, 8, 9]]); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let (count, broken, errs) = EcLocalScrubPlan::for_volumes(&refs).unwrap().run(); + assert!(count > 0, "the merged plan walked no needles"); + assert!( + broken.is_empty(), + "clean volume reported broken shards: {:?}", + broken + ); + assert!(errs.is_empty(), "clean volume reported errors: {:?}", errs); + + // The property this task exists to deliver, asserted directly on the + // plan rather than through a needle walk. With ~500 bytes of fixture + // data and a legacy 1GiB large block every needle interval lands in + // shard 0, so no needle walk can ever reach a sibling runtime's + // shards -- but the slot vector can. Neither runtime holds every + // shard here, so the gap at shard 3 makes compaction load-bearing: + // a compacting implementation could still satisfy a "populated slots + // are reachable" check while silently reindexing the vector. + let merged_plan = EcLocalScrubPlan::for_volumes(&refs).unwrap(); + assert_eq!( + merged_plan.shards.len(), + 14, + "slots must span the full 10+4 shard space" + ); + assert!( + merged_plan.shards[0].is_some(), + "disk 0's shard 0 must be reachable" + ); + assert!( + merged_plan.shards[3].is_none(), + "a shard no runtime holds must stay an empty slot -- compaction would fill it" + ); + assert!( + merged_plan.shards[7].is_some(), + "disk 1's shard 7 must be reachable -- the bug being fixed" + ); + + // A single-runtime plan still sees only its own disk, which is exactly + // what made aggregation necessary. + let solo = EcLocalScrubPlan::for_volumes(&[refs[0]]).unwrap(); + assert!(solo.shards[0].is_some()); + assert!( + solo.shards[7].is_none(), + "refs[0] alone must not see the sibling's shard 7" + ); + + // This fixture's few hundred bytes of needle data all land inside + // shard 0's legacy 1GiB large block (the tiny volume never fills even + // one), so shard 0 alone — held only by the first runtime — carries + // every needle. Corrupting a needle body byte there is the LOCAL + // analogue of Task 3's sibling-disk corruption: the whole point is + // that a partial-shard `scrub_local` cannot see it at all (it just + // silently skips a needle it can't reassemble locally — that is not + // an error), while the merged plan reaches it through refs[0]. + let shard0 = format!("{}/1.ec00", dir); + let mut bytes = std::fs::read(&shard0).unwrap(); + assert!( + bytes.len() > 268, + "fixture shrank below the corruption offset; split_runtimes's \ + needle payloads changed, so this offset needs picking again" + ); + bytes[268] ^= 0xFF; // inside needle 4's data payload + std::fs::write(&shard0, &bytes).unwrap(); + + let (_, _, merged_errs) = EcLocalScrubPlan::for_volumes(&refs).unwrap().run(); + assert!( + !merged_errs.is_empty(), + "merged plan must detect corruption on shard 0, reachable through refs[0]" + ); + + // refs[1] never mounted shard 0, so it cannot see this corruption at + // all — that silent blind spot is exactly the bug aggregation fixes. + let (_, _, refs1_errs) = refs[1].scrub_local(); + assert!( + refs1_errs.is_empty(), + "runtime without shard 0 has no way to detect this corruption: {:?}", + refs1_errs + ); + } + + /// LOCAL's legacy `shard_size` fallback (`dat_file_size == 0`) is now a + /// NODE-WIDE input: it feeds `locate_data`'s offset math for every merged + /// sibling's shards, not just the anchor's. `anchor.shard_file_size()` + /// returns the anchor's FIRST held shard rather than a maximum, so one + /// truncated shard on the anchor disk would mis-offset every needle read + /// across every disk and manufacture corruption reports wholesale. Take the + /// max over the merged slots, the way `verify_ec_shards` already answers the + /// same question (`if size > shard_size { shard_size = size }`). + #[test] + fn test_scrub_local_shard_size_fallback_takes_the_max_across_disks() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + + // Mount two shardless runtimes first so the shard files can be doctored + // before `add_shard` caches their sizes. + let mut runtimes = split_runtimes(dir, VolumeId(1), &[&[], &[]]); + + // Disk 0's shard 0 is truncated -- a partially-copied shard, which is + // exactly the containment this fallback used to have and now must not + // lose. + let shard0_path = format!("{}/1.ec00", dir); + let full = std::fs::metadata(&shard0_path).unwrap().len(); + assert!(full > 1, "fixture shard is too small to truncate"); + std::fs::OpenOptions::new() + .write(true) + .open(&shard0_path) + .unwrap() + .set_len(full / 2) + .unwrap(); + + runtimes[0] + .add_shard(EcVolumeShard::new(dir, "", VolumeId(1), 0)) + .unwrap(); + runtimes[1] + .add_shard(EcVolumeShard::new(dir, "", VolumeId(1), 1)) + .unwrap(); + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let intact = std::fs::metadata(format!("{}/1.ec01", dir)).unwrap().len() as i64; + let truncated = (full / 2) as i64; + assert!( + truncated < intact, + "fixture must leave disk 0's shard SHORTER than disk 1's" + ); + + let merged = merge_ec_runtimes(&refs).unwrap(); + assert!( + std::ptr::eq(merged.anchor, refs[0]), + "the truncated disk must anchor, or the old sizing was already right" + ); + assert_eq!( + refs[0].dat_file_size, 0, + "this fixture writes no .vif, so the legacy fallback is the path \ + under test; with a dat_file_size the branch is never reached" + ); + assert_eq!( + refs[0].shard_file_size(), + truncated, + "the old source -- the anchor's first held shard -- is the \ + truncated one, which is what makes this observable" + ); + + let plan = EcLocalScrubPlan::for_volumes(&refs).unwrap(); + assert_eq!( + plan.shard_size, + intact - 1, + "one disk's truncated shard must not size every merged sibling's \ + shards; locate_data would then mis-offset every needle on every \ + disk and report corruption that is not there" + ); + } + + /// LOCAL has no bitrot status gate, so an excluded runtime is always + /// reported. + #[test] + fn test_scrub_local_reports_skipped_runtimes() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut runtimes = split_runtimes(dir, VolumeId(1), &[&[0, 1, 2], &[3, 4, 5]]); + runtimes[0].encode_ts_ns = 100; + runtimes[1].encode_ts_ns = 200; + let refs: Vec<&EcVolume> = runtimes.iter().collect(); + + let (_, _, errs) = EcLocalScrubPlan::for_volumes(&refs).unwrap().run(); + assert!( + errs.iter().any(|e| e.contains("not verified")), + "excluded runtime must be reported, got {:?}", + errs + ); + } } #[cfg(test)] @@ -2692,7 +3433,10 @@ mod uniform_layout_tests { std::fs::remove_file(format!("{}.idx", base)).unwrap(); let mut vol = EcVolume::new(dir, dir, "", vid).unwrap(); - assert_eq!(vol.block_size, block_size, "block size not loaded from .vif"); + assert_eq!( + vol.block_size, block_size, + "block size not loaded from .vif" + ); for i in 0..10u8 { vol.add_shard(EcVolumeShard::new(dir, "", vid, i)).unwrap(); } @@ -2741,7 +3485,9 @@ mod uniform_layout_tests { /// and the legacy block layout, which reconstructs a custom-ratio or /// uniform volume through the wrong matrix. fn seed_uniform_sidecar(base: &str, ds: u32, ps: u32, block: i64) { - use crate::pb::volume_server_pb::{ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums}; + use crate::pb::volume_server_pb::{ + ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums, + }; use crate::storage::erasure_coding::ec_bitrot; let prot = EcBitrotProtection { algorithm: ChecksumAlgorithm::ChecksumCrc32c as i32, @@ -2761,11 +3507,7 @@ mod uniform_layout_tests { } fn seed_config_free_vif(base: &str) { - std::fs::write( - format!("{}.vif", base), - r#"{"version":3,"datFileSize":0}"#, - ) - .unwrap(); + std::fs::write(format!("{}.vif", base), r#"{"version":3,"datFileSize":0}"#).unwrap(); } // A .vif that omits ecShardConfig answers nothing about the layout, so it @@ -2805,7 +3547,11 @@ mod uniform_layout_tests { let data = tempfile::TempDir::new().unwrap(); let sib = tempfile::TempDir::new().unwrap(); let (dir, sibling) = (data.path().to_str().unwrap(), sib.path().to_str().unwrap()); - seed_config_free_vif(&crate::storage::volume::volume_file_name(dir, "", VolumeId(13))); + seed_config_free_vif(&crate::storage::volume::volume_file_name( + dir, + "", + VolumeId(13), + )); seed_uniform_sidecar( &crate::storage::volume::volume_file_name(sibling, "", VolumeId(13)), 12, @@ -2813,14 +3559,9 @@ mod uniform_layout_tests { 3 * 1024 * 1024, ); - let got = read_ec_shard_config_across_dirs( - dir, - dir, - &[sibling.to_string()], - "", - VolumeId(13), - ) - .unwrap(); + let got = + read_ec_shard_config_across_dirs(dir, dir, &[sibling.to_string()], "", VolumeId(13)) + .unwrap(); assert_eq!(got, (12, 4, 3 * 1024 * 1024)); } @@ -2829,7 +3570,11 @@ mod uniform_layout_tests { fn read_ec_shard_config_config_free_vif_without_a_sidecar_is_legacy() { let d = tempfile::TempDir::new().unwrap(); let dir = d.path().to_str().unwrap(); - seed_config_free_vif(&crate::storage::volume::volume_file_name(dir, "", VolumeId(14))); + seed_config_free_vif(&crate::storage::volume::volume_file_name( + dir, + "", + VolumeId(14), + )); let got = read_ec_shard_config(dir, dir, "", VolumeId(14)).unwrap(); assert_eq!(got, (10, 4, 0)); } @@ -2866,15 +3611,14 @@ mod uniform_layout_tests { // uniform volume to 10+4 legacy and reconstructs through the wrong matrix. #[test] fn read_ec_shard_config_finds_the_sidecar_in_its_own_index_dir() { - use crate::pb::volume_server_pb::{ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums}; + use crate::pb::volume_server_pb::{ + ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums, + }; use crate::storage::erasure_coding::ec_bitrot; let data = tempfile::TempDir::new().unwrap(); let idx = tempfile::TempDir::new().unwrap(); - let (dir, dir_idx) = ( - data.path().to_str().unwrap(), - idx.path().to_str().unwrap(), - ); + let (dir, dir_idx) = (data.path().to_str().unwrap(), idx.path().to_str().unwrap()); let base = crate::storage::volume::volume_file_name(dir_idx, "", VolumeId(7)); let prot = EcBitrotProtection { algorithm: ChecksumAlgorithm::ChecksumCrc32c as i32, @@ -2899,7 +3643,9 @@ mod uniform_layout_tests { #[test] fn read_ec_shard_config_finds_a_sibling_disks_sidecar() { - use crate::pb::volume_server_pb::{ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums}; + use crate::pb::volume_server_pb::{ + ChecksumAlgorithm, EcBitrotProtection, EcShardChecksums, + }; use crate::storage::erasure_coding::ec_bitrot; let a = tempfile::TempDir::new().unwrap(); @@ -2934,6 +3680,107 @@ mod uniform_layout_tests { } } +/// One volume id's per-disk runtimes resolved into a single scrubbable view. +/// Every scrub mode builds from this, so there is exactly one answer to +/// "which disks count" per volume. +pub(crate) struct MergedEcRuntimes<'a> { + /// Volume-level metadata source: geometry, `.ecx` handles, version. NOT + /// bitrot protection — the `.ecsum` sidecar is per-DISK state, so + /// `EcChecksumScrubPlan::for_volumes` sources it from the first `merged` + /// runtime that has any. + pub anchor: &'a EcVolume, + /// Runtimes whose shards are safe to verify together. + pub merged: Vec<&'a EcVolume>, + /// Indexed BY SHARD ID. `None` is a shard no merged runtime holds. The + /// volume's shard-id range is the anchor's geometry + /// (`0..data_shards + parity_shards`); consumers truncate to it. + pub slots: Vec>, + /// One line per runtime excluded by the identity or geometry fence. + /// Reported, never dropped. + pub skipped: Vec, +} + +/// Resolve `runtimes` (one vid's mounts, in location order) into a merged view. +/// `None` for an empty slice (the vanished-volume case). +/// +/// Two fences admit a runtime into `merged`: the anchor's `encode_ts_ns` (the +/// maximum, so `0` implies every runtime is `0` and nothing is excluded — +/// legacy leniency), and the anchor's geometry (`data_shards`, `parity_shards`, +/// `block_size`). Equal timestamps do not guarantee equal layouts, so a +/// same-generation runtime whose `.vif` disagrees is excluded and reported +/// rather than merged into a plan that would apply the anchor's offsets and +/// checksums to incompatible shards. +pub(crate) fn merge_ec_runtimes<'a>(runtimes: &[&'a EcVolume]) -> Option> { + let anchor_gen = runtimes.iter().map(|v| v.encode_ts_ns).max()?; + + let gen_matches: Vec<&'a EcVolume> = runtimes + .iter() + .copied() + .filter(|v| v.encode_ts_ns == anchor_gen) + .collect(); + + let anchor = gen_matches + .iter() + .copied() + .find(|v| v.shards.iter().any(|s| s.is_some())) + .unwrap_or(gen_matches[0]); + + let merged: Vec<&'a EcVolume> = gen_matches + .iter() + .copied() + .filter(|v| { + v.data_shards == anchor.data_shards + && v.parity_shards == anchor.parity_shards + && v.block_size == anchor.block_size + }) + .collect(); + + let width = merged.iter().map(|v| v.shards.len()).max().unwrap_or(0); + let mut slots: Vec> = vec![None; width]; + for v in &merged { + for (id, slot) in v.shards.iter().enumerate() { + if let Some(shard) = slot.as_ref() { + if slots[id].is_none() { + slots[id] = Some((*v, shard)); + } + } + } + } + + let skipped: Vec = runtimes + .iter() + .enumerate() + .filter(|(_, v)| { + v.encode_ts_ns != anchor_gen + || v.data_shards != anchor.data_shards + || v.parity_shards != anchor.parity_shards + || v.block_size != anchor.block_size + }) + .map(|(pos, v)| { + if v.encode_ts_ns != anchor_gen { + format!( + "EC volume {} shards at {} (position {}) belong to encode run {} but the scrub anchors on {}; they were not verified", + v.volume_id.0, v.dir, pos, v.encode_ts_ns, anchor_gen + ) + } else { + format!( + "EC volume {} shards at {} (position {}) share encode run {} but disagree on geometry ({}+{} bs {} vs {}+{} bs {}); they were not verified", + v.volume_id.0, v.dir, pos, v.encode_ts_ns, + v.data_shards, v.parity_shards, v.block_size, + anchor.data_shards, anchor.parity_shards, anchor.block_size + ) + } + }) + .collect(); + + Some(MergedEcRuntimes { + anchor, + merged, + slots, + skipped, + }) +} + /// Self-contained input for an EC checksum scrub: the sidecar plus a duplicate /// of every mounted local shard handle, taken under the store read guard. /// @@ -2955,15 +3802,125 @@ pub struct EcChecksumScrubPlan { pub status: crate::storage::erasure_coding::ec_bitrot::BitrotStatus, pub parity_shards: u32, /// One entry per LOCAL shard: its id and a duplicate of the mounted - /// handle (or the error to report). Private so the plan can only be built - /// by `EcVolume::checksum_scrub_plan`, which is what makes "captured under - /// the guard" an invariant rather than a convention. + /// handle (or the error to report). Private so a plan can only be built + /// (via `checksum_scrub_plan()` or `for_volumes()`) from `&EcVolume` + /// references, which cannot be obtained without the store guard — which + /// is what makes "captured under the guard" an invariant rather than a + /// convention. /// /// `dup` shares the kernel file offset, so every read here is positional. shards: Vec<(u32, std::io::Result)>, + /// Runtimes the identity/geometry fence excluded, one line each. Surfaced + /// through `run()`'s errors on the `On`/`Invalid` paths. + skipped: Vec, + /// `Some(source_dir)` when the sidecar that supplied `prot`/`status` was + /// resolved from a directory belonging to a runtime the fence excluded. + /// `run()` reports unverifiable protection instead of blaming shards. + unverifiable_sidecar: Option, + /// One line per merged runtime whose sidecar resolved `Invalid`. Reported + /// even when protection is taken from a sibling that is `On`, so a + /// malformed sidecar is never silently discarded. + invalid_sidecar_errors: Vec, } impl EcChecksumScrubPlan { + /// Build one plan over every per-disk runtime of a volume id. `None` only + /// when `runtimes` is empty (the vanished-volume case). + pub fn for_volumes(runtimes: &[&EcVolume]) -> Option { + use crate::storage::erasure_coding::ec_bitrot::BitrotStatus; + + let merged = merge_ec_runtimes(runtimes)?; + let anchor = merged.anchor; + + // Protection is per-DISK state: the `.ecsum` sidecar is not mirrored, + // and at mount each runtime resolves it from its own directories only, + // so after a restart the runtime without the sidecar mounts `Off`. + // Take the first merged runtime that has protection: `On` if any, else + // `Invalid`, else the anchor's `Off`. This only moves toward more + // verification (`Off->On`, `Off->Invalid`, `Invalid->On`), never toward + // silence — the anchor is itself in `merged`, so the fallback is `Off` + // only when every merged runtime is `Off`. + let protection_source = merged + .merged + .iter() + .copied() + .find(|v| matches!(v.bitrot_protection().1, BitrotStatus::On)) + .or_else(|| { + merged + .merged + .iter() + .copied() + .find(|v| matches!(v.bitrot_protection().1, BitrotStatus::Invalid)) + }) + .unwrap_or(anchor); + let (prot, status) = protection_source.bitrot_protection(); + + // Collect every merged runtime whose sidecar resolved Invalid, excluding + // the protection_source (whose error the Invalid arm of run() already + // reports). These surface when protection is taken from a sibling that + // is On, so a malformed sidecar is never silently discarded. + let invalid_sidecar_errors: Vec = merged + .merged + .iter() + .filter(|v| { + let v_ptr: *const EcVolume = **v; + let src_ptr: *const EcVolume = protection_source; + !std::ptr::eq(v_ptr, src_ptr) + && matches!(v.bitrot_protection().1, BitrotStatus::Invalid) + }) + .map(|v| { + format!( + "EC volume {} bitrot sidecar at {} is malformed/unverifiable (sidecar integrity)", + v.volume_id.0, v.dir + ) + }) + .collect(); + + let total = (anchor.data_shards + anchor.parity_shards) as usize; + + let shards = match status { + crate::storage::erasure_coding::ec_bitrot::BitrotStatus::On => merged + .slots + .iter() + .take(total) + .enumerate() + .filter_map(|(id, slot)| slot.map(|(_, shard)| (id as u32, shard.try_clone_file()))) + .collect(), + _ => Vec::new(), + }; + + // If the fence excluded a runtime AND the sidecar that supplied + // protection was resolved from that excluded runtime's directory, the + // checksums cannot be trusted against the merged shards. + let unverifiable_sidecar = if merged.skipped.is_empty() { + None + } else { + let own_dirs: Vec = merged + .merged + .iter() + .flat_map(|v| [v.dir.as_str(), v.dir_idx.as_str()]) + .map(|d| d.trim_end_matches('/').to_string()) + .collect(); + let src = protection_source.bitrot_source_dir.trim_end_matches('/'); + if src.is_empty() || own_dirs.iter().any(|d| d == src) { + None + } else { + Some(protection_source.bitrot_source_dir.clone()) + } + }; + + Some(EcChecksumScrubPlan { + volume_id: anchor.volume_id, + prot, + status, + parity_shards: anchor.parity_shards, + shards, + skipped: merged.skipped, + unverifiable_sidecar, + invalid_sidecar_errors, + }) + } + /// The byte-verification pass. Touches only the filesystem — no store, no /// lock — so it is safe to hand to `spawn_blocking`. pub fn run(self) -> (u64, Vec, Vec) { @@ -2972,40 +3929,40 @@ impl EcChecksumScrubPlan { let mut errors: Vec = Vec::new(); - // Resolve the active-generation protection AND its status, mirroring - // Go's `ChecksumScrub` (`prot, status := ecv.BitrotProtection()`): - // - BitrotOff => sidecars are OPTIONAL; an absent (or generation/ - // config-mismatched) sidecar simply means protection is not enabled - // for this generation. Return a CLEAN, EMPTY result — NOT an error — - // so legacy/intentionally-unprotected volumes are never reported - // broken. (Go: `case BitrotOff: return 0, nil, nil`.) - // - BitrotInvalid => the sidecar is PRESENT but malformed/unverifiable - // (self-integrity or manifest failure). That is the only status that - // yields an integrity error here. - // - BitrotOn => scan local shards against it. + // Mirrors Go's `ChecksumScrub` (`prot, status := ecv.BitrotProtection()`): + // - Off: sidecars are optional; return clean (Go: `case BitrotOff: return 0, nil, nil`). + // - Invalid: sidecar is present but malformed; report an integrity error. + // - On: scan local shards against it. let prot = match (self.prot, self.status) { (_, BitrotStatus::Off) => { - // Unprotected generation: nothing to verify. Not an error. return (0, Vec::new(), Vec::new()); } (_, BitrotStatus::Invalid) => { - return ( - 0, - Vec::new(), - vec![format!( - "EC volume {} bitrot sidecar is malformed/unverifiable (sidecar integrity)", - self.volume_id.0 - )], - ); + let mut errs = vec![format!( + "EC volume {} bitrot sidecar is malformed/unverifiable (sidecar integrity)", + self.volume_id.0 + )]; + errs.extend(self.skipped); + errs.extend(self.invalid_sidecar_errors); + return (0, Vec::new(), errs); } (Some(p), BitrotStatus::On) => p, (None, BitrotStatus::On) => { - // Unreachable: BitrotOn always carries a loaded sidecar. Treat a - // missing payload defensively as protection off (clean no-op). return (0, Vec::new(), Vec::new()); } }; + errors.extend(self.skipped); + errors.extend(self.invalid_sidecar_errors); + + if let Some(src) = self.unverifiable_sidecar { + errors.push(format!( + "EC volume {} bitrot sidecar was resolved from {}, which belongs to an excluded encode run; protection unverifiable", + self.volume_id.0, src + )); + return (0, Vec::new(), errors); + } + let block_size = prot.block_size as i64; let mut blocks_scanned: u64 = 0; @@ -3103,7 +4060,10 @@ impl EcIndexScrubPlan { if self.ecx_file_size == 0 { return ( 0, - vec![format!("zero-size ECX file for EC volume {}", self.volume_id.0)], + vec![format!( + "zero-size ECX file for EC volume {}", + self.volume_id.0 + )], ); } @@ -3112,12 +4072,7 @@ impl EcIndexScrubPlan { // after the two guards above so the error ordering is unchanged. let mut ecx_file = match self.ecx_handle { Ok(f) => f, - Err(e) => { - return ( - 0, - vec![format!("open ECX file {}: {}", self.ecx_path, e)], - ) - } + Err(e) => return (0, vec![format!("open ECX file {}: {}", self.ecx_path, e)]), }; crate::storage::idx::check_index_file(&mut ecx_file, self.ecx_file_size, self.version) } @@ -3175,13 +4130,95 @@ pub struct EcLocalScrubPlan { index: EcIndexScrubPlan, ecx_path: String, ecx_walk: std::io::Result, - /// Indexed BY SHARD ID; `None` is a shard this node does not hold. + /// Indexed BY SHARD ID; `None` is a shard no merged runtime holds. shards: Vec>, + /// Runtimes the identity fence excluded, one line each. LOCAL has no status + /// gate, so `run()` always reports these. + skipped: Vec, } impl EcLocalScrubPlan { + /// Build one plan over every per-disk runtime of a volume id. `None` only + /// when `runtimes` is empty. + /// + /// The shard vector keeps its SLOT structure — index is the shard id, gaps + /// are shards no merged runtime holds. `run()` reads that distinction to + /// decide whether a needle can be reassembled locally at all, so compacting + /// it would silently change which needles get verified. + pub fn for_volumes(runtimes: &[&EcVolume]) -> Option { + let merged = merge_ec_runtimes(runtimes)?; + let anchor = merged.anchor; + + Some(EcLocalScrubPlan { + volume_id: anchor.volume_id, + version: anchor.version, + data_shards: anchor.data_shards, + // locate_data wants shardSize = datFileSize / DataShards when known, + // else ecdFileSize - 1 (shards are padded to the small block size; + // the -1 avoids an off-by-one in the large-block row count). + // + // The fallback takes the MAX over every merged slot, not + // `anchor.shard_file_size()` -- which returns the anchor's FIRST + // held shard, not a maximum. Before aggregation the plan only read + // the anchor's own shards, so a truncated shard there mis-sized only + // its own runtime; now one disk's truncated shard would set + // `shard_size` for every merged sibling's shards, mis-offset + // `locate_data` and manufacture needle corruption across the whole + // node. `verify_ec_shards` already answers this same question the + // same way (`if size > shard_size { shard_size = size }`), so this + // is the in-tree convention rather than a preference. Reached only + // on the legacy `dat_file_size == 0` path. + // + // `.take(..)` honors the `slots` contract: the volume's shard-id + // range is the ANCHOR's geometry, and a same-generation runtime with + // a disagreeing `.vif` can populate slots beyond it. Scanning those + // would let an OUT-OF-GEOMETRY shard -- one nothing in this volume's + // layout describes -- set the offset math for every shard that IS in + // it, which is a narrower path to exactly the node-wide mis-sizing + // this max exists to close. + shard_size: if anchor.dat_file_size > 0 { + anchor.dat_file_size / anchor.data_shards as i64 + } else { + merged + .slots + .iter() + .take((anchor.data_shards + anchor.parity_shards) as usize) + .flatten() + .map(|(_, s)| s.file_size()) + .max() + .unwrap_or(0) + - 1 + }, + large_block_size: anchor.large_block_size(), + small_block_size: anchor.small_block_size(), + index: anchor.scrub_index_plan(), + ecx_path: anchor.ecx_file_name(), + // A second descriptor: the index plan's is consumed by its own walk, + // and both seek. + ecx_walk: File::open(anchor.ecx_file_name()), + shards: merged + .slots + .iter() + .map(|slot| { + slot.map(|(_, s)| EcLocalShard { + file: s.try_clone_file(), + file_size: s.file_size(), + info: s.to_ec_shard_info(), + }) + }) + .collect(), + skipped: merged.skipped, + }) + } + /// The needle walk. Filesystem only — no store, no lock. - pub fn run(self) -> (u64, Vec, Vec) { + pub fn run( + self, + ) -> ( + u64, + Vec, + Vec, + ) { let EcLocalScrubPlan { volume_id, version, @@ -3193,11 +4230,16 @@ impl EcLocalScrubPlan { ecx_path, ecx_walk, shards, + skipped, } = self; // Local scan also verifies the index. let (_, mut errs) = index.run(); + // LOCAL has no protection status to gate on, so an excluded runtime is + // always reported rather than silently unscanned. + errs.extend(skipped); + let mut broken_shards: HashSet = HashSet::new(); let mut count: u64 = 0; diff --git a/seaweed-volume/src/storage/store.rs b/seaweed-volume/src/storage/store.rs index f8eebdf72..4cf26bf15 100644 --- a/seaweed-volume/src/storage/store.rs +++ b/seaweed-volume/src/storage/store.rs @@ -1035,6 +1035,20 @@ impl Store { dirs } + /// Every per-disk `EcVolume` this store maps for `vid`, in location order. + /// Immutable twin of [`Self::find_all_ec_volumes_mut`]. + /// + /// Reconciliation can mount one vid as N runtimes holding disjoint shard + /// subsets, and the first-match `find_ec_volume` hides the siblings. Anything + /// that has to reach the whole volume, rather than any one runtime of it, + /// uses this. + pub fn find_all_ec_volumes(&self, vid: VolumeId) -> Vec<&EcVolume> { + self.locations + .iter() + .filter_map(|loc| loc.find_ec_volume(vid)) + .collect() + } + pub fn find_all_ec_volumes_mut(&mut self, vid: VolumeId) -> Vec<&mut EcVolume> { self.locations .iter_mut() diff --git a/seaweed-volume/src/storage/store_ec_reconcile.rs b/seaweed-volume/src/storage/store_ec_reconcile.rs index cfc27c7cd..9c2a751ca 100644 --- a/seaweed-volume/src/storage/store_ec_reconcile.rs +++ b/seaweed-volume/src/storage/store_ec_reconcile.rs @@ -1184,6 +1184,76 @@ mod tests { assert!(!std::ptr::eq(ev0, ev1)); } + /// `find_ec_volume` returns only disk 0's runtime, which is what hides + /// sibling-disk shards from every scrub mode. The plural lookup must + /// return one runtime per disk holding the vid, in location order. + #[test] + fn test_find_all_ec_volumes_returns_every_disk() { + let (store, _tmp) = build_split_disk_store(7010); + let vid = VolumeId(7010); + + let all = store.find_all_ec_volumes(vid); + assert_eq!(all.len(), 2, "expected one EcVolume per disk holding the vid"); + + // Disk 0 carries shards 0 and 12; disk 1 carries shard 1. + assert!(all[0].has_shard(0)); + assert!(all[0].has_shard(12)); + assert!(all[1].has_shard(1)); + + // The singular lookup sees only the first — the bug being fixed. + let first = store.find_ec_volume(vid).unwrap(); + assert!(std::ptr::eq(first, all[0])); + + // A vid nobody mounts yields an empty vec, not a panic. + assert!(store.find_all_ec_volumes(VolumeId(9999)).is_empty()); + } + + /// End-to-end: with the vid mounted on two disks, a scrub driven through + /// the Store must reach BOTH disks' shards. Before the aggregation fix + /// `find_ec_volume` returned disk 0 and disk 1's shard 1 was never read. + #[test] + fn test_scrub_plans_reach_every_disk_through_the_store() { + use crate::storage::erasure_coding::ec_volume::{ + merge_ec_runtimes, EcChecksumScrubPlan, EcLocalScrubPlan, + }; + + let (store, _tmp) = build_split_disk_store(7030); + let vid = VolumeId(7030); + + let runtimes = store.find_all_ec_volumes(vid); + assert_eq!(runtimes.len(), 2); + + // Reachability is the invariant, so assert on the resolved slots rather + // than on scrub message text: shards 0 and 12 live on disk 0, shard 1 on + // disk 1. The old first-match lookup could never see shard 1. + let merged = merge_ec_runtimes(&runtimes).expect("two runtimes merge"); + assert!(merged.slots[0].is_some(), "disk 0's shard 0 unreachable"); + assert!(merged.slots[12].is_some(), "disk 0's shard 12 unreachable"); + assert!(merged.slots[1].is_some(), "disk 1's shard 1 unreachable — the bug"); + assert!(merged.skipped.is_empty(), "same generation: {:?}", merged.skipped); + + // Shard 1 is owned by the sibling runtime, not the anchor. + let (owner, _) = merged.slots[1].unwrap(); + assert!(std::ptr::eq(owner, runtimes[1])); + + // Both plans build over the union rather than over disk 0 alone. + assert!(EcChecksumScrubPlan::for_volumes(&runtimes).is_some()); + assert!(EcLocalScrubPlan::for_volumes(&runtimes).is_some()); + // ...and `is_some()` is a real question: `for_volumes` has exactly one + // `None` (the vanished-volume case), so without this the two lines above + // would hold for any input at all. + assert!(EcChecksumScrubPlan::for_volumes(&[]).is_none()); + assert!(EcLocalScrubPlan::for_volumes(&[]).is_none()); + + // Regression guard: a single-runtime view still sees only its own disk, + // which is exactly what made aggregation necessary. + let disk0 = merge_ec_runtimes(&[runtimes[0]]).unwrap(); + assert!( + disk0.slots.get(1).copied().flatten().is_none(), + "disk 0's runtime must not see the sibling's shard" + ); + } + /// `Store::unmount_ec_shards` used to return after the first /// location with the vid, so a request to unmount a shard that /// lives on a sibling disk became a silent no-op. After the fix, diff --git a/weed/server/volume_grpc_scrub.go b/weed/server/volume_grpc_scrub.go index 325cde652..cfd8aafaf 100644 --- a/weed/server/volume_grpc_scrub.go +++ b/weed/server/volume_grpc_scrub.go @@ -11,6 +11,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" "github.com/seaweedfs/seaweedfs/weed/stats" "github.com/seaweedfs/seaweedfs/weed/storage" + "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" "github.com/seaweedfs/seaweedfs/weed/storage/needle" ) @@ -127,8 +128,18 @@ func (vs *VolumeServer) ScrubEcVolume(ctx context.Context, req *volume_server_pb vids := []needle.VolumeId{} explicit := len(req.GetVolumeIds()) != 0 if !explicit { + // A split-disk volume is mounted once per disk, so a node-wide + // listing would otherwise scrub it once per location. Dedupe in + // location order so the merged view still sees every runtime. + seen := map[needle.VolumeId]struct{}{} for _, l := range vs.store.Locations { - vids = append(vids, l.EcVolumeIds()...) + for _, vid := range l.EcVolumeIds() { + if _, ok := seen[vid]; ok { + continue + } + seen[vid] = struct{}{} + vids = append(vids, vid) + } } } else { for _, vid := range req.GetVolumeIds() { @@ -147,8 +158,14 @@ func (vs *VolumeServer) scrubEcVolumes(req *volume_server_pb.ScrubEcVolumeReques var brokenVolumeIds []uint32 var brokenShardInfos []*volume_server_pb.EcShardInfo for _, vid := range vids { - v, found := vs.store.FindEcVolume(vid) - if !found { + // Resolve every per-disk runtime, not just the first: a reconciled + // volume's shards are split across runtimes, and a scrub that only + // sees the first disk misses the rest. The merged view fences on + // encode generation and geometry so incompatible runtimes are + // reported rather than verified together. + runtimes := vs.store.FindAllEcVolumes(vid) + merged := erasure_coding.MergeEcRuntimes(runtimes) + if merged == nil { if explicit { return nil, fmt.Errorf("EC volume id %d not found", vid) } @@ -161,18 +178,20 @@ func (vs *VolumeServer) scrubEcVolumes(req *volume_server_pb.ScrubEcVolumeReques var serrs []error switch m := req.GetMode(); m { case volume_server_pb.VolumeScrubMode_INDEX: - // index scrubs do not verify individual EC shards - files, serrs = v.ScrubIndex() + files, serrs = merged.Anchor.ScrubIndex() + for _, sk := range merged.Skipped { + serrs = append(serrs, fmt.Errorf("%s", sk)) + } case volume_server_pb.VolumeScrubMode_LOCAL: - files, shardInfos, serrs = v.ScrubLocal() + files, shardInfos, serrs = merged.ScrubLocal() case volume_server_pb.VolumeScrubMode_FULL, volume_server_pb.VolumeScrubMode_READS: - files, shardInfos, serrs = vs.store.ScrubEcVolume(v.VolumeId, m, req.GetForceDeletedNeedlesCheck()) + files, shardInfos, serrs = vs.store.ScrubEcVolumeMerged(merged, m, req.GetForceDeletedNeedlesCheck()) case volume_server_pb.VolumeScrubMode_CHECKSUM: // Verify each local shard's raw bytes against the bitrot sidecar, - // exercising cold parity shards. Read-only. ChecksumScrub's first - // return is blocks scanned, not files — discard it so TotalFiles - // (a needle/file count) isn't inflated by the block count. - _, shardInfos, serrs = v.ChecksumScrub() + // exercising cold parity shards. Read-only. The first return is + // blocks scanned, not files — discard it so TotalFiles (a + // needle/file count) isn't inflated by the block count. + _, shardInfos, serrs = merged.ChecksumScrub() default: return nil, fmt.Errorf("unsupported EC volume scrub mode %d", m) } @@ -180,7 +199,7 @@ func (vs *VolumeServer) scrubEcVolumes(req *volume_server_pb.ScrubEcVolumeReques totalVolumes += 1 totalFiles += uint64(files) if len(serrs) != 0 || len(shardInfos) != 0 { - brokenVolumeIds = append(brokenVolumeIds, uint32(v.VolumeId)) + brokenVolumeIds = append(brokenVolumeIds, uint32(vid)) brokenShardInfos = append(brokenShardInfos, shardInfos...) for _, err := range serrs { details = append(details, err.Error()) diff --git a/weed/server/volume_grpc_scrub_ec_merge_test.go b/weed/server/volume_grpc_scrub_ec_merge_test.go new file mode 100644 index 000000000..b113a5c5c --- /dev/null +++ b/weed/server/volume_grpc_scrub_ec_merge_test.go @@ -0,0 +1,178 @@ +package weed_server + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + + "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" + "github.com/seaweedfs/seaweedfs/weed/stats" + "github.com/seaweedfs/seaweedfs/weed/storage" + "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" + "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" + "github.com/seaweedfs/seaweedfs/weed/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newMultiDiskEcScrubStore builds a store with one DiskLocation per dir, so a +// volume whose shards are split across dirs mounts as one EcVolume per disk. +func newMultiDiskEcScrubStore(t *testing.T, dirs ...string) *storage.Store { + t.Helper() + max := make([]int32, len(dirs)) + minFree := make([]util.MinFreeSpace, len(dirs)) + diskTypes := make([]types.DiskType, len(dirs)) + tags := make([][]string, len(dirs)) + for i := range dirs { + max[i] = 10 + diskTypes[i] = types.HardDriveType + } + // idxFolder shared so .ecx/.ecj resolve from any disk's index dir. + s := storage.NewStore( + grpc.WithTransportCredentials(insecure.NewCredentials()), + "127.0.0.1", 0, 0, "", "test-store", + dirs, max, minFree, + dirs[0], storage.NeedleMapInMemory, + diskTypes, tags, + 0, stats.DiskIOProbeConfig{}, + ) + t.Cleanup(s.Close) + return s +} + +// seedEcShardOnDisk writes the minimal files for one shard id on one disk and +// mounts it. The .ecx is shared (one entry) so ScrubIndex/ScrubLocal can run. +func seedEcShardOnDisk(t *testing.T, store *storage.Store, vid needle.VolumeId, collection, dir string, shardId int, encodeTs int64, dataShards, parityShards int, blockSize int64) { + t.Helper() + base := erasure_coding.EcShardFileName(collection, dir, int(vid)) + require.NoError(t, os.WriteFile(base+".ecx", make([]byte, types.NeedleMapEntrySize), 0o644)) + require.NoError(t, os.WriteFile(base+".ecj", nil, 0o644)) + require.NoError(t, os.WriteFile(base+erasure_coding.ToExt(shardId), []byte("s"), 0o644)) + vif := &volume_server_pb.VolumeInfo{ + Version: uint32(needle.Version3), + EcShardConfig: &volume_server_pb.EcShardConfig{ + DataShards: uint32(dataShards), + ParityShards: uint32(parityShards), + BlockSize: blockSize, + EncodeTsNs: encodeTs, + }, + } + require.NoError(t, volume_info.SaveVolumeInfo(base+".vif", vif)) + require.NoError(t, store.MountEcShards(collection, vid, erasure_coding.ShardId(shardId), types.HardDriveType.String())) +} + +// TestScrubEcVolume_DedupesSplitDiskVolume: a volume mounted on two disks must +// be scrubbed once (TotalVolumes == 1), not once per location. +func TestScrubEcVolume_DedupesSplitDiskVolume(t *testing.T) { + diskA, diskB := t.TempDir(), t.TempDir() + store := newMultiDiskEcScrubStore(t, diskA, diskB) + const vid = needle.VolumeId(900) + seedEcShardOnDisk(t, store, vid, "split", diskA, 0, 0, 10, 4, 0) + seedEcShardOnDisk(t, store, vid, "split", diskB, 1, 0, 10, 4, 0) + + vs := &VolumeServer{store: store} + res, err := vs.scrubEcVolumes( + &volume_server_pb.ScrubEcVolumeRequest{Mode: volume_server_pb.VolumeScrubMode_INDEX}, + []needle.VolumeId{vid}, true) + require.NoError(t, err) + assert.Equal(t, uint64(1), res.GetTotalVolumes(), "a split-disk volume must be scrubbed once, not once per disk") +} + +// TestScrubEcVolume_FencesOnEncodeGeneration: a runtime from an older encode +// run must be excluded and reported, not merged into the scrub. +func TestScrubEcVolume_FencesOnEncodeGeneration(t *testing.T) { + diskA, diskB := t.TempDir(), t.TempDir() + store := newMultiDiskEcScrubStore(t, diskA, diskB) + const vid = needle.VolumeId(901) + seedEcShardOnDisk(t, store, vid, "gen", diskA, 0, 1000, 10, 4, 0) + seedEcShardOnDisk(t, store, vid, "gen", diskB, 1, 500, 10, 4, 0) + + vs := &VolumeServer{store: store} + res, err := vs.scrubEcVolumes( + &volume_server_pb.ScrubEcVolumeRequest{Mode: volume_server_pb.VolumeScrubMode_INDEX}, + []needle.VolumeId{vid}, true) + require.NoError(t, err) + assert.Equal(t, uint64(1), res.GetTotalVolumes()) + var foundSkip bool + for _, d := range res.GetDetails() { + if strings.Contains(d, "belong to encode run 500") { + foundSkip = true + } + } + assert.True(t, foundSkip, "older encode-generation runtime must be reported as skipped, got details: %v", res.GetDetails()) +} + +// TestScrubEcVolume_FencesOnGeometry: same encode timestamp but disagreeing +// geometry must exclude and report the incompatible runtime. +func TestScrubEcVolume_FencesOnGeometry(t *testing.T) { + diskA, diskB := t.TempDir(), t.TempDir() + store := newMultiDiskEcScrubStore(t, diskA, diskB) + const vid = needle.VolumeId(902) + seedEcShardOnDisk(t, store, vid, "geo", diskA, 0, 1000, 10, 4, 3*1024*1024) + seedEcShardOnDisk(t, store, vid, "geo", diskB, 1, 1000, 12, 4, 3*1024*1024) + + vs := &VolumeServer{store: store} + res, err := vs.scrubEcVolumes( + &volume_server_pb.ScrubEcVolumeRequest{Mode: volume_server_pb.VolumeScrubMode_INDEX}, + []needle.VolumeId{vid}, true) + require.NoError(t, err) + assert.Equal(t, uint64(1), res.GetTotalVolumes()) + var foundGeoSkip bool + for _, d := range res.GetDetails() { + if strings.Contains(d, "disagree on geometry") { + foundGeoSkip = true + } + } + assert.True(t, foundGeoSkip, "geometry-mismatched runtime must be reported as skipped, got details: %v", res.GetDetails()) +} + +// TestScrubEcVolume_LocalReachesSiblingDisk: LOCAL mode over a split-disk +// volume must see shards from both disks (the merged view), not just the +// first. With both shards local, the index walk has no missing-shard error. +func TestScrubEcVolume_LocalReachesSiblingDisk(t *testing.T) { + diskA, diskB := t.TempDir(), t.TempDir() + store := newMultiDiskEcScrubStore(t, diskA, diskB) + const vid = needle.VolumeId(903) + seedEcShardOnDisk(t, store, vid, "sib", diskA, 0, 0, 10, 4, 0) + seedEcShardOnDisk(t, store, vid, "sib", diskB, 1, 0, 10, 4, 0) + + vs := &VolumeServer{store: store} + res, err := vs.scrubEcVolumes( + &volume_server_pb.ScrubEcVolumeRequest{Mode: volume_server_pb.VolumeScrubMode_LOCAL}, + []needle.VolumeId{vid}, true) + require.NoError(t, err) + assert.Equal(t, uint64(1), res.GetTotalVolumes()) +} + +// TestMergeEcRuntimes_ReportsAnchorGeneration: the anchor is the maximum +// encode generation, and a single-runtime volume anchors on itself. +func TestMergeEcRuntimes_ReportsAnchorGeneration(t *testing.T) { + dir := t.TempDir() + store := newMultiDiskEcScrubStore(t, dir) + const vid = needle.VolumeId(904) + seedEcShardOnDisk(t, store, vid, "anchor", dir, 0, 7777, 10, 4, 0) + + runtimes := store.FindAllEcVolumes(vid) + require.Len(t, runtimes, 1) + merged := erasure_coding.MergeEcRuntimes(runtimes) + require.NotNil(t, merged) + assert.Equal(t, int64(7777), merged.Anchor.EncodeTsNs) + assert.Empty(t, merged.Skipped) + assert.Len(t, merged.Merged, 1) +} + +// TestMergeEcRuntimes_NilForEmptyInput: an empty runtime slice (vanished +// volume) yields a nil merged view. +func TestMergeEcRuntimes_NilForEmptyInput(t *testing.T) { + assert.Nil(t, erasure_coding.MergeEcRuntimes(nil)) + assert.Nil(t, erasure_coding.MergeEcRuntimes([]*erasure_coding.EcVolume{})) +} + +// Ensure filepath import is used (helper for future multi-disk tests). +var _ = filepath.Join diff --git a/weed/storage/erasure_coding/ec_volume_merge.go b/weed/storage/erasure_coding/ec_volume_merge.go new file mode 100644 index 000000000..d031264b4 --- /dev/null +++ b/weed/storage/erasure_coding/ec_volume_merge.go @@ -0,0 +1,301 @@ +package erasure_coding + +import ( + "fmt" + + "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" +) + +// MergedEcRuntimes resolves one volume id's per-disk runtimes into a single +// scrubbable view. Every scrub mode builds from this, so there is exactly one +// answer to "which disks count" per volume. +// +// Two fences admit a runtime into Merged: the anchor's EncodeTsNs (the +// maximum, so 0 implies every runtime is 0 and nothing is excluded — legacy +// leniency), and the anchor's geometry (DataShards, ParityShards, BlockSize). +// Equal timestamps do not guarantee equal layouts, so a same-generation +// runtime whose .vif disagrees is excluded and reported rather than merged +// into a plan that would apply the anchor's offsets and checksums to +// incompatible shards. +type MergedEcRuntimes struct { + // Anchor is the volume-level metadata source: geometry, .ecx handles, + // version. It is NOT the bitrot protection source — the .ecsum sidecar is + // per-DISK state, so ChecksumScrubMerged sources it from the first Merged + // runtime that has any. + Anchor *EcVolume + // Merged holds the runtimes whose shards are safe to verify together. + Merged []*EcVolume + // Slots is indexed BY SHARD ID. A nil entry is a shard no merged runtime + // holds. The volume's shard-id range is the anchor's geometry + // (0..DataShards+ParityShards); consumers truncate to it. + Slots []*EcVolumeShard + // Skipped holds one line per runtime excluded by the identity or geometry + // fence. Reported, never dropped. + Skipped []string +} + +// MergeEcRuntimes resolves runtimes (one vid's mounts, in location order) into +// a merged view. Returns nil for an empty slice (the vanished-volume case). +func MergeEcRuntimes(runtimes []*EcVolume) *MergedEcRuntimes { + if len(runtimes) == 0 { + return nil + } + + anchorGen := int64(0) + for _, v := range runtimes { + if v.EncodeTsNs > anchorGen { + anchorGen = v.EncodeTsNs + } + } + + var genMatches []*EcVolume + for _, v := range runtimes { + if v.EncodeTsNs == anchorGen { + genMatches = append(genMatches, v) + } + } + + anchor := genMatches[0] + for _, v := range genMatches { + if len(v.Shards) > 0 { + anchor = v + break + } + } + + var merged []*EcVolume + for _, v := range genMatches { + if geometryMatches(v, anchor) { + merged = append(merged, v) + } + } + + total := ecDataShards(anchor) + ecParityShards(anchor) + width := total + for _, v := range merged { + if n := int(v.Shards[len(v.Shards)-1].ShardId) + 1; n > width { + width = n + } + } + slots := make([]*EcVolumeShard, width) + for _, v := range merged { + for _, shard := range v.Shards { + id := int(shard.ShardId) + if id >= len(slots) { + continue + } + if slots[id] == nil { + slots[id] = shard + } + } + } + + var skipped []string + for pos, v := range runtimes { + if v.EncodeTsNs != anchorGen { + skipped = append(skipped, fmt.Sprintf( + "EC volume %d shards at %s (position %d) belong to encode run %d but the scrub anchors on %d; they were not verified", + v.VolumeId, v.dir, pos, v.EncodeTsNs, anchorGen)) + continue + } + if !geometryMatches(v, anchor) { + skipped = append(skipped, fmt.Sprintf( + "EC volume %d shards at %s (position %d) share encode run %d but disagree on geometry (%d+%d bs %d vs %d+%d bs %d); they were not verified", + v.VolumeId, v.dir, pos, v.EncodeTsNs, + ecDataShards(v), ecParityShards(v), ecBlockSize(v), + ecDataShards(anchor), ecParityShards(anchor), ecBlockSize(anchor))) + } + } + + return &MergedEcRuntimes{ + Anchor: anchor, + Merged: merged, + Slots: slots, + Skipped: skipped, + } +} + +func geometryMatches(a, b *EcVolume) bool { + return ecDataShards(a) == ecDataShards(b) && + ecParityShards(a) == ecParityShards(b) && + ecBlockSize(a) == ecBlockSize(b) +} + +func ecDataShards(v *EcVolume) int { + if v.ECContext != nil { + return v.ECContext.DataShards + } + return 0 +} + +func ecParityShards(v *EcVolume) int { + if v.ECContext != nil { + return v.ECContext.ParityShards + } + return 0 +} + +func ecBlockSize(v *EcVolume) int64 { + if v.ECContext != nil { + return v.ECContext.BlockSize + } + return 0 +} + +// asVolume returns a synthetic EcVolume that shares the anchor's volume-level +// state (ecx handles, version, geometry, bitrot protection) but presents the +// merged shard set. ScrubLocal/ChecksumScrub read Shards, FindEcVolumeShard, +// ECContext, ecxFile, BitrotProtection and Version — all of which the anchor +// supplies except Shards, which is rebuilt from the merged slots. +// +// For legacy volumes (no datFileSize in .vif), LocateEcShardNeedleInterval +// derives the shard size from Shards[0].ecdFileSize. The merged shard set is +// compacted in shard-ID order, so a truncated lowest-ID shard would shrink +// every interval and misread intact sibling shards. To prevent that, asVolume +// synthesizes a datFileSize from the maximum mounted shard size when the +// anchor lacks one, so the datFileSize>0 path in LocateEcShardNeedleInterval +// uses the largest shard's size across all merged runtimes. +func (m *MergedEcRuntimes) asVolume() *EcVolume { + anchor := m.Anchor + shards := make([]*EcVolumeShard, 0, len(m.Slots)) + for _, s := range m.Slots { + if s != nil { + shards = append(shards, s) + } + } + datFileSize := anchor.datFileSize + if datFileSize == 0 && anchor.ECContext != nil && anchor.ECContext.DataShards > 0 { + var maxShardSize int64 + for _, s := range shards { + if s.ecdFileSize > maxShardSize { + maxShardSize = s.ecdFileSize + } + } + // Subtract 1 to match the legacy fallback in LocateEcShardNeedleInterval + // (ecdFileSize - 1): an exact large-block boundary is ambiguous, and + // the unadjusted size would select an extra large row. + if maxShardSize > 0 { + datFileSize = (maxShardSize - 1) * int64(anchor.ECContext.DataShards) + } + } + return &EcVolume{ + VolumeId: anchor.VolumeId, + Collection: anchor.Collection, + dir: anchor.dir, + dirIdx: anchor.dirIdx, + ecxActualDir: anchor.ecxActualDir, + ecxFile: anchor.ecxFile, + ecxFileSize: anchor.ecxFileSize, + ecxCreatedAt: anchor.ecxCreatedAt, + Shards: shards, + Version: anchor.Version, + diskType: anchor.diskType, + datFileSize: datFileSize, + ECContext: anchor.ECContext, + EncodeTsNs: anchor.EncodeTsNs, + bitrot: anchor.bitrot, + bitrotStatus: anchor.bitrotStatus, + } +} + +// ScrubLocal checks the integrity of local shards across every merged +// runtime, mirroring EcVolume.ScrubLocal over the merged shard set. Skipped +// runtimes are reported alongside any scrub errors. +func (m *MergedEcRuntimes) ScrubLocal() (int64, []*volume_server_pb.EcShardInfo, []error) { + files, shardInfos, errs := m.asVolume().ScrubLocal() + for _, s := range m.Skipped { + errs = append(errs, fmt.Errorf("%s", s)) + } + return files, shardInfos, errs +} + +// ChecksumScrub verifies every merged runtime's local shards against the +// bitrot sidecar, mirroring EcVolume.ChecksumScrub. The sidecar is per-DISK +// state: protection is taken from the first merged runtime that has any (On, +// else Invalid, else the anchor's Off), and every merged runtime whose +// sidecar resolved Invalid is reported even when protection is taken from a +// sibling that is On. Skipped runtimes are reported alongside any scrub +// errors. +func (m *MergedEcRuntimes) ChecksumScrub() (int64, []*volume_server_pb.EcShardInfo, []error) { + // Pick the protection source: first On, else first Invalid, else anchor. + var protectionSource *EcVolume + for _, v := range m.Merged { + if _, status := v.BitrotProtection(); status == BitrotOn { + protectionSource = v + break + } + } + if protectionSource == nil { + for _, v := range m.Merged { + if _, status := v.BitrotProtection(); status == BitrotInvalid { + protectionSource = v + break + } + } + } + if protectionSource == nil { + protectionSource = m.Anchor + } + + prot, status := protectionSource.BitrotProtection() + + // Fence the sidecar's encode generation: a merged runtime can load a + // sidecar from a sibling metadata directory (ReloadBitrotSidecar), and + // the merge fence may then exclude the runtime owning that directory. + // Generation-0 sidecars do not identify the encode run, so geometry + // validation alone cannot prove the borrowed manifest describes the + // anchor's shards. If the sidecar records a non-zero EncodeTsNs that + // disagrees with the anchor's, scanning would apply stale checksums to + // current shards and report false corruption. Refuse instead. + if status == BitrotOn && prot != nil && prot.EcShardConfig != nil { + sidecarGen := prot.EcShardConfig.EncodeTsNs + if sidecarGen != 0 && m.Anchor.EncodeTsNs != 0 && sidecarGen != m.Anchor.EncodeTsNs { + errs := []error{fmt.Errorf( + "ec volume %d: bitrot sidecar at %s records encode run %d but the scrub anchors on %d; protection is unverifiable", + m.Anchor.VolumeId, protectionSource.dir, sidecarGen, m.Anchor.EncodeTsNs)} + for _, rt := range m.Merged { + if rt == protectionSource { + continue + } + if _, s := rt.BitrotProtection(); s == BitrotInvalid { + errs = append(errs, fmt.Errorf( + "ec volume %d bitrot sidecar at %s is malformed/unverifiable (sidecar integrity)", + rt.VolumeId, rt.dir)) + } + } + for _, s := range m.Skipped { + errs = append(errs, fmt.Errorf("%s", s)) + } + return 0, nil, errs + } + } + + // Run the byte scan against the protection source's sidecar, but over + // the merged shard set. The synthetic volume inherits the protection + // source's bitrot state so ChecksumScrub's BitrotOff/Invalid arms fire. + v := m.asVolume() + v.bitrot = prot + v.bitrotStatus = status + + blocks, broken, errs := v.ChecksumScrub() + + // Collect Invalid-sidecar errors from every merged runtime except the + // protection source (whose error the Invalid arm already reports), so a + // malformed sidecar is never silently discarded when a sibling is On. + if status != BitrotInvalid { + for _, rt := range m.Merged { + if rt == protectionSource { + continue + } + if _, s := rt.BitrotProtection(); s == BitrotInvalid { + errs = append(errs, fmt.Errorf( + "ec volume %d bitrot sidecar at %s is malformed/unverifiable (sidecar integrity)", + rt.VolumeId, rt.dir)) + } + } + } + for _, s := range m.Skipped { + errs = append(errs, fmt.Errorf("%s", s)) + } + return blocks, broken, errs +} diff --git a/weed/storage/store_ec_scrub.go b/weed/storage/store_ec_scrub.go index 4a5c11169..ce236cf3f 100644 --- a/weed/storage/store_ec_scrub.go +++ b/weed/storage/store_ec_scrub.go @@ -25,7 +25,50 @@ func (s *Store) ScrubEcVolume(vid needle.VolumeId, mode volume_server_pb.VolumeS if err := s.cachedLookupEcShardLocations(ecv); err != nil { return 0, nil, []error{fmt.Errorf("failed to locate shard via master grpc %s: %v", s.MasterAddress, err)} } + return s.scrubEcVolumeWalk(ecv, mode, forceDeletedNeedlesCheck) +} +// ScrubEcVolumeMerged is the merged-runtime entry point for FULL/READS. It +// resolves the runtime matching the anchor's encode generation rather than +// the first disk's, so the needle walk and the parity phase inspect the same +// encode run. Skipped runtimes are reported alongside any scrub errors. +func (s *Store) ScrubEcVolumeMerged(merged *erasure_coding.MergedEcRuntimes, mode volume_server_pb.VolumeScrubMode, forceDeletedNeedlesCheck bool) (int64, []*volume_server_pb.EcShardInfo, []error) { + anchor := merged.Anchor + expectedEncodeTs := anchor.EncodeTsNs + + // Resolve the runtime matching the anchor's encode generation, not the + // first-match FindEcVolume — otherwise the needle walk can scan an older + // run while the merged view anchored on the newest. + var ecv *erasure_coding.EcVolume + if expectedEncodeTs != 0 { + for _, v := range s.FindAllEcVolumes(anchor.VolumeId) { + if v.EncodeTsNs == expectedEncodeTs { + ecv = v + break + } + } + } + if ecv == nil { + var found bool + ecv, found = s.FindEcVolume(anchor.VolumeId) + if !found { + return 0, nil, []error{fmt.Errorf("EC volume id %d not found", anchor.VolumeId)} + } + } + if err := s.cachedLookupEcShardLocations(ecv); err != nil { + return 0, nil, []error{fmt.Errorf("failed to locate shard via master grpc %s: %v", s.MasterAddress, err)} + } + + files, shardInfos, errs := s.scrubEcVolumeWalk(ecv, mode, forceDeletedNeedlesCheck) + for _, sk := range merged.Skipped { + errs = append(errs, fmt.Errorf("%s", sk)) + } + return files, shardInfos, errs +} + +// scrubEcVolumeWalk is the per-needle local+remote walk shared by ScrubEcVolume +// and ScrubEcVolumeMerged. +func (s *Store) scrubEcVolumeWalk(ecv *erasure_coding.EcVolume, mode volume_server_pb.VolumeScrubMode, forceDeletedNeedlesCheck bool) (int64, []*volume_server_pb.EcShardInfo, []error) { // full scan means verifying indexes as well _, errs := ecv.ScrubIndex()