mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-09 07:47:54 +02:00
dfbdc864a0bb516cbdf90e5441a520642f5960e7
93
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
adaf3534fa |
rust: clippy-clean both crates and adopt the std APIs the 1.91 MSRV allows (#11312)
* rust: apply clippy --fix to both crates The mechanical part of a clippy sweep: `cargo clippy --all-targets --fix` on seaweed-volume and the seaweed-worker workspace, hand-reviewed. Both manifests declare their MSRV (1.91.1 and 1.94.1), so every suggestion clippy applied is within it: the collapsible_if sites become let chains (1.88, edition 2024), `% n == 0` becomes is_multiple_of (1.87), chunks_exact with a constant becomes as_chunks (1.88), repeat().take() becomes repeat_n (1.82), and io::Error::new(Other, ..) becomes io::Error::other (1.74). The rest is redundant clones, borrows, casts, closures and field names. Nothing here changes behaviour. The three let_and_return sites in needle_map.rs and store_ec.rs deserve a note: the `let result = ..; result` shape was a deliberate edition-2021 workaround to drop a redb guard before the table it borrows. Edition 2024 drops tail-expression temporaries before locals, which is why clippy now flags it, and the two comments that described the workaround say so instead. Manual edits on top of the tool output: the blocks clippy rewrote are re-indented the way rustfmt lays them out (only those blocks — the crate is not rustfmt-clean and a whole-crate fmt would bury this diff), the blank lines let_and_return left behind are removed, and the CRC legacy_value test compares against a literal worked out from the original shift formula rather than restating rotate_right. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CjZY429aVU74SLDmo1wiuU * rust: clear the clippy warnings --fix cannot apply, and say why the rest stay Hand fixes for the lints clippy only reports. Behaviour is unchanged throughout; each rewrite is the one clippy names. - needless_range_loop (7): index loops over shard vectors become iterator loops. Where the old code indexed `v[..n]` the new loop iterates `v[..n]` so an undersized vector still panics the same way. - field_reassign_with_default (6): struct literals with `..Default`. - redundant_pattern_matching (3): `if let Err(_) = guard.check()` becomes `.is_err()`, which also releases the read guard at the end of the condition instead of at the end of the block. - manual_strip (2), manual_checked_ops, format_in_format_args, redundant_locals, wrong_self_convention (to_vif takes self by value, so it is into_vif; CompactEntry is Copy, so to_needle_value takes self). - type_complexity (2): `OrphanShardLoad` and `RawNeedleEntry` name two tuples that were spelled out inline. - new_without_default: CompactNeedleMap gets a Default that calls new(). - suspicious_open_options: a test helper spells out `.truncate(false)`, which is what `.create(true).write(true)` already did. What stays, and the attribute that says so: - too_many_arguments (10): `#[expect]` on each function. Folding 8–15 parameters into a struct is a design change, not a lint fix. - await_holding_lock / readonly_write_lock: one test holds the store write guard across a sleep on purpose, as a barrier that parks the copy task at the mount block. `#[expect(.., reason = ..)]` records it. - module_inception: needle/needle.rs mirrors the Go package layout. Two lints become crate-wide policy in `[lints.clippy]`, with the reason next to each: result_large_err, because every RPC path returns tonic::Status (176 bytes) and boxing it would change every handler signature; and needless_update, because `..Default::default()` on a protobuf message literal is what lets a proto gain a field without touching every constructor (all 11 sites are pb messages). The worker workspace gets the same table and its members opt in with `lints.workspace = true`; its generated plugin.rs also allows large_enum_variant on prost's oneof enums. Both crates are now clean under `cargo clippy --all-targets -- -D warnings`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CjZY429aVU74SLDmo1wiuU * rust volume: use the std APIs the 1.91 MSRV already pays for The crate declares rust-version 1.91.1, so a few things the code still worked around are plain std now. All of them come from the 1.85–1.91 release notes; nothing here needs a newer toolchain than the manifest already requires. - std::sync::LazyLock (1.80) replaces the lazy_static! block in metrics.rs, and the lazy_static dependency goes. Every use site reads the same through Deref, so no caller changes. - Duration::from_mins / from_hours (1.91) replace `from_secs(v * 60)` and `from_secs(v * 3600)` in the option parser and the shard-location refresh TTLs. One difference for the parser: an absurd count that overflows u64 seconds now panics in release builds too, where the multiplication used to wrap. - Result::flatten (1.89) replaces `.and_then(|r| r)` on the replication join handle. - OsStr::display (1.87) replaces `to_string_lossy()` where the name was only being formatted; the output is byte-identical. - `#[allow]` becomes `#[expect]` (1.81) on the suppressions that are meant to be permanent, so a suppression that stops being needed becomes a warning rather than lingering. Doing that found four that already had: dead_code on ChunkManifest, base_name and last_io_error, and too_many_arguments on read_from_data_shards, which is down to seven parameters. Those attributes are deleted. The three allows that depend on cfg (a unix-only mutation, a linux-only field set, a profiling-only parameter) stay as allow, because expect would be unfulfilled on the other platforms. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CjZY429aVU74SLDmo1wiuU * ci: add a commented-out clippy step to both Rust workflows Both crates are warning-free under `cargo clippy --all-targets -D warnings` now. Whether that becomes a gate is a policy call, so the step is present but commented out; uncommenting it is the whole change. The comment points at the `[lints.clippy]` table where crate-wide exceptions are recorded, so the gate does not become a reason to sprinkle allows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CjZY429aVU74SLDmo1wiuU * rust volume: guard parse_duration against overflow panics Duration::from_mins/from_hours panic when the count overflows u64 seconds. Use checked_mul so an oversized CLI value falls back to the parser default instead of crashing volume startup. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
5b2fe374fc |
[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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUf2cmVKHNhAZPTNv39rDE * ec: make the scrub plan tests falsifiable Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> 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) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
3ae9e332ec |
rust volume: honour is_last in the tail sender instead of rescanning the whole volume (#11273)
* rust volume: honour is_last in the tail sender instead of rescanning
volume_tail_sender discarded the is_last flag from
binary_search_by_append_at_ns:
Ok((offset, _is_last)) => {
if offset.is_zero() { Ok(sb_size) } ...
is_last means the caller is already caught up. Go answers that with a
heartbeat and does not scan at all (volume_grpc_tail.go, `if isLastOne`).
Dropping it is expensive rather than untidy, because the branches interact:
when the search reports caught-up it returns Offset::default(), which is
zero, so the start offset falls back to sb_size -- the beginning of the
data -- and scan_raw_needles_from materialises every needle from there to
EOF into a Vec. The timestamp filter discards all of it, the loop sleeps
2s, and it happens again.
A volume being moved is marked read-only before the copy, so it is ALWAYS
caught up during the tail phase. Measured on one volume.move of a 2.15 GB
volume, sampling the source's cgroup anon every 2s against the move's own
phase output:
copying 16 -> 37 MB CopyFile streams correctly, stays bounded
tailing 904 -> 2166 -> 629 -> 2166 -> 342 -> 2173 -> 2179 MB
deleting 46 MB
Six full-volume allocate/free cycles in 35s, peak 2179 MB against a volume
of 2147 MiB. The destination never exceeded 35 MB, so this is entirely
source-side. Under a per-process memory cap it OOM-kills the source
whenever the volume exceeds the cap.
The ordering here is the whole fix and is easy to get wrong: resolve the
start offset and is_last under a brief lock, return the heartbeat
immediately when caught up, and only then reach the scan. An earlier cut
set the flag correctly but placed the early return after the block that
performs the scan -- the heartbeat fired and the destination received
nothing, yet every iteration still read the whole volume and discarded it.
Production showed no improvement (1770 MB across five cycles), which is
what caught it. The binary search is over the .idx and costs nothing; the
scan is the expensive part and must not run speculatively.
Three tests, and the last two matter as much as the first: a fix that
always reported "caught up" would make tailing silently lose needles, a
worse bug than the one being fixed. One asserts is_last for a caller at or
beyond the newest append_at_ns; one asserts NOT is_last for a caller that
is behind, so real tail data is still scanned and shipped; one asserts NOT
is_last when the only newer record is a delete, and that scanning from the
returned offset ships exactly that tombstone.
Left deliberately unfixed, and worth separate changes: the scan still
collects into a Vec rather than streaming through a visitor as Go's
ScanVolumeFileFrom does, and it runs while holding store.read(), the same
lock-across-a-large-read shape as #11235. Both are latent once the rescan
is gone, since remaining scans are bounded by genuinely new data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MFr2v4BUqrXdgj4LEUAwVF
Claude-Session: https://claude.ai/code/session_018VF7E9SHPihG1jC1grU9H3
* rust volume: resolve and scan the tail under one store guard
The tail sender took store.read() once for the binary search and again
for the scan. A vacuum commit takes the store write lock and swaps
.dat/.idx, so it could land between the two: the offset resolved against
the old files would then be applied to the new ones and start the scan
inside an unrelated record. The code before the is_last fix held a
single guard for both. Restore that, and scan only when the caller is not
caught up, so the caught-up heartbeat still skips the scan and is sent
outside the lock.
Also pin the compacted-volume boundary raised in review. Compaction
writes .idx in needle-id order in both Go and Rust, so the search can
report caught-up while an earlier row is newer; such a caller's since_ns
is the last row's timestamp, so those rows were in the files it copied.
A write made afterwards is appended as the final row, which the search
cannot step past. The new test asserts it still reaches the scan.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018VF7E9SHPihG1jC1grU9H3
* rust volume: make the compaction tail test a genuine overwrite
The compaction regression test's second id=1 write reused the first
write's data, so write_needle's dedup short-circuit (is_file_unchanged)
returned without appending or updating append_at_ns. Compaction then
kept key 1's original (older) timestamp, so the test passed without
exercising the overwrite it describes -- key 2 was the final row only
because key 1 was never actually newer.
Give the overwrite distinct data so it appends a new record, and assert
key1_ns > key2_ns up front so a future dedup regression fails the test
instead of silently hollowing it out. Trim the verbose comments on the
tail sender and the binary-search tests to their essentials.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
|
||
|
Feng Shao
|
13bf056a15 |
Mount req with collection (#11249)
* volume mount req support specify collection * rust mirror change |
||
|
|
516e251f9e |
rust volume: move the crate to edition 2024 (#11244)
* rust volume: move the crate to edition 2024 Edition 2024 turns three things in this crate into hard errors, and changes drop order in a further 34 places without changing compilation. The compiler errors are fixed here; the silent changes were audited against `RUSTFLAGS='-W rust-2024-compatibility' cargo check --all-targets` output captured before the flip, since edition 2024 stops reporting them. `std::env::set_var`/`remove_var` are unsafe as of 2024 because they race with concurrent readers. All six call sites are safe by construction rather than by assertion, and the SAFETY comments say why: the build script runs single-threaded before anything else in the process, and every test reaching the `config.rs` helpers holds `process_state_lock()` for the duration. The two `ref` bindings in handlers.rs sit in patterns that already borrow implicitly, so removing the modifier leaves both bindings at `&String`. On the 34 drop-order sites: no lock guard's scope is extended anywhere, and `volume.rs` has none. Most are moved-from `Option`/`Result` husks — `if let Some(v) = map.remove(&k)`, `while let Some(m) = stream.next().await` — where the value is moved into the binding and the temporary has nothing left to drop; where closing order actually matters these paths already call `v.close()`, `ec_vol.destroy()` or `drop(writer)` explicitly. Two sites get strictly better ordering: the metrics read guard in `run_metrics_push_loop` shrinks to the end of its initializer block (it never crossed an `.await` either way), and an EC test now closes the volume's descriptors before the `TempDir` removes the directory. No `rust-version` is declared. Edition 2024 needs rustc 1.85, but that is not the binding constraint — the dependency tree already requires 1.91.1 through the `aws-sdk-s3`/`aws-smithy-*` family, so `cargo +1.85 check` fails on the deps regardless. CI builds on `dtolnay/rust-toolchain@stable`. `vendor/reed-solomon-erasure` is a separate package and keeps edition 2021. Cargo.lock is unchanged despite edition 2024 implying resolver 3. Verified: `cargo test` 551 passed / 0 failed, `cargo test --no-default-features` 550 passed / 0 failed (the two feature sets produce an identical migration site list), `cargo build --release` clean. No automated test covers shutdown ordering, so the channel and runtime sites in `main.rs`, `write_queue.rs` and `grpc_server.rs` were read individually. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018nty5Rj7ssMQdFxHHjZgDC * rust volume: address edition-2024 review feedback Three fixes from review of the edition bump. Serialize the two environment-reading tests. The SAFETY comments on the `env::set_var`/`remove_var` helpers claim every test touching the environment holds `process_state_lock()`, but `test_resolve_config_defaults_dir_to_platform_temp_dir` and `test_resolve_config_index_accepts_redb_and_leveldb_aliases` called `resolve_config` — which reads HOME/USERPROFILE, SEAWEED_WRITE_QUEUE and the WEED_* set — without taking it. `set_var` is unsafe precisely because a concurrent *reader* is UB, not only a concurrent writer, so the comment was overclaiming. An audit of the module found exactly these two; every other environment-touching test already held the lock. The race predates edition 2024, which only made the requirement explicit. Declare `rust-version = "1.91.1"`. The edition needs 1.85, but that was never the binding constraint: `cargo +1.90 check --all-targets` fails on the `aws-sdk-s3`/`aws-smithy-*` family, and 1.91.1 checks clean. Declaring the verified floor turns a wall of per-dependency errors into one clear message. Cargo.lock is unchanged despite this making the resolver MSRV-aware. Update the README, which advertised "Rust 1.75+ (2021 edition)". 1.75 was already stale before this branch — the tree has needed 1.91 for a while. Verified: `cargo test` 551 passed / 0 failed, `cargo test --no-default-features` 550 passed / 0 failed, `cargo build --release` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018nty5Rj7ssMQdFxHHjZgDC * rust volume: state the exact MSRV patch release in the README The README said "Rust 1.91+", which reads as 1.91.0 and is wrong by one patch release: `cargo +1.91.0 check --all-targets` fails on the aws-sdk-s3 family, `cargo +1.91.1` passes. Say 1.91.1+, matching `rust-version` in Cargo.toml, and call out that the patch component is load-bearing so nobody installs 1.91.0 and hits the same wall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018nty5Rj7ssMQdFxHHjZgDC * rust worker: move the workspace to edition 2024 Moves the seaweed-worker workspace (core, lance, sort) from edition 2021 to 2024, the same migration seaweed-volume just got in this branch. Edition 2024 turns exactly one thing in this workspace into a hard error. The baseline came from RUSTFLAGS='-W rust-2024-compatibility' cargo check --all-targets, run before the flip; unlike seaweed-volume's 34 silent + 8 hard sites, the worker reports only the one hard site and no tail_expr_drop_order or if_let_rescope sites at all. The worker is a much smaller crate and none of its expressions hold a guard or temporary whose drop order the edition changes, so there is nothing to audit on the silent side. Fixed (1 site): std::env::set_var is unsafe as of 2024 because it races with concurrent readers. The single call is in crates/core/build.rs, which sets PROTOC from protoc_bin_vendored the way seaweed-volume's build script does. A build script's main runs single-threaded before anything else in the process, so no other thread can be reading the environment concurrently; the SAFETY comment says so. There are no config.rs-style test helpers here -- the worker's tests do not mutate the environment -- so unlike the volume crate there are no process_state_lock() callers to audit. No redundant ref bindings to clean up: a grep for ref across the three crates finds none. MSRV: rust-version = "1.94.1", verified rather than inferred. Edition 2024 only needs 1.85, but the dependency tree needs more: lance's aws feature pulls in a newer cut of the same aws-sdk-*/aws-smithy-* family that sets seaweed-volume's 1.91.1 floor, and that newer cut requires 1.94.1. cargo +1.94.0 check --all-targets fails on that family; cargo +1.94.1 check --all-targets is clean. The worker's floor is therefore higher than the volume's, and moves with lance and the AWS SDK rather than with the edition. CI builds on dtolnay/rust-toolchain@stable, so nothing changes there. The edition is set once in [workspace.package] and inherited by each member via edition.workspace = true; rust-version is added the same way. The workspace keeps its explicit resolver = "2" -- edition 2024 would default to resolver 3, but the pin is deliberate and Cargo.lock is unchanged by this commit either way. The README gains a "Requires Rust 1.94.1+ (2024 edition)" line in its Building section, matching the one seaweed-volume's README now carries, and calling out that the patch release is load-bearing (1.94.0 does not build) so nobody installs 1.94.0 and hits the same wall. Verification: * cargo check --all-targets -- clean, zero warnings (default toolchain 1.97) * cargo +1.94.1 check --all-targets -- clean * cargo +1.94.0 check --all-targets -- fails on the AWS SDK, as claimed * cargo test --all-targets -- 40 passed, 0 failed (core 13, sort 11, lance lib 3, lance bin 2, compaction 6, lifecycle 1, sort integration 4) * Cargo.lock unchanged Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
2ffa696809 |
fix(volume): handle faulty storage media (Go + Rust) (#11233)
* fix(volume): track EC shard read errors and unmount on faulty media Extract the volume EIO tracker into a reusable IoErrorTracker and add the same tracking to EcVolume. Sustained EIO on .ecx lookups or .ecd shard reads now unmounts the EC volume in the heartbeat (without deleting files) so the master re-replicates from healthy peers, mirroring the existing volume replica quarantine. Closes #11227 (EC shard unmount). * rust(volume): mirror EC shard read error tracking and unmount Add EIO tracking to the Rust EcVolume mirroring Go: a streak counter with IO_ERROR_TOLERANCE, a sticky quarantine flag, and unmount (not file deletion) in the heartbeat so the master re-replicates from healthy peers. * feat(metrics): expose storage IO error counter and quarantine gauge Add a storage_io_error_total counter incremented on every EIO recorded by the volume or EC shard tracker, and an io_quarantine gauge labelled by kind (volume/ec_shard) reflecting the count of replicas suppressed in the heartbeat. Mirrored in Go and Rust. * feat(healthz): report 503 when local replicas are IO-quarantined Add Store.HasIoQuarantine (Go) / Store::has_io_quarantine (Rust) and have /healthz return 503 when any local volume or EC shard is quarantined due to sustained storage-media EIO, so a load balancer can drain a server whose underlying media is faulty. Mirrored in Go and Rust. * fix(volume): keep quarantined EC volumes in memory and reset EIO on success Address review feedback: instead of unloading quarantined EC volumes (which discards the quarantine state healthz needs), keep them in memory and just skip them from heartbeat reporting, mirroring the regular volume quarantine. Also clear the EIO streak on successful .ecx reads in Rust so a transient error does not accumulate, and add an ec_shard label to the io_quarantine gauge in both Go and Rust. * fix(volume): exclude quarantined EC shards from heartbeat and add Rust volume tolerance Address review feedback: - Filter quarantined EC volumes from CollectErasureCodingHeartbeat (Go) and collect_ec_shard_delta_messages / collect_live_ec_shards (Rust) so the master stops advertising faulty shards and re-replicates from healthy peers. - Add consecutive EIO count and sticky quarantine to the Rust regular Volume, mirroring Go IoErrorTracker: a single EIO no longer deletes the replica; the heartbeat quarantines after the tolerance threshold and keeps the volume in memory. - Use the quarantine flag (not last_io_error) in has_io_quarantine so /healthz reflects sustained, not transient, failures. * fix(volume): make Rust quarantined volumes read-only and wire recovery Address Devin review: - Set no_write_or_delete on Rust volumes when quarantined in the heartbeat, so cached or direct clients cannot mutate a faulty replica after the master removes it (mirrors Go). - Wire reset_io_error_state into Volume::set_writable so an operator making a volume writable again clears the sticky quarantine and the volume re-enters heartbeat rotation. * fix(volume): clear EC quarantine on shard re-mount for operator recovery Address Greptile review: re-mounting EC shards (Go loadEcShardWithIdxDir / Rust mount_ec_shards_with_idx_dir) now calls ResetIoErrorState on the existing EcVolume, giving operators a documented recovery path that clears the sticky quarantine and returns the EC volume to heartbeat rotation. Mirrored in Go and Rust. * fix(volume): do not clear EC quarantine on routine shard mounts Address review feedback: clearing the EC IO quarantine on every mount (including duplicate, retry, sibling-shard, and reconciliation mounts) is too aggressive and can re-advertise known-bad shards before the storage media has been validated. Remove the automatic reset from the mount path; quarantine clears naturally on restart or full unmount when a fresh EcVolume is created with clean state. * test(volume): update Rust IO error test for quarantine semantics The heartbeat now quarantines a volume with sustained EIO (keeps it mounted, makes it read-only, omits it from heartbeat) instead of deleting it. Update test_collect_heartbeat_deletes_io_error_volume to assert the volume stays in the store with no_write_or_delete set, and update set_last_io_error_for_test to set the consecutive error count at the tolerance threshold so the test reflects a sustained error. * fix(volume): reset EIO streak after full write and match Windows media errors Move the success-side EIO reset from append_needle (after write_all only) to the end of do_write_request, after flush_dat/flush_idx complete, so a successful write_all followed by a failed fsync no longer resets the counter before the EIO is recorded. Repeated fsync EIOs now accumulate toward the quarantine threshold as intended. Recognize Windows storage-media failure codes ERROR_CRC (23) and ERROR_IO_DEVICE (1117) in addition to Unix EIO (errno 5), so quarantined heartbeat behavior is preserved on Windows. Mirrors the change in both Go and Rust volume servers. * fix(volume): preserve checkpoint EIO and clear streak on successful delete maybe_checkpoint_index now returns whether the checkpoint succeeded; the success-side EIO reset in do_write_request and do_delete_request only fires when it did, so a checkpoint media failure is no longer erased by the unconditional reset that followed it. do_delete_request also gains the success reset that was lost when append_needle stopped clearing the streak, so a successful delete still clears an earlier failure streak. is_storage_io_error now uses libc::EIO on Unix instead of a hard-coded 5, and the ECX binary-search read path gains a Windows fallback (seek + read_exact) so the buffer is no longer zeroed on non-Unix targets. |
||
|
|
9b12d13934 |
volume server: release the store lock before scrubbing EC volumes (#11235)
* volume server: release the store lock before scrubbing EC volumes
`ec.scrub` makes a Rust volume server stop serving for the duration of the
scrub, and then kills its own gRPC connection:
error: rpc error: code = Unavailable desc = keepalive ping failed to
receive ACK within timeout
Measured on a 4.46 cluster (17 Rust volume servers on one host, ~520 volumes
and 53 EC volumes, --index=redb, EC 10+4). It reproduces against a SINGLE
node in 30-70s, in checksum, index and local modes, at -maxParallelization 1.
## Cause
The CHECKSUM arm of scrub_ec_volume reads every byte of every local shard
while holding the caller's store.read() guard:
let store = self.state.store.read().unwrap();
let ecv = store.find_ec_volume(vid)...?;
let (blocks, broken, errs) = ecv.checksum_scrub(); // GBs of I/O, lock held
VolumeServerState::store is a std::sync::RwLock, which is write-preferring.
The periodic heartbeat's collect_heartbeat_with_snapshot takes store.write()
and blocks; once that writer is pending, every later store.read() queues
behind it. Every HTTP handler takes store.read(), so the node serves nothing,
stops heart-beating, and cannot answer the scrub RPC's own keepalive - the
scrub kills the connection it is running on.
The INDEX and LOCAL arms have the same shape, and the node-wide scrub_volume
loop is worse: it held ONE guard across every volume on the node.
## Evidence
offcputime, off-CPU stacks >1s in a 30s window during a scrub:
futex_wait
seaweed_volume::server::heartbeat::collect_heartbeat_with_snapshot
- tokio-rt-worker
27967020 <- 27.97s blocked, of a 30s window
A single HTTP /status request issued 12s into a scrub, with 180s of patience,
was accepted and queued for 120 seconds, then served once the scrub released.
Thread states throughout: 1 D + 48 S. One thread working, 48 idle - not
executor starvation and no thread pileup, which is what a single lock holder
looks like.
Memory was tested and ruled out as the cause: the same scrub was run at
MemoryMax 3G, 8G and unlimited. With no limit there is no reclaim at all,
page cache grows freely to 22 GB, and the node still goes unresponsive at
t+30s. anon stays flat at 48-86 MB in every run.
## Fix
checksum_scrub, scrub_index and scrub_local gain plan types -
EcChecksumScrubPlan, EcIndexScrubPlan and EcLocalScrubPlan - snapshotted from
the volume under a brief guard. The handler builds a plan, drops the guard,
and runs the scan in spawn_blocking, off the async workers, since it is
synchronous CPU + file I/O either way.
A plan captures DESCRIPTORS, not paths. Resolving a path again after the
guard is dropped would let a writer that legitimately unlinks the files - the
heartbeat's delete_expired_ec_volumes, which reaches EcVolume::destroy(), or
volume_ec_shards_delete - surface an intentional removal as "scrub read
error: No such file or directory" and put the volume in broken_volume_ids. A
descriptor outlives the name.
For the shards it duplicates the handle the mounted EcVolumeShard already
holds (try_clone_file), which is what Go does: ChecksumScrub reads through
shard.ReadAt (weed/storage/erasure_coding/ec_volume_scrub.go:71), never
through a path. That also inherits open_volume_file's O_NOATIME and drops a
dead branch - the old code built {base}.ec{id}.v{gen} for a non-zero
generation, a name nothing in this tree writes. dup shares the kernel offset,
so shard reads stay positional; the .ecx gets a fresh open instead, since
check_index_file seeks.
FULL/READS is unchanged here: it already released the guard across the index
walk, and still re-takes it per needle in store_ec::scrub_snapshot_under_lock
for that needle's local shard intervals - short holds, many of them.
scrub_volume now takes the read guard PER VOLUME instead of across the whole
loop, so the heartbeat can land between volumes. Its per-volume work still
runs under the guard; Volume needs an equivalent plan to fix that properly,
left as a follow-up and noted in the code.
## A failed scrub task must not take the whole RPC down
Moving the scans into spawn_blocking changed where a panic lands. It no
longer unwinds inside the handler's own future; it comes back as a JoinError
at the .await, and all four join points sat behind a `?`. So one bad volume
out of six hundred returned Err from the entire handler: the
broken_volume_ids, broken_shard_infos and details already gathered for the
other 599 were dropped, and emit_scrub_metrics - the only writer of
SCRUB_LAST_TIME_SECONDS, SCRUB_VOLUME_FAILURES and SCRUB_SHARD_FAILURES - was
never reached, so the staleness alert kept firing while real corruption went
unreported.
And there is a reachable panic behind it. EcLocalScrubPlan::run() sized its
reassembly buffer with
Vec::with_capacity(get_actual_size(size, version) as usize)
which for any negative size that is not the -1 tombstone skipped above is a
capacity-overflow abort. Mode 3 (LOCAL) is the default of `weed shell
ec.scrub`, and a scrub is what you point at an index you already suspect, so
an arbitrary i32 in a .ecx size field is in-scope input. The buffer is
Rust-only - Go appends to a nil slice and has no capacity hint here. Guard on
`want <= 0` and fall through with an empty buffer: locate_data returns no
intervals for a non-positive size, read stays 0, and the existing
`read != want` error reports the row exactly as Go does.
Each join point now records the failure against its own volume and continues.
A panic is evidence about the volume and counts as broken; a non-panic
JoinError is not - spawn_blocking only reports one when the runtime is going
down, the volume was never scanned, and counting it would put a false
corruption into SCRUB_VOLUME_FAILURES. total_volumes moves before the join in
modes 1, 3 and 4 (2|5 already counted there) so a failed join cannot silently
shrink it. Mode 2|5's verify_ec_shards join is the one that must not
`continue`: the needle walk above has already produced findings for that
volume.
The tombstone guard stays is_tombstone() on purpose. ScrubLocal in
ec_volume_scrub.go:228 skips only IsTombstone(), while the distributed walk
in store_ec.go:516 skips all IsDeleted() - the asymmetry is Go's, and both
Rust walks mirror their own counterpart.
## Both servers: a node-wide scrub skips a volume that vanished mid-run
Releasing the lock makes the volume set legitimately mutable during a scrub,
so a node-wide run can reach a volume that has since been unmounted. That is
not a scrub failure. A node-wide run now logs and skips it; an explicitly
requested volume id still returns NotFound. The Go server is changed the same
way, so both implementations answer the same shell command identically.
mark_broken_volumes_readonly tolerates the same teardown one step later,
instead of throwing away the whole scrub report.
## Test
test_scrub_plans_are_self_contained_and_match_direct_call drops the EcVolume
and runs both plans on another thread, asserting the results match the direct
calls. A plan that borrowed from EcVolume could do neither, so the test stops
compiling if the snapshot regresses to a borrow.
test_scrub_plans_survive_files_removed_after_snapshot unlinks every shard and
the .ecx after the plans are built, then asserts the results still equal the
direct call. Against a path-resolving version it fails with all 14 shards
reported as "No such file or directory".
test_local_scrub_plan_reports_negative_size_ecx_row rewrites a .ecx row's
size to -1000 and runs the local plan on another thread, so the join is the
assertion - that thread is the spawn_blocking whose panic used to fail the
RPC. Without the capacity guard it fails with "capacity overflow"; with it,
the row is reported.
The Go tests cover both halves of the vanished-volume rule for volumes and EC
volumes.
517 lib tests pass, plus 34 across the other targets (`cargo test`).
`go test ./weed/server -run Scrub` passes.
## Known remaining, not fixed here
`ec.scrub -volumeId=N` is still fanned out to every node, and a node that
holds no shard of N returns NotFound, so the shell command errors even when
the nodes that do hold shards scrub cleanly. That is a shell-side fan-out
question rather than a volume-server one, and both servers keep the existing
behaviour for an explicitly requested id.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DvHoW85w6SNKNBPvrqLMmK
* scrub: discard checksum block count from total_files; capture .ecx fd for FULL walk
Two review fixes:
1. CHECKSUM arm: plan.run() returns blocks scanned, not a file count.
Go discards it (_, shardInfos, serrs = v.ChecksumScrub()) so TotalFiles
stays a needle/file count. The Rust arm was adding it to total_files,
inflating the count. Discard it to match Go.
2. FULL/READS (scrub_ec_volume_distributed): the needle walk reopened the
.ecx by PATH after the store guard was released, so a concurrent teardown
that unlinks or replaces the .ecx (heartbeat delete_expired_ec_volumes,
volume_ec_shards_delete) could surface an intentional removal as a scrub
error or mix index generations within one scrub. Capture a second .ecx
descriptor under the guard (the index plan handle is consumed by its own
structural walk, and both seek) and read through it instead -- the same
descriptor-outlives-name invariant the checksum plan shard handles use.
* scrub: bind FULL/READS walk to one encode generation
Address Devin review: after capturing the .ecx descriptor under the guard,
scrub_snapshot_under_lock still re-resolves the volume by id per needle, so
a teardown-and-remount of the same vid between two rows would apply the
captured .ecx offsets to a replacement volume's shards -- falsely reporting
corruption.
Capture the volume's encode_ts_ns (encode-run identity) in Phase A and pass
it to scrub_snapshot_under_lock. If the mounted volume's encode_ts_ns no
longer matches, abort the walk like a mid-scan unmount instead of mixing
generations within one scrub.
* scrub: run FULL/READS index scan in the blocking pool
Address CodeRabbit review (5147767192): index_plan.run() reads the whole
.ecx synchronously, so running it on the async executor worker could block
unrelated RPC work handled on the same executor. Move it into spawn_blocking,
matching the treatment the CHECKSUM/LOCAL arms already give their plans. A
join failure (panic/cancellation) is reported as a seed error so the
per-volume findings below are not silently dropped.
* scrub: move ecx walk to blocking pool, classify join errors, guard encode_ts_ns==0
Three CodeRabbit review fixes (5148034447):
1. Move the FULL/READS needle walk (walk_index_file over the captured ecx
descriptor) into spawn_blocking. It reads the full .ecx synchronously and
was still running on the async executor worker, the same blocker the
index_plan.run() fix in the previous commit addressed.
2. Preserve JoinError classification in both spawn_blocking join points in
scrub_ec_volume_distributed. A panic is evidence about the volume and
counts as broken; a cancellation only happens at runtime shutdown, the
volume was never scanned, and returning it as an error would put a false
corruption into broken_volume_ids (the FULL/READS arm marks the volume
broken on any non-empty errs). Panics return an error; cancellations
return clean.
3. Do not treat encode_ts_ns == 0 as a verified generation match. The .vif
assigns 0 when it carries no encode-run identity (legacy/pre-feature
volumes), so 0 == 0 would accept a teardown-and-remount and apply the old
.ecx offsets to the replacement volume's shards. Only enforce the
generation check when the captured identity is non-zero; when it is zero,
fall back to the pre-check behavior (no generation binding) rather than
aborting a scrub that was already running without the guard.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
|
||
|
Chris Lu
|
15e4da65f7 |
volume: avoid read-only replica write targets (#11195)
* master: carry replica read-only state in volume lookups * volume: refresh writable replica targets * volume: preserve read-only replicas for deletes * master: propagate read-only delete capability * volume: target delete-capable replicas * volume: honor configured HTTPS for replica deletes * volume: reject insecure delete authorization forwarding * master: broadcast delete capability changes * volume: align Rust replica routing * http: protect credentialed replica redirects * master: preserve digest compatibility for delete capability * volume: propagate read-only state in short heartbeats * volume: report changed short volume state * http: guard TLS client redirects * master: announce mounted volume read-only state * volume: replace changed identity deltas * master: replace incremental volume layouts in order * master: keep moved volume lookup available * volume: announce read-only mounts |
||
|
Eliah RusinandClaude Opus 5
|
ade4bdf9e6 |
rust volume: stop a tier move whose caller has gone (#11192)
Both tier-move handlers run in a detached tokio::spawn and report progress through a closure that returns (), with the send result discarded. Nothing observes the caller leaving, so an abandoned move uploads or downloads the whole .dat anyway and then commits the transition. Go aborts both. Its progress callback returns `stream.Send`'s error, which surfaces out of the reader in s3_upload.go:99 and the writer in s3_download.go:84 and fails the transfer, so the volume info is never rewritten. The Rust port dropped that by typing the callback as FnMut(i64, f32) with no result. Give the callback Go's signature -- FnMut(i64, f32) -> Result<(), String> -- and abort when the caller's channel is closed. Checked on every part rather than only where progress is reported, since the report is rate-limited to one a second and would miss a caller that left in between. A merely full channel is a slow reader, not a departed one, so only TrySendError::Closed counts as cancellation. Two consequences of aborting mid-transfer that the old code never had to handle: - upload_file now aborts the multipart upload when the transfer fails. An abandoned multipart upload does not show up in an ordinary object listing but still accrues storage charges until a lifecycle rule reaps it, and cancellation makes that a routine path rather than a rare one. - The tier-down handler removes the partial .dat. download_file pre-allocates the destination to the object's full size, so an aborted download leaves a file of the right length and the wrong content -- and this handler refuses to run at all when a local .dat exists, so leaving one wedges every retry on "already on local disk" and a restart would load the sparse file as the volume's data. There is deliberately no check between a finished transfer and the bookkeeping that follows. Once the object is in S3, or the .dat is on disk, that bookkeeping is what makes the state consistent; stopping there would leave an object paid for and referenced by nothing, or a complete local .dat the volume still calls remote. Go does not gate there either -- its callback only runs during the transfer. Claude-Session: https://claude.ai/code/session_0122W3eqt6gmLUMxmRoZdPAb Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4b41329e12 |
rust volume: stop a VolumeCopy whose caller has gone (#11188)
* rust volume: stop a VolumeCopy whose caller has gone VolumeCopy runs its copy in a detached tokio::spawn and reports progress with the send error discarded, so nothing observes the client leaving. When the caller cancels the RPC -- which weed-admin's batch balance does routinely, starting far more copies than it finishes -- the server streamed the whole volume from the source, wrote it to disk, and mounted it. The destination is then left holding a volume nobody took delivery of: its index cache is never reclaimed, and under replication=000 one volume id ends up on two servers, both writable, which concurrent writes can diverge. Three checks now reach the task: - Every chunk in copy_file_from_source, via the sender's is_closed(). This is the one that matters in practice. The first progress report is 128MB in, so for a smaller volume -- the ordinary balance move -- no send ever happens and its result says nothing; only the closed channel does. The sender is passed for the .idx and .vif copies too, with reporting gated separately, so those phases notice as well. - The throttle sleep, which for a throttled copy runs for seconds at a time, now races the sender's closed() instead of being slept through. - Immediately before mount_volume, and once at the top of the task. Cancellation surfaces as an ordinary Err(Status::cancelled), so it lands in the existing error branch that already removes the partial .dat/.idx/ .vif and the .note. That branch also logs now: the error otherwise went to a channel nobody was reading, leaving the operator with the balancer's "delete that copy, then re-run the move" and no cause. This also clears the stranded read-only sources reported on the issue. They are downstream of the orphan mount, not a separate defect: LiveMoveVolume's cleanup probes the target before undoing the freeze (volume_move.go:95, "the server can finish the copy and mount the target even when the client loses the stream"), and when it finds a mounted copy it cannot attribute, or cannot delete, it deliberately keeps the source readonly rather than risk two writable replicas -- the messages at volume_move.go:110 and :123. With nothing mounted on the target the probe reports clean and the freeze is undone. On Go parity: the progress send result is honoured here too, matching `return false` in volume_grpc_copy.go. But that report is Go's only abort signal, and measured against a 120MiB volume -- above the throttler's activation threshold, below the 128MiB report interval -- a Go destination mounts an abandoned copy as well. The issue's premise that Go aborts holds only above the report interval. The Rust side now stops in both cases; the Go behaviour is worth its own issue. Tests: the integration test runs against both implementations and is green on Go, red on Rust before this change. Its 192MiB fixture is sized for two separate constraints, documented at the fixture: IoBytePerSecond is a no-op below ~100ms of wall clock (64MiB copies in ~110ms on a tmpfs loopback cluster), and the payload must exceed the 128MiB report interval for the Go leg to pass at all. The two Rust unit tests cover what the integration test cannot reach: cancellation detected with no progress report at all, and the cleanup of the partial files plus the .note. Fixes #11186 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122W3eqt6gmLUMxmRoZdPAb * rust volume: surface VolumeCopy cancellation as Status::cancelled copy_file_from_source returned Result<_, String>, so the per-chunk cancellation path -- the one that matters in practice for volumes below the 128MB report interval -- was wrapped to Status::internal at the call sites. The spawn logging branch then classified it as a generic failure instead of the intended "abandoned by caller", defeating the logging change in the same PR for the case that occurs most often. Return Result<_, Status> from copy_file_from_source: Status::cancelled for caller-gone, Status::internal for the existing errors. Drop the .map_err(|e| Status::internal(e)) at the three call sites. The unit test now asserts the code is Cancelled, not just the message text, so the classification is locked in. * rust volume: close cancellation gaps in VolumeCopy Address two review findings on the same PR: 1. Roll back a mount that races a departing caller. The pre-mount is_closed() check cannot close the window between the check and mount_volume: if the receiver drops in that gap, the volume mounts and the final tx.send(Ok(...)) fails, but its error was discarded (let _ =), so the task returned Ok(()) and the error branch never ran. The destination then held an orphaned mounted replica — the exact defect this PR prevents. Fix: track a mounted flag. The final send now checks its result; on failure it returns Status::cancelled, and the error branch calls store.delete_volume (which unmounts AND removes the files) when mounted is true, instead of only unlinking. 2. Observe cancellation while awaiting the source stream. The per-chunk is_closed() check only runs after stream.message().await returns. A stalled source (slow disk, partition, GC pause) never delivers a chunk, so a caller that has already left cannot preempt the read: the task, the source connection, and the partial files (including the .note) all outlive the caller indefinitely. Fix: race stream.message() against progress_tx.closed() in a tokio::select!, so a departing caller preempts a stalled source. Adds test_volume_copy_after_mount_cancellation_rolls_back_mount to cover the after-mount rollback path. cargo test --release green (497 + 5 + 1 + 28). * rust volume: keep remote data on after-mount rollback, race RPC startup Two review findings on the after-mount rollback added in |
||
|
Chris Lu
|
f79d83abf4 |
volume: expire TTL volumes whose only traffic is deletes (#11167)
* volume: count a TTL volume's age from its last write, not the .dat mtime A delete appends a tombstone needle and vacuum rewrites the .dat wholesale, so the file's mtime moves without any write ever landing. The loader read lastModifiedTsSeconds back from that mtime, so every restart of a volume taking delete traffic re-armed expired() for another full TTL: an overwrite-heavy collection kept growing until it hit the max-volume cap. Recover the clock from the newest .idx entry that is not a tombstone and read that needle's append timestamp, falling back to the mtime when no write is recoverable. Only TTL volumes pay for the scan. Fixes #11160 * volume: count the .vif destroy time from the last write too ExpireAtSec is what an EC volume is reclaimed on, and it was recomputed as now+TTL every time the .vif was written. A read-only mark, a tier upload or an EC encode therefore handed an already expiring volume another full TTL, the same way the .dat mtime did. Derive it from the volume's last write, falling back to now for a volume that has not taken one yet so a fresh volume is not born expired. * volume: mirror the last-write TTL clock in the Rust volume server Same recovery as the Go loader: scan the .idx backwards for the newest entry that is not a tombstone and take that needle's append timestamp, leaving the clock on the .dat mtime when no write is recoverable. * volume: mirror the last-write destroy time in the Rust volume server Both .vif writers and the EC encode computed ExpireAtSec as now+TTL, the same way Go did, so the destroy time moved every time the sidecar was rewritten. Route all three through the volume's last write. * volume: report the .dat mtime in the Rust heartbeat, like Go does The Rust server reported its TTL clock as ModifiedAtSecond while Go reports the .dat mtime. The shell's quiet-period gates (volume.tier.move, volume.delete_empty) read that field as "last touched", which a delete has to count towards even though the TTL clock deliberately ignores it -- and with the clock now recovered from the last write, the two drift further apart. * volume: take the newest write by timestamp on a vacuumed volume The reverse .idx scan trusted position, which holds only while the .dat is append ordered. Vacuum rewrites it in key order, and since an overwrite keeps its original key, the highest-key survivor is not necessarily the newest write -- the recovered clock could land up to a TTL early and take the volume with data still inside its TTL. A volume that has been vacuumed (CompactionRevision > 0) now takes the maximum append timestamp over a bounded window of write entries instead. An append-ordered volume still answers in one read. * volume: never guess a vacuumed volume's last write, and resolve wrapped offsets Two holes in the reverse scan, both from review: A vacuumed volume's writes are ordered by key, so any of them can hold the newest timestamp. Reading a capped window sampled the highest keys, which could still miss a recently overwritten low-key needle and expire data inside its TTL. The scan now covers every write a vacuumed volume indexes, and a volume too large to scan keeps the .dat mtime rather than report a partial maximum -- late is recoverable, early is not. A .dat past MaxPossibleVolumeSize wraps the offsets in its .idx, so reading a timestamp at the unwrapped offset picks up an unrelated needle. Resolve the entry against the needle header first and retry one volume size in, the way doCheckAndFixVolumeData already does. * volume: drop GitHub issue references from TTL comments |
||
|
Chris LuandDevin
|
24b8646ec3 |
volume: let evacuation proceed on a server in maintenance mode (#11145)
Maintenance mode exists to fence a volume server so it can be evacuated without taking new writes (#7977), but the gate added in #8115 also rejected the RPCs evacuation issues against the source: VolumeMarkReadonly (the first step of every move, and the failure reported in #11066), VolumeDelete (the last step), and VolumeEcShardsDelete (the last step for EC shards). volumeServer.evacuate, volume.move and ec.balance therefore all failed on exactly the server they were meant to drain. Those three RPCs only remove data or restrict the server further, the same class as DeleteCollection and the unmount RPCs that were never gated, so they are exempted from the maintenance check in both the Go and Rust volume servers. Everything that adds data or reopens the server for writes (AllocateVolume, WriteNeedleBlob, BatchDelete, VolumeCopy, ReceiveFile, EC generate/copy/rebuild, vacuum, tiering, VolumeMarkWritable) stays blocked. A side effect is that scrub can now fence broken volumes readonly on a server already in maintenance. Fixes #11066 Generated with [Devin](https://devin.ai) Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
8112f2733a |
filer: batch exact lookup RPC, authoritative volume lookup, VolumeDelete status codes (#11122)
* storage: make DeleteVolume errors inspectable with errors.Is An absent volume wraps ErrVolumeNotFound and an only-empty refusal now wraps ErrVolumeNotEmpty with %w instead of %v, so callers no longer have to match on the message. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * volume server: return NotFound and FailedPrecondition from VolumeDelete An absent volume maps to codes.NotFound and a non-empty volume under only_empty to codes.FailedPrecondition, so a caller retiring a volume can treat NotFound as already done. The store message is kept in the status description because the EC empty-replica sweep still matches on it. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * wdclient: add LookupVolumeIdsAuthoritative Bypasses the vid map and asks the provider directly, for callers where a stale positive location is unsafe. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: add LookupDirectoryEntries batch lookup RPC Up to 4096 exact-path lookups in one call, resolved concurrently with results in request order, plus one deduplicated location lookup for every volume the returned entries reference and per-fid read tokens when the filer signs reads. unavailable_volume_is_miss lets cache-style callers take an entry whose volume has no live location as a miss, resolved against the master rather than the filer's location cache. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: test that an expired file entry is deleted on read Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: test that AssignVolume and CreateEntry resolve the same TTL rule Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * master: refuse partial lookups while warming up LookupVolume returned Unavailable during warm-up only when every requested volume was missing. A batch mixing a reported volume with one whose server has not reconnected yet came back as a partial answer with a per-volume not-found, which a caller treating the master as authoritative reads as gone. Any not-found during warm-up is now Unavailable, which callers already retry. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: build batch test requests instead of copying a proto message Copying a generated message copies its internal mutex, which go vet's copylocks check rejects. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: match ErrNotFound with errors.Is and state the miss rule's contract A wrapped not-found from the store would otherwise be reported as an error rather than a miss. The comments now say why a nil location map is the only sign of an unanswered lookup: the provider returns nil when it got no answer and a populated map, with unserved volumes reported as errors, when the master did answer. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * volume server: map absent and non-empty VolumeDelete errors in the Rust server Matches the Go server: an absent volume is NotFound and an only_empty refusal is FailedPrecondition instead of Internal, with the messages the EC empty-replica sweep matches on. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: test that a malformed entry keeps its error outside cache mode Same test file as the enterprise tree, so the next sync sees one version. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm |
||
|
Chris Lu
|
74b520113e |
ec: pin auto-selected shard placement to the disk that already owns the shard (#11029)
* ec: pin auto-selected shard placement to the disk that already owns the shard A multi-disk server legitimately mounts one EC volume on several disks, so FindEcShardTargetLocation's per-volume tiers tie at "mounted" and the free-shard-count tie-break decides — pointing at whichever disk is emptier, not at the disk that already holds the shard being placed. A re-copy of a shard the server already has (a retried ec.balance / ec.rebuild move) then lands on a sibling disk, and both disks register the same (volume, shard id): the shard is reported to the master from two disk ids, and which claimant serves reads or survives a later unmount/delete becomes an accident of Locations order. Add a tier above "mounted": a disk that already claims one of the shard ids being placed wins, ahead of the space filters too — re-copying in place needs no new shard slot, and a genuinely full disk should fail the write rather than silently split the claim. Applied to the Go selector and the VolumeEcShardsCopy auto-select (ReceiveFile refuses mounted EC volumes, so no claim can exist there) and mirrored in the Rust volume server. Claude-Session: https://claude.ai/code/session_01AWpefvdi4U3HLng18x5CJ9 * ec: refuse a copy batch whose shards are already owned by different disks Review follow-up: ownership-aware selection ranks a mixed-owner batch (shard 0 on disk A, shard 2 on disk B — the legitimate multi-disk spread) into one destination, so the copy would still duplicate the losing disk's claim. No production caller sends such a batch (balance moves one shard, rebuild and encode copy shards the target lacks), so fail closed: report every owning disk via Store.EcShardOwnerDisks and refuse the copy with an error naming them, telling the caller to split per shard or pass disk_id. Go and Rust, with unit tests for the owner-reporting contract. Claude-Session: https://claude.ai/code/session_01AWpefvdi4U3HLng18x5CJ9 |
||
|
Chris Lu
|
88c873ecd4 |
ec: uniform shard block layout (#10932)
* ec: uniform shard block layout An EC volume is striped as 1GiB blocks until less than one row remains, then 1MiB blocks, and consecutive blocks land on different shards. With ec.encode's -fullPercent 95 against the 30GiB default limit, ~30% of every volume sits in that 1MiB tail, so a 4MB filer chunk there is five stripes on five servers. New encodes now use one block per shard, sized ceil(datSize/dataShards) rounded up to 1MiB and recorded in the .vif (EcShardConfig.block_size, also carried by the .ecsum manifest). A needle now maps to one shard unless it is larger than the block or straddles a boundary. The chosen size equals the legacy layout's padded shard length for every input, so shard sizes, capacity math, and the shard-size credibility checks are unchanged; only the byte placement moved. Reads, decode, and scrub resolve the block sizes from the volume's .vif; absence keeps the legacy interpretation, so existing EC volumes read exactly as before. Rebuild is layout-agnostic. weed fix -ecx recovers the layout from the .vif, else the .ecsum sidecar, and with neither de-stripes under both candidate layouts and keeps the one that indexes more valid needles. Same change in the Rust volume server, which now also streams the encode in 256KB sub-batches like Go instead of allocating whole blocks, and computes the large-row count as shardSize/largeBlock to match Go on exact multiples. On a 26MB fixture both encoders produce byte-identical shards, and a Go-written .vif parses in Rust with the block size intact. * ec: resolve the rust ecx rebuild through the recorded layout The Rust rebuild path regenerated a lost .ecx by scanning the logical .dat through a hand-rolled pure-1MiB striping, which was already wrong for legacy volumes with large-block rows and is wrong for any uniform volume with a block past 1MiB. Route the scan through locate_data with the .vif-recorded block size, the same mapping the read path uses. Also seed the new tests' random data instead of the deprecated global math/rand.Read. * ec: fail the Rust ecx rebuild on any shard read error A read error mid-scan published the entries collected so far as a successful .ecx, and read_at's byte count was ignored so a legal short read passed as complete — a truncated or failing shard could produce a silently incomplete recovery index. Exact-read semantics in read_from_data_shards, error propagation in the needle walk, and a truncated-shard regression test. * ec: fail the mount on an unreadable or malformed vif Both servers silently fell back to the legacy layout when an existing .vif could not be read or parsed. Every new encode records a positive uniform block size there, so the fallback mounted the same shards with legacy offset math and could return wrong data. Absent stays legal (legacy volumes predate the sidecar), and a zero-byte stub still reads as absent (Go's MaybeLoadVolumeInfo convention, now mirrored in Rust); a present-but-unreadable or malformed .vif fails the mount instead. * ec: bound the reconstruct fan-out of one needle's intervals A degraded interval fans out a read to every reachable shard location, each with a buffer the size of the interval. Reading a needle's intervals in parallel multiplied that by the interval concurrency: a needle spanning 8 blocks could hold 8 x MaxShardCount remote reads and buffers at once, where the sequential version peaked at MaxShardCount. Give each needle a single reconstruct budget its intervals share, held for the buffer's lifetime, so separate reads stay independent but one read cannot multiply its own fan-out. * ec: drop the duplicated shard-size formula calculateExpectedShardSize reimplemented the padding rule that UniformBlockSize already owns — TestUniformBlockSizeMatchesLegacyShardSize asserts the two agree for every input — so a change to the rule would have had to be made in both. Defer to the helper, keeping the historic answer for an empty .dat. * ec: resolve the shard block layout from whatever records it Four places still answered the layout question by inference when a record of it was available, or accepted an answer that was not one: - A mount with no .vif defaulted to the legacy layout; the bitrot sidecar records the same config at encode time, so take it when present, as weed fix -ecx already does. The vif itself is now parsed once per mount rather than twice. - The Rust ecx rebuild derived its row count from the padded shard extent, which under the legacy layout reads a shard that is an exact large-block multiple as one row too many. Pass the encode-time .dat size from the .vif and keep the extent as the fallback. - weed fix -ecx read the block size outside the EC-config guard (collapsing the unknown sentinel into a definitive legacy), only wrote the recovered layout back when the .vif was absent rather than unusable, and broke a scan tie by candidate order instead of the documented reach. - The uniform layout tripped writeDatFile's large-block ambiguity guard, which cannot apply when the large and small blocks are the same size. * ec: give the index-recovery tests a parseable vif The fixtures wrote the literal bytes "volinfo" as the source .vif and the recovery copies it verbatim, so the receiving server then mounted the volume from a .vif it could not parse. That used to pass by silently defaulting to the legacy layout; a mount now refuses a vif it cannot read, which is what the tests were exercising all along without meaning to. * ec: validate the layout a vif records, not just its syntax Review follow-ups on the mount-strictness change: - A .vif can parse and still record a block size no encoder could have produced (negative, or not a whole number of small blocks). Both servers took it and mapped every read through it. ValidateBlockSize / the Rust mirror now refuse the mount, the same way an unparseable vif does; 0 stays valid as the legacy two-tier layout. - The bitrot-sidecar fallback accepted parity_shards == 0 and summed the counts in their own width, so values near the ceiling wrapped past the MaxShardCount bound. Require both counts and sum in a wider type. - weed fix -ecx treated a config with only DataShards > 0 as usable, so a half-written .vif suppressed the recovery paths AND survived the rewrite. Require a complete, in-range config before trusting it. - Returning the vif-load error left the .ecx and .ecj descriptors open; repeated mount attempts on malformed metadata could exhaust them. * ec: refuse to act on a layout the metadata does not establish - The worker encode only logged a failed .vif write and skipped it in the distribution set, and treated the .ecsum write as best-effort. A worker whose disk filled after the much larger shards landed could still distribute, mount, verify shard inventory, and delete the source replicas — leaving holders with shards whose geometry nothing records. Both writes and both inclusions are encode success conditions now. - A generation-matching .ecsum that disagreed with the .vif geometry only disabled checksums in Go, and in Rust was not compared at all, so protection stayed On while reads used the other layout. Both files record the layout their generation was encoded with, so a disagreement now fails the mount. * ec: reject an invalid recorded block size in weed fix -ecx A .vif with valid shard counts but a negative or unaligned block size was marked usable: a positive invalid value pinned the scan to a geometry that de-stripes to garbage, and a negative one ran the dual scan but left the invalid .vif in place afterwards. Validate it with the same rule the mount applies, and when it fails leave the layout unknown so the scan recovers it and the file is rewritten. * ec: validate the sidecar layout weed fix -ecx recovers from The .ecsum fallback was taken on DataShards > 0 alone, so a CRC-valid sidecar carrying the wrong generation, an incomplete ratio, or an unaligned block size would pin the reconstruction to one incorrect uniform-layout candidate instead of letting the dual scan decide. Require generation 0, a complete in-range ratio, and a valid block size; anything less leaves the layout unknown, which is the answer that still recovers by scanning. * ec: let only a genuinely absent sidecar choose the legacy layout With no .vif the bitrot sidecar is the only record of a volume's layout, and the mount fallback read a failed load, an unusable config, or a sidecar stamped for another generation as "assume legacy". A uniform generation-0 volume could therefore mount with legacy or another generation's geometry and answer reads with the wrong bytes. Present-but-unusable now fails the mount; only actual absence keeps the legacy defaults. Shared as EcShardConfigFromSidecar so every caller reads the sidecar the same way. * ec: treat a recorded-but-impossible layout as corruption, not as legacy - A .vif whose ecShardConfig is PRESENT but records an impossible ratio was answered with the default 10+4 and the legacy block layout, in both languages. That reads a uniform volume's shards at the wrong offsets and returns the wrong bytes. Only an entirely absent config still means "this predates the record"; a present one that cannot be true fails the mount. - The shard-count bound summed two uint32 counts as int, which wraps on a 32-bit build: 0x7fffffff + 0x7fffffff lands at -2 and slips under MaxShardCount. ValidEcShardCounts sums in uint64, and every EC call site that checked a recorded ratio now goes through it. * ec: rebuild on the geometry the sidecar records, and flag it when it disagrees The rebuild RPC passes BackgroundECContext, so RebuildEcFiles resolves the layout itself — and it resolved a missing or invalid .vif to the default 10+4 with the legacy block size. Two consequences: a 12+4 volume was reconstructed through a 10+4 matrix, which produces wrong bytes and never regenerates shards 14-15; and the chosen geometry then contradicted a valid uniform sidecar, which loadRebuildSidecar reported as BitrotOff — silently skipping the input and regenerated-shard checksum checks precisely when the volume had already lost its metadata. The layout now resolves from the bitrot sidecar (found across the server's disks, not just beside the base name) before falling back to the defaults, and a present-but-impossible ratio fails instead of being replaced. A sidecar that contradicts the chosen geometry is BitrotInvalid, which the existing unsafeIgnoreSidecar override still lets an operator push past. * ec: let the Rust rebuild read metadata off a sibling disk read_ec_shard_config searches only the location the rebuild writes into, so a volume whose .vif or generation-0 .ecsum sits on another of the server's disks resolved to the default 10+4 with the legacy block layout — the Rust half of the geometry-guessing the Go rebuild just stopped doing. It then reconstructs a custom-ratio or uniform volume through the wrong Reed-Solomon matrix and de-striping geometry. The rebuild now looks for the .vif in its own location and then each sibling, falls back to the generation-0 sidecar wherever that lives, and only defaults when neither exists anywhere. The encode-time .dat size the ecx rebuild needs is resolved the same way. * ec: resolve a rebuild's vif from every directory that may hold it RebuildEcFiles probed only <data-base>.vif. The caller knows the selected location's index directory and the sibling locations, but passed neither for metadata: additionalDirs carried shard directories only, and were searched for shards and the checksum sidecar. A split -dir/-dir.idx layout, or a disk holding only shards, therefore resolved a pre-sidecar custom-ratio volume to 10+4 and reconstructed through the wrong matrix — never regenerating shards 14-15. The caller now hands over the index and sibling directories, and the resolver probes the vif across all of them, matching what the Rust resolver already does for both the vif and the sidecar. * ec: make every rebuild consumer agree on the layout it resolved - The post-rebuild bitrot backfill re-derived the geometry from this directory's .vif alone and dropped the block size entirely, so a rebuild that resolved its layout from a sibling, the sidecar, or a uniform vif wrote a manifest describing a DIFFERENT layout — one later mounts reject, or that covers only the default shard count. The layout is resolved once now, through an exported ResolveRebuildECContext, and the rebuild and the backfill share that answer. - The Rust rebuild collected only each location's data directory, so a sibling's INDEX directory — where a split -dir/-dir.idx layout keeps .ecx/.ecj/.vif — was never probed, and a custom-ratio volume still resolved to 10+4 with the legacy layout. Both directories of every location are carried now, deduped against the rebuild's own. - A shard delivery can bring the checksum manifest with it, but the receive path only writes the file: a server that already had the volume mounted kept its resolved protection state (off) until a remount. The mount RPC re-resolves it once the shards it describes have been added. * ec: cover the rebuild's directory search with tests Reviewers flagged the sibling index directory twice, and the fix that closed it had no test of its own: the assembly sat inline in the rebuild handler, reachable only through a gRPC call against a populated store. Lifting it into rebuildSearchDirs / select_rebuild_location makes the rule assertable — a sibling contributes BOTH its data and its index directory, a shared index directory is listed once, and the rebuild's own data directory never repeats. Writing the Rust cases surfaced that the two implementations do not agree on where the rebuild's own index directory belongs, and both are right: Go's resolver takes a single directory list, so that directory has to be inside it, while Rust's takes the rebuild's data and index directories as their own arguments and would search them twice. The tests now state which contract each side is holding to, so neither drifts into the other's shape. Pure refactor otherwise; no behaviour change. * ec: search the index directory for the layout sidecar The Rust resolver looked for the generation-0 .ecsum in the rebuild's data directory and the sibling list, but not in the rebuild's own index directory — while the .vif lookup directly above it did, and Go's findBitrotSidecar has always checked both bases. On a split -dir/-dir.idx location that directory is where the metadata lives, and callers leave it out of the sibling list precisely because it is passed here separately, so nothing searched it. With no .vif anywhere the sidecar is the only surviving record of the layout. Missing it resolved a 12+4 uniform volume to 10+4 with the legacy striping — the test added here fails with (10, 4, 0) against the old code — and the rebuild then reconstructs through the wrong matrix and writes .ecx offsets that no reader can follow. * ec: let the rebuild see its own index directory The Rust rebuild takes a single flat directory list — the shape Go's RebuildEcFiles uses — so it cannot be handed the rebuild location's index directory separately the way the layout resolvers are, and the handler was passing the sibling list, which deliberately omits exactly that directory. On a split -dir/-dir.idx location that is where .ecx and .vif live, so the shard and index lookups could not see them. Go has always carried that directory in additionalDirs; this lines the two call sites up. * ec: let a config-free vif fall through to the layout sidecar A .vif that carries no ecShardConfig answers nothing about the layout, so it is no more informative than an absent one — but both trees treated its mere existence as the end of the search. Go went straight to the 10+4 legacy defaults without consulting the sidecar at all; Rust returned whatever ec_shard_config_from could make of a single directory. A 12+4 uniform volume with a legacy config-free vif therefore resolved as 10+4 legacy, and every read landed at the wrong shard offset. The sidecar lookup was also single-directory on both sides, while a split -dir/-dir.idx layout keeps .vif and .ecsum with the INDEX. Go's findBitrotSidecar has always taken both bases; the callers here passed only the data base, and the Rust bitrot resolver derived its path from the data base alone. Rust's layout resolver now takes a candidate directory list — data, index, then any siblings — and searches all of it, which also removes the early return that made the vif's presence decisive. load_vif_info_across_dirs reported `dir` even when load_vif_info had found the vif in `dir_idx`. Nothing reads that field today, so this changes no behaviour; it stops the next caller that resolves the rest of the volume's metadata against the answer from being sent to a disk holding none of it. Absence stays legal throughout: a volume with neither record is genuinely legacy. Present-but-unusable still fails the mount, now in the config-free-vif branch too. * ec: activate a delivered sidecar on every per-disk runtime A vid mounts as one EcVolume per disk, each with its own resolved protection state, but the post-delivery reload used the first-match lookup and so touched exactly one of them. The siblings kept reporting no protection until a remount — and since shard distribution deduplicates the metadata files onto the first target disk for a node, the runtime that got the .ecsum is not necessarily the one the lookup returns. Iterate every runtime instead, via a new FindAllEcVolumes and its Rust mut equivalent. Combined with each runtime now resolving its sidecar against its index directory as well as its data directory, a server sharing one -dir.idx across its disks activates all of them from the single delivered copy. The Rust volume server had no post-mount reload at all; it gets one here, matching Go. * ec: resolve the delivered sidecar across every EC metadata directory Reloading every per-disk runtime, added last round, did not by itself make the delivered manifest reachable. Startup mirroring copies .ecx/.ecj/.vif to every shard-bearing disk so each mounts self-contained, but deliberately not .ecsum, and a repair delivers exactly one copy. Each runtime was resolving against its own two directories, so every sibling of the disk that received the file kept reporting no protection however often it reloaded. Resolve one authoritative copy across every EC metadata directory instead of duplicating the file. Mirroring .ecsum would have to keep pace with a file that is rewritten as shards are repaired, and would not help the reported case at all: the delivery happens at runtime, and mirroring only runs at startup. The regression test pins both halves — a reload restricted to the volume's own directories still finds nothing, and the same reload given the server's metadata directories turns protection on. * ec: ask every directory before writing a TOFU baseline After a rebuild the opportunistic backfill asks whether this volume already has a checksum manifest, and answered from the data base alone. A split -dir/-dir.idx layout keeps the sidecar with the index, and a multi-disk server may keep it on a sibling, so an existing manifest read as absent. The consequence is worse than a missed read. On a false "no" the backfill writes a fresh sidecar at the data base from whatever the shards say right now — and the data base is the first candidate every resolver checks, so that TOFU baseline shadows the real manifest rather than sitting beside it. A shard that was silently corrupt gets blessed, and the record that would have caught it stops being consulted. FindBitrotSidecar exports the search the package already used internally, so the question is asked of the data base, the index base and the sibling disks — the same candidates the rebuild resolves its layout from. * ec: refuse a shard block size no encoder could have produced weed fix -ecx derived one from the raw shard extent, so a truncated or partially copied shard wrote a .vif that NewEcVolume then permanently refuses — the volume the tool was run to rescue could never mount again. An extent that is not a whole number of small blocks cannot have come from a uniform encode, so it is no longer offered as a candidate, and nothing unvalidated reaches the .vif. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * ec: derive the .vif's dat size and block size from one measurement VolumeEcShardsGenerate stat'ed the .dat before the encode while WriteEcFiles stat'ed it again to size the blocks. A write landing between the two produced a .vif whose own two fields describe different files. WriteEcFiles now leaves both on the context, and fills a placeholder context in place so the caller can read them back. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * ec: keep the source volume until every holder serves its shard layout The uniform layout rides in a .vif field older volume servers never knew: they discard it, mount the shards as legacy and return wrong bytes with nothing erroring, and the shard files are the same length either way so no other check notices. The upgrade order lived only in the release note. VolumeEcShardsInfo now reports the block size the holder actually serves, in both the Go and Rust servers, and the pre-delete verification refuses to drop the source unless every reachable holder echoes the one the shards were encoded with — while a rollback still exists. A server that predates the field answers 0, which is the negative answer. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * ec: drop the rebuild's dead block-size parameters generateMissingEcFiles never reads largeBlockSize/smallBlockSize — Reed-Solomon reconstruction is layout-agnostic — so passing the legacy constants only advertised a layout the rebuild does not use. Also move UniformBlockSize's doc off ValidateBlockSize. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * ec: warn about EC defaults only when the mount used them The "vif file not found, using defaults" warning fired even after the bitrot sidecar supplied a non-default layout, sending anyone triaging wrong bytes after the legacy layout the volume never mounted on. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * ec: stat the distributed bitrot sidecar once The strict check re-stat'ed the file immediately before the stat that already gates inclusion, and a failed sidecar write now fails the encode outright, so the first could only fire on a deletion between the two lines. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * ec: say what the reconstruct budget actually bounds A shard's buffer stays in bufs until its interval reconstructs, which is after the read that filled it released its permit, so the semaphore bounds round trips in flight and not retained bytes. Peak memory is the intervals reconstructing at once times the shards each reaches times the interval size. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 * test: let the fake volume server report its delivered EC layout The pre-delete verification now asks each holder which shard block layout it serves, and a fake that always answered "unset" looked exactly like a volume server too old to know the field. Distribution ships the .vif to every holder alongside its shards, so read the layout back out of it as a real holder does. Claude-Session: https://claude.ai/code/session_011FRRoNKBiGbH58rs2AQyA7 |
||
|
Chris Lu
|
3967ca23be | rust: cover the READS scrub reconstruction path (#11027) | ||
|
Chris LuandLisandro Pin
|
fcc2ea61d3 |
ec: scrub a volume through its parity data (#11006)
* Introduce a new `READS` scrub mode. `READS` performs a full volume scrub but, unlike `FULL`, it will attempt to reconstruct data for missing/damaged shard intervals from other shards in the cluster when necessary. The goal of this check is to ensure that EC volume contents _are readable by Seaweed_ even on a degraded storage state, by exercising parity data which is not read in `FULL` mode. This is useful not only to validate data is user-readable, but also to detect potential parity shard issues which may be difficult to pinpoint otherwise - particularly for older volumes lacking sidecar data, and hence unaffected by `CHECKSUM` scrubs. For regular volumes, this operation is equivalent to `FULL`. Example: ``` > ec.shard.unmount --volumeId=1 --shardId=0,3,11 --delete --apply Live shard topology for volume ID 1 (14 shards): 0@10.200.18.89:9001 1@10.200.18.89:9002 2@10.200.18.89:9003 3@10.200.18.89:9004 4@10.200.18.89:9005 5@10.200.18.89:9006 6@10.200.18.89:9007 7@10.200.18.89:9008 8@10.200.18.89:9009 9@10.200.18.89:9013 10@10.200.18.89:9010 11@10.200.18.89:9011 12@10.200.18.89:9012 13@10.200.18.89:9020 Will unmount + delete 3 shard(s): 0@10.200.18.89:9001 3@10.200.18.89:9004 11@10.200.18.89:9011 Unmounting shard 0@10.200.18.89:9001 for volume ID 1... Deleting shard 0@10.200.18.89:9001 for volume ID 1... Unmounting shard 3@10.200.18.89:9004 for volume ID 1... Deleting shard 3@10.200.18.89:9004 for volume ID 1... Unmounting shard 11@10.200.18.89:9011 for volume ID 1... Deleting shard 11@10.200.18.89:9011 for volume ID 1... All done! > ec.scrub --volumeId=1 --node=10.200.18.89:9002 --mode=full using FULL mode Scrubbing 10.200.18.89:9002 (1/1)... Scrubbed 6 EC files and 1 volumes on 1 nodes Got scrub failures on 1 EC volumes and 1 EC shards :( Affected volumes: 10.200.18.89:9002:1 Affected shards: 10.200.18.89:9002:1:0 > ec.scrub --volumeId=1 --node=10.200.18.89:9002 --mode=reads using READS mode Scrubbing 10.200.18.89:9002 (1/1)... Scrubbed 6 EC files and 1 volumes on 1 nodes ``` * ec: report the shards a READS scrub had to rebuild A READS scrub that recovers an interval was recording nothing, so a volume missing three shards came back clean and nobody repaired it. The unreadable shard is now recorded before the rebuild is attempted: READS reports the same broken shards as FULL and differs only in whether the needles themselves failed, which is the signal worth having - shards are gone, data is still there. forceDeletedNeedlesCheck now applies to READS as well, in the shell and in the RPC guard: it runs the same needle walk as FULL. Regenerated the proto instead of hand-editing it, so the pancis typo (which protoc-gen-go-grpc emits into eight other files here) and the header whitespace stay as generated. Mirrors into the Rust volume server, which also now honors force_deleted_needles_check rather than hardcoding it off. Claude-Session: https://claude.ai/code/session_014yMNebkUjSbx9sfUCWJJtq * ec: answer a deleted needle from a READS rebuild as deleted #11020 gave the Rust recovery a deleted flag alongside its bytes, and it answers a deleted needle with no bytes at all. The READS scrub appended that empty answer, which does not compile against the new signature and, once it did, would leave the needle short and report the size mismatch as damage. Zero-fill the interval instead, the way the direct read beside it already does: the assembled needle then reaches read_bytes as the delete-state mismatch the walk already tolerates. Go takes the same branch off the flag its recovery returns, rather than discarding it. Claude-Session: https://claude.ai/code/session_014yMNebkUjSbx9sfUCWJJtq --------- Co-authored-by: Lisandro Pin <lisandro.pin@proton.ch> |
||
|
Chris Lu
|
cd5013f116 |
Re-check an EC shard map a failed read has disproved (#11023)
* Re-check an EC shard map a failed read has disproved A read that fails against a cached location drops that shard from the map, which leaves it one short of complete -- and a map one short is trusted for seven more minutes. So a moment's trouble between volume servers cost minutes in which every read of that shard skipped the direct fetch and paid for a Reed-Solomon recovery instead, at DataShards times the memory and the peer load. Mark the map when a read disproves it, and re-check a marked map on the same eleven-second footing as one that never had enough shards to begin with. The mark clears on refresh, so it buys one prompt re-check rather than a master lookup per read. The tiers move into a helper; they were three overlapping conditions in one expression, and the reading of them was not obvious. Rust keeps the entry rather than dropping it -- a dead peer fails fast on the next attempt, and it was the freshness window, not the entry, hiding a shard that had moved. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Invalidate the location of an EC shard whose own read failed Recovery fans out to the other shards, so the one whose direct read just failed is the only location nothing ever invalidates: a shard that moved to another server was reconstructed on every read until the map's own window expired, up to thirty-seven minutes for a map still complete. Mark the map there too. The entry stays -- a moved shard's old holder fails fast, and the next refresh is seconds away. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Consume the stale mark before the lookup, not after A read that fails while the master is answering has disproved the very map that answer is about to install, and clearing the mark on the refresh's return swallowed it. Clear it where it is acted on instead. A lookup that then fails loses the mark, which costs nothing: the refresh time is only advanced on success, so the next read looks up regardless. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Judge the shard map and consume its mark in one critical section Reading the mark and clearing it were two separate acquisitions, so a mark raised between them was cleared by a refresh that had not seen it. In Go that gap was a few instructions; in Rust the mark was read when the read first snapshotted the volume and cleared at the decision point, with the local interval reads in between. Take both under one hold. Rust needs a mutex rather than an atomic to do it, and no longer carries the mark through the snapshot. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Put the stale mark back when the lookup does not answer for it Consuming the mark up front assumed the lookup would supersede it. A lookup that fails, or comes back with fewer than DataShards holders, supersedes nothing: the map is unchanged, its refresh time unadvanced, and with the mark gone the map a read had disproved is trusted for its full window again on the strength of a lookup that never landed. Put the mark back on both branches. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN |
||
|
Chris Lu
|
95248f7492 |
Bound the memory an EC shard recovery holds (#11020)
* Reconstruct an EC shard from the shards already on this server recoverOneRemoteEcShardInterval only ever fanned out to the cached shard locations, so a server holding shards of the volume still fetched them over gRPC from itself -- and when the peers were unreachable it could not reconstruct at all, even holding the whole volume on local disk. Seed the Reed-Solomon buffers from the locally mounted shards first; each one is a peer round trip, and an interval-sized buffer, the fan-out no longer needs. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Fetch only the EC shards reconstruction still needs The recovery fan-out read every surviving shard, so a 10+4 volume pulled 13 interval-sized buffers to feed Reed-Solomon 10 -- a third more memory held, and a third more load asked of peers that were, by definition, already having trouble. Fetch what is missing, and widen only when some of those reads fail. A shard reporting the needle deleted ends the walk: the rest would only answer the same. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Bound the bytes EC recovery holds in flight Recovery is the one read path that multiplies the served bytes: it holds an interval-sized buffer per shard until Reed-Solomon runs, and a peer that is slow to fail keeps them all alive for the whole gRPC timeout. Nothing bounded how many of those fan-outs ran at once, so a transient problem between volume servers turned every read into a DataShards-fold allocation and the server died of it -- 64 concurrent 4MB intervals pin 3.6GB, and that is a small burst. Charge each recovery against a process-wide budget, so a burst queues on the semaphore instead of on the heap. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Answer a deleted EC needle as deleted, not as a failed recovery A holder reporting the needle deleted is authoritative: deletes are never invented and never undone. Recovery already collected that flag, then dropped it on the branch where too few shards came back -- so a read of a deleted needle that had to recover surfaced as "cannot recover shard", and the volume server answered 500 where it owed a 404. Carry the flag out of the shortfall, and let it decide ahead of the error it came with. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Check the encode run of a locally seeded EC shard in Rust The Rust recovery seeded Reed-Solomon straight from the mounted shards, without the encode-run check the remote reads and Go's readLocalEcShardInterval both apply. A volume remounted from a newer encode between the read's snapshot and its recovery would have fed mixed-generation bytes into the reconstruction. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Say what the recovery budget actually guarantees Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN * Seed Rust EC recovery from shards on every local disk find_ec_volume returns the first disk's EcVolume, so a reconciled volume whose shards are split across data dirs had the siblings ignored and could report "cannot recover" while holding enough shards locally. Resolve each shard together with the disk that owns it, the way Go's recovery already does, and check that owner's encode run. Claude-Session: https://claude.ai/code/session_01SM5ARdPvFcnvGWNpBPgNRN |
||
|
Chris Lu
|
eed3c27d15 |
volume: cut the memory a server holding millions of volumes still uses (#10999)
* volume: stop the .vif guard depending on which entry the scan handed over A volume has both an .idx and a .vif, and loadExistingVolume skipped a .vif next to an .ecx as EC shard metadata. That was only ever correct because os.ReadDir sorted .idx ahead of .vif: an interrupted encode, where the .idx is still there, has to reach validateEcVolume to be reclaimed. Ask for the .idx instead of trusting the order. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy * volume: walk volume directories in batches instead of listing them whole os.ReadDir builds, and sorts, a slice of every entry before the caller sees the first one. A disk holding millions of volumes has a .dat, .idx and .vif per volume, so each startup scan costs hundreds of MB of peak heap that the runtime is slow to hand back -- and there are several of them before the first volume loads. Walk in batches instead, and keep only the entries each scan acts on: loadAllEcShards now sorts and stats the shard and index files alone rather than every file on the disk. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy * volume: skip the sibling-.dat scan when no EC volume is loaded pruneIncompleteEcWithSiblingDat only ever prunes EC volumes that are loaded, but it first walks every disk and keys a map by every .dat on the server. On a store with no EC volumes at all that is millions of map entries built to answer no question. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy * volume: stop keeping a departure message for every volume The report state held a VolumeShortInformationMessage per volume copy so a departure could be named, but almost no volume ever departs. Hold a handle to the identity instead -- volumes share very few distinct ones -- and build the message on the way out. Measured over a populated report state: 195 -> 83 bytes per volume. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy * rust volume: stop keeping a whole volume message per volume held The send loop kept a VolumeInformationMessage for every volume just to notice mounts and unmounts, and rebuilt the map from scratch on every beat. Keep the identity a delta names, which is what the Go report state keeps for the same reason. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy * rust volume: keep only the EC files the shard scan acts on load_all_ec_shards named every file on the disk twice -- once in the dedup set and once in the sorted vector -- before deciding it only wanted .ec?? and .ecx. Filter while reading instead. Mirrors the same change in loadAllEcShards. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy * volume: share the strings every .vif repeats A tiered volume's .vif names its replication and its backend, and every decode allocates a fresh copy, so a server holding millions of them holds millions of copies of the same handful of names. Route them through the interning table the volume info decode already uses. The remote key names one volume and is left alone. Claude-Session: https://claude.ai/code/session_01NWpFUwAJcR2KUENrhLc9Sy |
||
|
Chris Lu
|
b77d954f55 |
rust volume: fail closed on sorted-index failures and reconcile tier-up (#10956)
* rust volume: fail closed on sorted-index failures and reconcile tier-up Follow-ups to the .sdx sorted needle map (#10951): - get() folded open/read failures into None, so an EIO, a torn .sdx, or a failed pooled reopen answered reads with NotFound and let do_delete_request acknowledge the delete as Ok(0) without writing a tombstone. It now returns io::Result and every caller propagates; redb's get() had the same shape and is fixed with it. is_file_unchanged cannot propagate, so it reports unknown and logs rather than treating an unreadable index as proof of a change. - A delete whose .idx append landed but whose .sdx mark failed left the map still resolving the old live entry, so deleted content stayed readable until a reload. The map now records the tombstone before touching .sdx and only clears it once the mark lands; lookups consult that first and report the needle deleted, which is what the next reload concludes anyway. - Mode reconciliation ran one way. Entering remote mode made use_sorted_index() true, which returned early, so a volume tiered while the server runs kept its in-memory map and pinned .idx descriptor until restart — the RAM and fd win never applied. It now reconciles in both directions. - Tier-down dropped the remote reference before the fallible refresh, so a failure left volume_info local, the remote backend attached, the .vif still remote, and a retry reporting "already on local disk". The transition is snapshotted and rolled back. - The read-only fallback set no_write_or_delete but left no_write_can_delete, so metrics and mode checks called the volume delete-capable while every delete was refused. * rust volume: count a sorted-map delete against the durable .idx append The deletion counters sat after the in-place .sdx mark, so a mark that failed left them at their pre-delete values while the tombstone was already durable in .idx — and with retries now idempotent, nothing applied them later either. Heartbeats, status responses, and the garbage calculation would report the volume as free of that garbage until a reload. Move them to the append that makes the delete durable, which is also what a reload of .idx would count. Covered by a test that injects a mark failure through a cfg(test) seam: no portable filesystem trick reproduces it, since a read-only .sdx fails the borrow long before the mark. * rust volume: hide a pending tombstone from the sorted-map scans too The overlay that keeps a needle deleted after a failed .sdx mark was only consulted by get(). visit_live_entries still read the stale valid record straight off .sdx, so ascending_visit, iter_entries and save_to_idx all reported the needle live — and compaction takes iter_entries for the complete live set, so it would copy the deleted content forward and save_to_idx would write it back into the rebuilt .idx as live. Snapshot the overlay once per scan and skip its keys, which is the same conclusion the next reload reaches from the .idx tombstone. * rust volume: quarantine a durable write whose index lookup fails The prior-mapping lookup that decides whether to index a fresh append runs after the record is already down and flushed, so a failing lookup leaves exactly the state a failing put leaves: a durable .dat record nothing indexes. The put path marks the volume read only for it; this one returned the error and kept taking writes, and the next append would bury the orphan mid-file where the .dat tail check on reload cannot see it. Give it the same treatment. |
||
|
Chris Lu
|
b58d52ac16 |
rust volume: search .sdx for read-only volumes instead of holding the index (#10951)
* rust volume: search .sdx for read-only volumes instead of holding the index The Go volume server loads every read-only volume through SortedFileNeedleMap: the index lives on disk as a sorted .sdx, a lookup is a binary search, and since #10950 no descriptor is held between lookups. The Rust server had no counterpart. Read-only volumes built a full in-memory CompactNeedleMap, and cloud-tiered ones — noWriteCanDelete, so not the read-only branch — went through the writable path and pinned an .idx append handle on top of it. At the hundreds of thousands of tiered volumes a real server carries, that is an index in RAM and a descriptor each, for volumes nobody reads. Port the sorted map and the bounded handle pool. A tiered volume now costs zero descriptors and zero index bytes when idle; the pool keeps the hot handles open so a busy volume does not pay an open() per needle. Handles are Arc<File>, so an eviction cannot close one a reader still holds. The generated .sdx is byte-identical to Go's — same sort, same last-write-wins, same dropped tombstones — so a volume moved between a Go and a Rust server reads whichever copy is already on disk. A test pins the bytes against a Go-generated fixture. * rust volume: fail compaction on an unreadable .sdx, and rebuild the map on tier-down Two ways the sorted map could lose data. iter_entries swallowed read errors and returned however many entries it managed to collect. Compaction takes that vector for the complete live set, so a truncated .sdx or a mid-scan I/O fault would commit a volume missing every needle past the failure. Return a Result instead and abort. redb's collect_entries dropped errors the same way on the same path, so it goes with it. Tier-down clears the remote mode and publishes the volume as writable, but the map it booted with is the read-only sorted one. Its put always fails, so the first write would append to the local .dat and then fail to index it, leaving bytes nothing references — and a non-fsync write repeats it. Fold the reopen_idx_for_write swap into refresh_remote_write_mode so the map always matches the mode it just published; a rebuild that fails pins the volume read-only rather than letting it take writes it cannot record. Go reaches neither: its tier-down leaves noWriteCanDelete set, so the volume stays read-only until a reload or an explicit mark-writable, which already goes through reopenIdxForWrite. * rust volume: keep read-only volumes mountable on a read-only index dir, and batch the .sdx scan Building .sdx writes to the index directory, and load_index_sorted_file also created a missing .idx there. A volume whose index sits on a read-only mount took both paths and failed to load, where before it mounted read-only off an in-memory index and served reads. Create the .idx only where deletes are allowed, and fall back to the in-memory map when the sorted one cannot be built, so a directory nobody can write costs memory rather than availability. The end-to-end scan behind iter_entries, ascending_visit and save_to_idx read one entry per syscall. Read 1024 at a time instead, the batch size idx::walk_index_file uses. Positional reads, not a cursor: the handle is shared with any other borrower. Also gate the Go byte-parity fixture on the 5bytes feature it describes, which is otherwise dead code in a 4-byte-offset build. * rust volume: roll back a failed writable mark, and rebuild a torn .sdx set_writable clears the read-only flags before it can know the rest will succeed, but only the map rebuild rolled them back. An .idx writer that fails to attach left the volume advertising writable over a needle map with no writer, so puts landed in memory and were gone after a restart — the exact failure the function exists to prevent. The read-only-mount fallback made it reachable: that path loads an in-memory map with no writer attached. All three steps now run behind one rollback point. A .sdx whose length is not a whole number of entries was accepted as long as it looked fresh, and truncation is what makes it look fresh. The entry count then floored, hiding the last needle from lookups and from compaction, which would commit the shorter set. Treat a torn file like a stale one and rebuild it from .idx. Go writes .sdx in place rather than through a temporary, so a crash mid-generation is a real way to produce one. Appends now start at the last whole .idx entry too, so a torn tail there is overwritten by the next tombstone instead of misaligning every row after it. * rust volume: trim a torn .idx before writing to it, keep delete-only volumes online, count sorted-map deletes Three from review. Flooring the sorted map's append offset only protected its own positional writes. Every writable path appends at EOF instead, so a partial row left by a short write pushed the next row off alignment and the following load parsed the rest of the file as garbage. Drop the partial row before attaching any writable index writer — it is unrecoverable anyway, and every loader already skips it. Go refuses to load such a volume at all; trimming keeps it mountable with the rows before the tear intact. The unwritable-index-dir fallback stopped one step short for volumes that allow deletes, which is every tiered one: the in-memory loader opens .idx read-write there and fails on the same directory that just refused the .sdx, so the volume stayed offline. Give up the deletes instead — without a writer no tombstone could be recorded anyway — and a remount on a writable directory restores them. Sorted-map deletes left the counters untouched, so a tiered volume reported itself garbage-free until it restarted. They now land where a reload would put them: the tombstone is another .idx row, and both it and the row it supersedes count as deletions under the rule the load-time metric applies. Go skips this too, and should not. |
||
|
Chris Lu
|
51eb5333d3 |
ec: read a needle's intervals in parallel (#10911)
* ec: read a needle's intervals in parallel A needle spanning more than one EC block gets one interval per block, and consecutive blocks live on different shards. We read those intervals in sequence, so a 4MB chunk landing in a volume's 1MB small-block region cost five round trips to five different servers. Read them concurrently into disjoint slices of a single buffer, at most 8 in flight. Same change in the Rust volume server's phase C. * ec test: seed the random payload instead of the deprecated rand.Read |
||
|
Chris Lu
|
0f85d005ad |
server: 416 only when no requested range overlaps, with Content-Range, and the Rust mirror (#10889)
* filer, volume server: return 416 when no requested range overlaps the content * seaweed-volume: return 416 when no requested range overlaps the content * server: check the range test error, use the request context, fix the no-overlap comment boundary |
||
|
Chris Lu
|
c0a9b110dd |
volume: stop reporting read-only volumes that are no longer here (#10867)
* volume: clear per-collection metrics when a collection leaves a server The read-only and disk size gauges are only ever set for collections the heartbeat still finds here, and nothing zeroes the rest. volume.balance marks a volume read-only to move it, so the last heartbeat that saw it counts it read-only - and if it was the collection's last volume on that server, that count stands until the process restarts. The dashboard then shows read-only volumes that volume.list -readonly cannot find anywhere. Remember what each heartbeat set, and drop what is gone on the next one. * volume: stop the read-only volume count from wrapping at 256 The per-collection counters were uint8, so a server holding 256 read-only volumes of one collection reported zero of them. * volume: read the read-only flags once when counting them The heartbeat asked IsReadOnly for the verdict and then read noWriteOrDelete and noWriteCanDelete straight off the volume, unlocked, so the reasons could disagree with the verdict they were explaining. Take them together, under one lock. The location is now nil-checked rather than skipped by short-circuit evaluation, so a volume that has not joined a disk location yet stays safe. * volume: let only a surviving volume keep its collection reported A volume being deleted for expiry still made an entry in the read-only counts, which is what the cleanup reads as "this collection is still here". The collection's last volume could go and its series would stand for one more heartbeat. Count the survivors only. * volume: size a collection from the volumes it still has The size totals are rebuilt from scratch every heartbeat, so subtracting a volume that is about to be deleted took the surviving volumes' sizes down with it: a collection keeping a small volume and losing a larger one reported the difference, or lost its entry and kept the previous heartbeat's number. * volume: cover the deleted bytes total in the surviving volume test Deleted bytes are totalled the same way as sizes and were going unchecked, so the test now leaves deleted needles on both volumes and pins that gauge too. |
||
|
Chris Lu
|
3bd218e030 |
volume: cut idle memory at high volume counts (#10861)
* volume: start a volume's batch write worker on first use Mounting a volume started a goroutine parked on a 128-slot channel, plus the 128-entry batch slice it had already allocated. That is around 6.7KB per volume the server pays whether or not the volume ever takes a write: 7231 bytes per mounted volume, of which 4101 is goroutine stack. Only a write that asks for fsync ever reaches the worker, and a remote-tiered or read-only volume never can. Create the channel and its goroutine on the first such request instead, and let a write arriving after Destroy fall back to the inline path rather than queue onto a worker that has gone. Measured over 20000 mounted volumes: 7231 -> 1269 bytes each. * volume: update the heartbeat report state in place Every heartbeat built a second map of what it was about to tell the master, holding a freshly allocated short information message per volume, then swapped it in over the old one -- and computed departures through a third map of the live volume ids. A server holding 2M volumes rebuilt all three every VolumePulsePeriod for a report that usually says nothing. Number the heartbeats instead and mark the entry already held with the pass that found the copy, so a quiet volume costs a map lookup and no allocation. Departures are the entries a pass did not mark; the live-id map is now built only when there are some, sized to them. Measured over 10000 mounted volumes: 436 -> 196 bytes allocated per volume per heartbeat. * volume: fill one volume information message per heartbeat, not per volume The heartbeat built a message for every volume held so it could hash it, then dropped all but the few it had something to say about. At 2M volumes that is 2M messages allocated every VolumePulsePeriod to send almost none of them. Fill a message the caller supplies instead, and replace it only when the heartbeat keeps it, so a server with nothing to report fills the same one all the way through. Measured over 10000 mounted volumes: 196 -> 4 bytes allocated per volume per heartbeat, and a heartbeat runs a third faster. * volume: drop the per-volume trace from the heartbeat's status read glog.V(4).Infof evaluates its arguments whether or not the verbosity is on, so every volume boxed its id into a fresh interface slice on every heartbeat: 759 of the 773 allocations a 1000-volume heartbeat made, for a line that at this scale would print millions of unreadable rows. Measured over 1000 mounted volumes: 4776 -> 1792 bytes and 759 -> 14 allocations per heartbeat, which no longer grows with the volume count. * seaweed-volume: mirror the in-place heartbeat report state Same change as the Go volume server: number the heartbeats and mark the entry already held with the pass that found the copy, instead of building a second map of hashes and swapping it in. The volume snapshot must leave the reporting state as it found it, so it keeps asking through changed() while a real heartbeat marks through record(). * volume: refuse writes to a closed volume instead of dereferencing nil Close and Destroy leave the needle map and data backend nil, but a caller that already holds the volume can still reach the write path, where both are used unguarded: a write racing a volume deletion took the server down. syncDelete has always checked; syncWrite and the batch worker had not. Reachable before this series and now also from the inline fallback a durable write takes when the worker has gone. * seaweed-volume: guard the report state with one mutex, as Go does The full-list flag and the generation that answers it have to move together. Split across separate atomics they cannot: a request landing between begin's two reads returns full == false with the generation it just raised, and one landing between commit's read and its clear is marked answered by a heartbeat that carried no list. Either way the resend is dropped. Neither is reachable today -- every caller reaches this through the store's RwLock, the flag setters under a read lock and the heartbeat build under a write lock, so they cannot interleave. The type should not depend on that being true two files away, and Go holds a single mutex over exactly these fields. * test: build the servers under test to match the harness's offset size The mixed Go/Rust suites run both servers against one dataset, so both have to agree on the offset width. They did not: the harness built Go with no tags, 4-byte offsets, while the Rust crate defaults to its 5bytes feature, and the Rust server then refused the .vif the Go server had just written -- "bytes_offset mismatch: found 4, expected 5". Build each side to match the offset size the test binary itself was compiled with, so a plain `go test` and one with -tags 5BytesOffset both get a matched pair. |
||
|
Chris Lu
|
e0c4732e5e |
rust: stop writing when a durable write's index flush fails (#10825)
* rust: stop writing when a durable write's index flush fails A durable write flushes the .dat, publishes the needle map row, then flushes the .idx. If that last flush failed we returned the error and carried on: the row stayed live, the volume stayed writable, and the handler answered 500 without replicating. The primary then served a needle its replicas never saw, for a write the client was told had failed - and if the unflushed row was lost on restart, the durable .dat tail took the volume read only anyway. Taking the row back out is not an option: it means undoing published state on a disk that is already failing, and a truncate afterwards would leave an .idx row pointing past the end. So the volume stops taking writes instead, the same as when the truncate after a failed .dat flush cannot be done. Nothing more gets appended past a record whose index is in doubt, and the master routes writes elsewhere once the volume heartbeats read only. The divergence against the replicas is still there, but it is bounded and it is visible. A failed nm.put after the .dat is down leaves the same durable but unindexed record, so it takes the same route. * rust: drop the import the rollback removal left behind NeedleValue came in with rollback_unflushed_write, which went away when the durable path moved to flushing before it publishes. Nothing has used the type since. * rust: mark the test-only heartbeat helper as such collect_heartbeat has only ever been called from the tests - the send loop uses collect_heartbeat_with_snapshot, which it wraps - so a lib build rightly called it dead code. * rust: flush the index on a durable write that dedups A durable write matching content already in the volume flushed the .dat and returned before reaching the index flush. So a fsync=true write that deduped against an earlier non-durable one was acked with the row that indexes it still in the page cache - the same false promise the index flush exists to rule out, and the same read-only volume on restart if the row is lost. The dedup path now flushes both files, and the quarantine on a failed index flush moved into flush_idx so it applies wherever the flush is reached rather than only at the one call site that had it inline. |
||
|
Chris Lu
|
887910b377 |
rust: honor fsync on the volume server write path (#10816)
The Rust volume server ignored the fsync parameter completely: nothing parsed it, and write_volume_needle -> write_needle -> append_needle never flushed. So a ?fsync=true upload was acked out of the page cache, and since ReplicatedWrite forwards the parameter, a Go primary handing a durable write to a Rust replica got the same empty promise. The upload handler now reads fsync the way Go's r.FormValue does, off the decoded query fields, and threads it down to the volume. A durable write appends, flushes the .dat, publishes the needle map entry, then flushes the .idx, and only then is it acked. Nothing points at bytes that are not down yet, so a failed flush only has to take its own append back off the end - the index never moved and the volume's counters never saw the rejected write. If that truncate cannot be done the volume stops taking writes, rather than letting a later append bury the rejected record mid-file where the tail integrity check cannot see it. The .idx flush is what keeps the ack honest: load() rebuilds the map from .idx, so an acked write whose row was lost comes back as a .dat tail the integrity check cannot account for, and the volume loads read only. A dedup hit flushes too: there is nothing to append, but the write it matched may have been non-durable, and the caller is asking for the content to be on disk. Batched writes carry the flag per request rather than one flush per batch, so the write queue's module doc no longer claims otherwise. |
||
|
Chris Lu
|
d713ab49f9 |
volume: validate replica targets and restrict gcs credentials in FetchAndWriteNeedle (#10755)
* volume: validate replica upload targets in FetchAndWriteNeedle The replica leg forwarded the fetched needle to a caller-supplied address without checking it, so a malformed target could redirect the upload to an unintended host or path. Require each replica target to be a bare host:port whose host is not loopback / link-local / unspecified, reusing the address deny-list; cluster peers legitimately sit on private networks, so RFC 1918 / CGNAT stay allowed and -volume.allowUntrustedRemoteEndpoints still opts out. Validate every target up front so a bad one fails the request before the local write, and upload through a client that re-checks the resolved address at connect time so a replica hostname cannot rebind to a blocked address after validation. Mirrored in Rust (validation moved ahead of the local write; the Rust S3 path's connect-time re-check is still a follow-up there). * volume: only accept inline gcs credentials in FetchAndWriteNeedle The gcs credentials value on this request could name a local filesystem path, which the SDK reads from disk. Accept only inline JSON here; the server-side GOOGLE_APPLICATION_CREDENTIALS env var still supplies a path. The Rust volume server has no gcs backend, so there is nothing to mirror. |
||
|
Chris Lu
|
ae2cc8225e |
rust volume: mirror the VolumeConsolidateIndex RPC from Go (#10752)
The Go volume server has VolumeConsolidateIndex, which moves a volume's .idx out of the data directory into the configured -dir.idx directory (where an EC decode/reconstruct can leave it co-located) and reloads the volume in place. The Rust port's proto omitted the RPC entirely, so its generated VolumeServer trait was one method short of Go's. Add the proto message and rpc, the gated grpc handler, and Store::consolidate_volume_index / Volume::relocate_index_to, mirroring Go's Store.ConsolidateVolumeIndex and Volume.RelocateIndexTo -- including the cross-device copy fallback and the reopen-against-the-old-dir path when the move fails. Integration tests cover the real move (index relocated, volume still serves reads and the move is idempotent), the no-op paths (index already in place, no separate idx dir) and the not-found error, plus the grpc handler end to end. |
||
|
Chris Lu
|
c0f33d599b |
rust volume: mirror Go volume server logic to gate the admin RPCs (#10748)
rust volume: gate the remaining admin RPCs behind check_grpc_admin_auth
The Go volume server gates 29 destructive VolumeServer RPCs on the
-whiteList admin check; the Rust port only gated 14. Add the gate to the
other 15 -- batch_delete, read_all_needles, fetch_and_write_needle, the
EC-shard generate/rebuild/copy/unmount/to-volume RPCs, both tier-move RPCs,
volume_copy, volume_tail_receiver, set_state, scrub_ec_volume and
volume_needle_status -- so a configured whitelist restricts them the same
way it already does on the Go side.
check_grpc_admin_auth also required peer info before checking whether any
control was configured, unlike Go's `if vs.guard == nil { return nil }`.
Short-circuit when no whitelist and no signing key are set, so in-process
callers keep working with security inactive and only the gate ordering
changes for configured servers.
tests/admin_auth_coverage.rs mirrors the Go coverage test: every handler
must either gate or be listed as intentionally open with a reason, so the
two implementations can't silently drift apart again.
|
||
|
Chris Lu
|
76d3fd0e9d |
grpc: optional client_cert/client_key for outgoing mTLS connections (#10747)
* grpc: optional client_cert/client_key for outgoing mTLS connections * scaffold: list client_cert/client_key in each grpc section |
||
|
Chris Lu
|
c6e1387f59 |
shell: multi-target fs.mergeVolumes and volume.mark -readonlyCanDelete (#10706)
* shell: fs.mergeVolumes distributes one volume across multiple -toVolumeId targets * volume: volume.mark -readonlyCanDelete rejects writes but keeps accepting deletes * seaweed-volume: mirror readonlyCanDelete volume state |
||
|
Chris Lu
|
a2ffc7aadf |
heartbeat: keep the master current through collection churn (#10657)
* heartbeat: name departed volumes in delta heartbeats * master: release the lookup index with a deleted collection * master: keep a fresh grow safe from the report that raced it * volume: name the volumes a deleted collection took with it Deleting a collection left the master to work out what went by omission from the next full volume list, which it no longer gets: heartbeats carry the whole list only when the master asks for it. The volumes a bucket's churn creates and destroys between two of those requests are never named in either direction, so the master keeps counting their slots as occupied and a cluster that creates and drops collections quickly runs its free-slot accounting dry -- assigns fail with no free volumes left while the disk holds a handful of volumes. The destroy path already knows exactly which volumes it removed, so send them down the same channel every other deletion uses. * rust: name the volumes a deleted collection took with it Mirrors the Go volume server. The notify path derives its deltas by diffing snapshots, so a collection delete that does not wake it is invisible until the master next asks for the whole list. |
||
|
Chris Lu
|
ce7d388639 |
heartbeat: send only the volumes that changed (#10640)
* pb: let a heartbeat carry only the volumes that changed A partial list cannot travel in volumes: a master that did not understand it would read the absences as deletions. So changes get their own field, used only once the master has said it compares digests and can tell when it has fallen behind. * master: apply the volumes a heartbeat reports as changed Only the named volumes are touched. A full report says the server holds exactly these; a changed report says nothing about the ones it leaves out, so absence must not read as removal. Also advertises that the master compares digests, which is what lets a server stop sending its whole list. Advertising it once per connection means a server reconnecting to a master that does not is back to full lists straight away. * volume: send only the volumes that changed once the master accepts them The whole list goes on every heartbeat until the master says it compares digests, and again whenever it asks, so a master that cannot tell when it has fallen behind never has to. has_no_volumes stays derived from a full list alone. Deriving it from what a heartbeat happens to carry would make a quiet one read as a server that had lost every volume, and the master would drop them all. The digest still covers every volume held rather than the ones sent, which is what lets the master confirm that applying the changes left it current. Reporting state is per-connection: a server that reconnects, or reaches a different master, starts again from the full list. * volume: let the zero reporting state stand for having told no master anything A Store built as a literal, which tests do, left the reporting state nil and panicked on the first heartbeat. As a value its zero form already means nothing has been reported to anyone, which is exactly the state that sends the whole list. * rust: send only the volumes that changed once the master accepts them Mirrors the Go volume server, with one hazard the Go side does not have: mount and unmount deltas here are derived by diffing successive heartbeats, so a heartbeat that carries a partial list would report every volume it left out as unmounted. Collecting now returns the full set alongside the message, and every site that diffs uses that rather than what went on the wire. * volume: do not let a full-list request be lost to the heartbeat it raced The request arrived while a heartbeat was already being built as a delta, and committing that heartbeat cleared it, so the master waited for another digest mismatch before asking again. Count the requests and clear only the one the heartbeat answered. * rust: stop marking volumes reported by a heartbeat that is thrown away The state-notify path collected a heartbeat only to diff its volume list, then sent a delta message of its own and dropped the one it had collected. Once collecting recorded what the master had been told, every mount or unmount silently marked the changed volumes as sent, and the master learned of them only after a digest mismatch. Snapshotting no longer records anything, and no longer expires ec volumes whose deletion that path was already discarding. * master: announce only the volumes a change actually brought Every changed volume was broadcast as a new location. Volumes grow constantly and growth moves no location, so on a busy cluster that told every connected client about volumes it could already reach, filling bounded broadcast queues and pushing out the topology updates that matter. * master: ask for the full list when only one can repair the master Delta heartbeats stop the full report, and with it the only thing that re-registers a volume the lookup index lost. The volume server cannot see that divergence and its digest cannot show it, so the master now checks its own two indexes agree and asks for the list when they do not. A node reporting one volume id twice is kept on full lists for the same reason rather than merely skipped: its digest can never be verified, so nothing else would tell the master what it had stopped holding. * master: keep the volume options on every heartbeat response A volume server takes them from whatever response arrives, and preallocate is a bare bool with no way to tell off from unmentioned. A response sent to ask for the volume list therefore turned preallocation off until the server reconnected. Responses sent mid-stream now start from the configured options rather than being built field by field. * master: announce a volume the lookup index had lost Repairing the index makes the volume servable again, but clients were told it went when the node dropped out and nothing told them otherwise: the disk map still held it, so it did not count as an arrival. Reaching the lookup index is what makes a volume servable, so recovering an entry there is an arrival as far as clients are concerned, on both the full report and the changed-volume path. |
||
|
Chris Lu
|
6d08b08f37 |
heartbeat: carry a volume digest and verify it (#10627)
* pb: carry a volume digest on the heartbeat The full volume list is the only way a master notices a volume that vanished without a delta, so it cannot simply be dropped. A digest gives the same guarantee without the list, and a way back to the list when they disagree. The digest has explicit presence: a server holding no volumes reports 0, which has to stay distinguishable from a server that does not compute one at all. * volume: report a digest of the volumes each heartbeat carries Digests exactly what goes on the wire: volumes skipped as quarantined, phantom or expired are absent from both the list and the digest, so the master compares against the same set the server meant to report. Runs the master's own hash over the master's own conversion of the message, so the two ends cannot drift into disagreeing about a field. * master: check the reported volume digest and ask for the list on a mismatch Compared after everything the heartbeat carried has been applied, so agreement means the master is current rather than that nothing changed. Servers reporting no digest are untouched, and a mismatch on a heartbeat that already carried the full list is reported rather than answered: there is nothing further to ask for, so asking again would loop. Nodes reporting one volume id twice are skipped for the same reason. * rust: report the heartbeat volume digest Mirrors the Go volume server. The master compares this against a digest it computes itself, so the hash has to agree byte for byte across the two implementations, not merely be a hash of the same fields: report_hash_vectors pins it against values generated by the Go side, and the ttl and replica placement narrowing the master applies when it decodes a message is applied here too rather than assumed away. A drift there would not corrupt anything, but every volume server on this implementation would report a digest the master can never match and fall back to sending its whole volume list forever, which is the cost the digest exists to avoid. * master: pin what the digest check does to each kind of report The upgrade story rests on these: a server that reports no digest is never asked for anything, so the two sides can be upgraded in either order, and a disagreement that resending cannot fix is reported rather than re-asked, so it cannot loop. * topology: enumerate the digest coverage test from the message The list of fields was written out by hand, so a field added to VolumeInformationMessage later would fall outside the digest while the test went on passing, and a change to it would never reach the master. Walk the message descriptor instead. Some fields are narrowed or normalised on the way into VolumeInfo, so the smallest change to the wire value can land back on the stored one; the test offers several values per field and asks only that some change is visible. |
||
|
Chris Lu
|
9351202ca9 |
volume: scan for on-disk EC shards when staging a decoded volume (#10465)
The staged-new-volume placement skipped a disk holding the vid's EC shards using only the in-memory ecVolumes map, missing a shard present on disk but not mounted. Also scan the candidate disk for <vid>.ecNN files, so the promise holds regardless of mount state. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
Chris Lu
|
3b3e8af430 |
volume: skip a shard-holding disk when staging a decoded volume (Go+Rust) (#10464)
volume: skip a shard-holding disk when staging a decoded volume ReceiveFile staged-new-volume mode picked any free disk of the target medium. Skip a disk that already holds the vid's EC shards (Go DiskLocation.FindEcVolume / Rust ec_volumes), so a decoded .dat never lands in the same directory as a shard. This lets a caller safely stage onto a shard host that has a spare disk, instead of requiring a host with no shard of the vid at all. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
Chris Lu
|
9c37e52c9b |
volume: EC decode onto a clean peer via staged-new-volume adopt (Go+Rust) (#10463)
Decoding EC shards back to a normal volume in place reconstructs <vid>.dat
in the shards' own directory, so the vid is momentarily registered as both
an EC and a normal volume in one location — the load/scan path then sees it
as both, risking mount ambiguity and needle loss. VolumeEcShardsToVolume
still supports that in-place path; this adds the primitives to decode onto
a *clean* peer instead:
- ReceiveFile gains a staged-new-volume mode: when the volume does not
exist here and ReceiveFileInfo.disk_type is set, pick a free-slot disk
of that medium and write <base><ext>.copying (not a valid volume name,
so the scanner never half-loads a partial push).
- VolumeEcShardsToVolume gains from_staged: adopt the pushed .dat/.idx/
.vif — rename .copying into place under a .note in-progress marker,
then mount — so <vid> lands on the peer only as a normal volume.
The caller decodes the shards off-box and streams the finished volume to a
peer holding no shard of the vid on the target medium. Go and Rust volume
servers get identical handlers. Proto: ReceiveFileInfo.disk_type (12; 8-11
reserved for versioned-EC), VolumeEcShardsToVolumeRequest.from_staged (3) +
disk_type (4).
Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu
|
||
|
Chris Lu
|
84d3d62697 |
rust volume: mark-readonly notifies the live leader, not the static seed (#10461)
VolumeMarkReadonly mutates raft-replicated master topology, so it must reach the leader. notify_master_volume_readonly targeted the static seed (config.masters.first()), so after any master failover it hit a follower and failed "not current leader". Prefer current_master_url (the live leader the heartbeat tracks), fall back to the seed before the first heartbeat, mirroring store_ec.rs and Go's vs.GetMaster(). Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
dependabot[bot]andChris Lu
|
186a72c39d |
build(deps): bump rand from 0.8.5 to 0.10.2 in /seaweed-volume (#10428)
* build(deps): bump rand from 0.8.5 to 0.10.2 in /seaweed-volume Bumps [rand](https://github.com/rust-random/rand) from 0.8.5 to 0.10.2. - [Release notes](https://github.com/rust-random/rand/releases) - [Changelog](https://github.com/rust-random/rand/blob/master/CHANGELOG.md) - [Commits](https://github.com/rust-random/rand/compare/0.8.5...0.10.2) --- updated-dependencies: - dependency-name: rand dependency-version: 0.10.2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> * rust volume: follow the rand 0.10 renames thread_rng is now rng, the Rng extension trait is RngExt, and RngCore is Rng. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
5a54beac80 |
EC decode: read shards with the encode-time block layout (#10385)
* erasure_coding: WriteDatFile takes the encode-time dat size for the shard block layout * volume server: derive EC decode layout from the encode-time dat size, not the live extent * erasure_coding: test decode after tail deletions shrink the live extent below a large-block row * seaweed-volume: write_dat_file_from_shards takes the encode-time dat size for the shard block layout * seaweed-volume: derive EC decode layout from the encode-time dat size, not the live extent * seaweed-volume: test decode after tail deletions shrink the live extent below a large-block row * erasure_coding: reject decoding with no data shards * worker: record the encode-time dat size in the .vif * erasure_coding: fall back to the shard-derived layout only when the encode-time dat size is missing * erasure_coding: reject an ambiguous shard-derived block layout * seaweed-volume: fall back to the shard-derived layout only when the encode-time dat size is missing * seaweed-volume: reject an ambiguous shard-derived block layout |
||
|
Chris Lu
|
267f595660 |
batch delete: align the shard test and Rust server with continue-past-mismatch (#10349)
Commit
|
||
|
Chris Lu
|
c1a1e3c1e3 |
shell: volume.tier.upload keeps volume replicas (#10314)
* volume: copying a remote-backed volume only needs space for the index VolumeCopy sized its target-location check by the source .dat even when that .dat lives in a cloud tier and only .idx/.vif land locally, so re-replicating a tiered volume demanded the full remote size in free disk. Require the index size instead. * shell: volume.tier.upload keeps volume replicas Tiering a replicated volume deleted every replica but the upload source, leaving one server holding the only .idx and the only .vif that knows the remote object key — losing that server orphaned the volume even though its data sat intact in the cloud. Replicate the uploaded .idx/.vif onto the other replica servers instead (VolumeCopy skips the .dat for remote-backed volumes), so all replicas serve reads from the same remote object and the volume keeps its replica count. An already-tiered replica is preferred as the upload source, so a rerun after a partial failure reuses the existing remote object instead of uploading a second copy under a new key. * shell: group tier upload locations instead of re-prepending * rust volume: copying a remote-backed volume only needs space for the index Mirror the Go VolumeCopy change: size the free-location check by the source .idx when the .dat lives in a cloud tier, since only .idx/.vif land locally. |
||
|
Chris Lu
|
e98cbfc8f1 |
seaweed-volume: async, buffered writes in VolumeEcShardsCopy (#10237)
* seaweed-volume: async, buffered writes in VolumeEcShardsCopy The EC-shards-copy RPC handler wrote each streamed chunk to disk with a synchronous std::fs::File::write_all inside the async handler, blocking a Tokio worker thread for the duration of every write — noticeable for a large .ecx on a slow or busy disk. Factor the five near-identical receive-and-write loops (.ec shards, .ecx, .ecj, .vif, .ecsum) into drain_copy_stream_to_file, which uses tokio::fs + BufWriter for async, buffered I/O. Behavior is otherwise unchanged: the .ecj append mode, the .ecsum byte count and 0-byte-file cleanup, and all error messages are preserved. Claude-Session: https://claude.ai/code/session_01Ny5Rt1ph9VWeKmfY936GtF * seaweed-volume: remove partial copy target on error in EC-shards-copy Follow-up: drain_copy_stream_to_file now deletes the destination file on any recv/write/flush error, so a failed VolumeEcShardsCopy no longer leaves a truncated .ecNN/.ecx/.ecj/.vif/.ecsum on disk for a later reader to trip on. Matches receive_file / the Go volume server. Best-effort cleanup; the original stream error is still returned. Claude-Session: https://claude.ai/code/session_01Ny5Rt1ph9VWeKmfY936GtF |
||
|
Chris Lu
|
c332323b01 |
rust volume: pin rustls to aws-lc-rs so TLS gRPC startup doesn't panic (#10233)
aws-lc-rs and ring both get linked transitively, so rustls can't auto-select a crypto provider and tonic's client TLS panics the moment the volume server dials a master over TLS. Install aws-lc-rs as the process default in main(), matching the provider the server config already uses. |
||
|
|
cc4043c9d2 |
fix(volume [rust]): compare live compaction_revision instead of stale last_compact_revision (#10189)
* fix(volume [rust]): compare live compaction_revision instead of stale last_compact_revision * fix(volume [rust]): compare live compaction_revision instead of stale last_compact_revision - unit tests * s3: invalidate stale reader cache locations on chunk read failure (#10156) * s3: invalidate stale reader cache locations on chunk read failure * filer: share the chunk-read self-heal across reader cache and streaming paths The reader cache retry added a third copy of the invalidate-relookup-compare-retry dance already inlined in PrepareStreamContentWithThrottler and duplicated in retryWithCacheInvalidation. Extract retryFetchWithFreshLocations and route all three through it, parameterized by the refetch primitive. * filer: drop redundant completedTimeNew store in reader cache success path startCaching already stamps completedTimeNew unconditionally before the fetchErr branch; the second store inside the success branch is dead. * filer: make NewReaderCache cache invalidator an explicit parameter The variadic ...CacheInvalidator only ever read the first element, so a caller could pass two and silently get one. Take a single explicit argument and have the non-S3 callers pass nil. * filer: inject reader cache chunk fetch as a struct field Replace the process-global readerCacheFetchChunkData test seam with a per-instance fetchChunkDataFn field defaulted in NewReaderCache, matching how lookupFileIdFn is already wired. Tests set the field on the cache instead of swapping a shared global. * filer: log the location count, not full URLs, on self-heal retry --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> * fix(shell): honor explicit fs.mergeVolumes from/to direction (#10159) * fix(shell): honor explicit fs.mergeVolumes from/to direction mergeVolumes only ever merged a smaller volume into a larger one. When the user named both -fromVolumeId and -toVolumeId with the source larger than the target, the planner produced an empty plan and the command printed just "max volume size: N MB" and moved nothing. Build the requested pair directly when both ids are given, instead of routing through the size-descending heuristic. Read-only, empty, and wrong-collection endpoints are rejected with a clear error rather than a silent no-op. * fix(shell): allow fs.mergeVolumes into an empty target volume Merging chunks into an empty volume is valid, e.g. consolidating data into a freshly created or recently vacuumed volume. Only reject an empty source, which has nothing to move. * fix(shell): reject self-map in directed mergeVolumes planner createMergePlan with from == to returned a {vid: vid} self-merge when called directly. Guard it in the planner so it is correct independent of the Do entrypoint. * fix(volume [rust]): compare compaction_revision in u32, not truncated u16 `req.compaction_revision as u16` truncates any request value above 65535, so a stale revision of 65537 aliases to a live revision of 1 and the "is compacted" guard wrongly passes. Widen the volume's revision to u32 and compare there, matching Go's uint32(v.CompactionRevision) != req.CompactionRevision. --------- Co-authored-by: adri <adri@digitalunited.net> Co-authored-by: Aleksey <48918167+MilanFun@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
cf64cafc3b |
volume: drop stale volume-location cache on under-replication (#10185)
* volume: drop stale volume-location cache on under-replication A replicated write looks up the volume's locations and caches them for 10 minutes. When the master briefly reports fewer replicas than the copy count (e.g. a stale heartbeat drops a just-added volume), that under-replicated result got cached, so every write failed with "replicating operations is less than replication copy count" until the entry expired -- long after the master re-registered the replica. Invalidate the cached entry when the location count is below the copy count, so the next write re-queries the master and recovers as soon as it heals. * volume: mirror the replication copy-count guard in seaweed-volume do_replicated_request accepted a write even when the master reported fewer locations than the volume's copy count, silently under-replicating. Reject it, matching Go's GetWritableRemoteReplications. lookup_volume is uncached, so the next write recovers as soon as the missing replica re-registers. |
||
|
Chris Lu
|
f6032cf23d |
fix(ec): read chunk-manifest chunks stored on EC volumes (rust volume server) (#10187)
* fix(ec): read chunk-manifest chunks stored on EC volumes Chunk-manifest expansion read every chunk through store.read_volume_needle, which only resolves a local regular volume. Once a chunk's volume is EC-encoded, that lookup returns NotFound and the GET fails 500 with "read chunk ...: not found", so a chunked object over an EC tier is unreadable even though its parity is intact and reconstructable. Resolve each chunk to wherever it lives — a local regular volume, a local EC volume (reconstruct-on-read from the surviving shards), or a peer via master lookup — matching Go's ChunkedFileReader, which never assumes chunks are local regular needles. * fix(ec): validate the chunk cookie on local manifest chunk reads A chunk fetched from a peer is cookie-checked by that peer's GET handler, but the local regular and EC reads returned data without comparing the needle's cookie to the one in the chunk fid. Check it, matching the main GET paths, so a stale or guessed id can't serve another needle's bytes. * fix(ec): clamp manifest chunk copy to its declared size Expansion writes each chunk into result[offset..] by offset, so a chunk whose bytes exceed its declared size could overwrite the next chunk's window. Clamp the copy to chunk.size (and reject a negative size) so an over-long or malformed chunk stays within its own range. |
||
|
Chris Lu
|
b4a99b996d |
feat(ec): EC bitrot CHECKSUM scrub on the Rust volume server (#10154)
* proto: add EC bitrot checksum messages + CHECKSUM scrub mode Mirror weed/pb/volume_server.proto byte-for-byte (field numbers + types) so the .ecsum sidecar payload is wire-identical across the Go and Rust binaries: EcBitrotProtection / EcShardChecksums / ChecksumAlgorithm, VolumeScrubMode.CHECKSUM=4, and VolumeEcShardsCopyRequest.copy_ecsum_file. No code uses them yet — the .ecsum format, producer, mount-load, copy, and scrub land in following commits. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * feat(ec): port the .ecsum bitrot checksum module ec_bitrot.rs mirrors weed/storage/erasure_coding/ec_bitrot.go: the .ecsum sidecar format (14-byte big-endian ECSU header + CRC32C over a prost-serialized EcBitrotProtection payload), the per-shard per-block CRC32C producer (ShardChecksumBuilder), save/load with payload self-integrity, manifest validation, status resolution, and verify_shard_file_blocks for the CHECKSUM scrub. A byte-exact test pins the serialized bytes against the Go reference's identical constant so a format drift in either binary fails loudly. Producer wiring (encode/vacuum), mount-load, copy, and the mode-4 dispatch land in following commits. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * test(ec): pin .ecsum sidecar bytes for cross-binary interop Deterministic EcBitrotProtection -> exact on-disk bytes, asserted against a canonical constant on BOTH sides (this test and ec_bitrot.rs), so a format drift in either binary fails its own suite rather than silently desyncing a Go-written .ecsum from a Rust-written one. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * feat(ec): write the .ecsum bitrot sidecar during EC encode write_ec_files now feeds each shard's bytes through a per-shard ShardChecksumBuilder as it writes them, then persists the generation-0 sidecar (<base>.ecsum) alongside the shards — mirroring weed's WriteEcFiles + SaveBitrotSidecar. Best-effort: a failed sidecar write leaves the generation unprotected rather than failing the encode. A test confirms the produced sidecar validates and its per-block CRCs match every on-disk shard. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * feat(ec): load the .ecsum at mount + EcVolume::checksum_scrub EcVolume now loads and validates its generation-0 .ecsum sidecar at mount, caching the parsed protection + BitrotStatus (Off/On/Invalid), and exposes bitrot_protection() mirroring Go's EcVolume.BitrotProtection(). checksum_scrub() verifies every locally-held shard's raw bytes against the sidecar block CRCs — the only path that exercises cold parity shards — reporting mismatched shards without mutating anything; a wholesale mismatch beyond parity is flagged as a suspect sidecar rather than mass shard corruption. Mirrors Go's ChecksumScrub. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * feat(scrub): dispatch EC CHECKSUM (mode 4) to checksum_scrub Accept VolumeScrubMode.CHECKSUM=4 and route it to EcVolume::checksum_scrub, accumulating blocks scanned + mismatched shards into the scrub response, plus the CHECKSUM scrub-mode metric label. Read-only bitrot verification over local shards. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * feat(ec): copy the .ecsum sidecar during VolumeEcShardsCopy Honor copy_ecsum_file: when set, copy the generation-0 .ecsum alongside the shards so protection travels with them, mirroring Go's non-2PC copy path. Tolerant of a missing source (empty stream) — the 0-byte file is dropped so mount sees no sidecar (protection off) rather than a truncated/invalid one. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * feat(ec): remove the .ecsum sidecars when destroying an EC volume remove_ec_volume_files now clears <base>.ecsum (and any versioned .ecsum.v<N>) from the data and idx dirs, so a vid reuse can't load a stale sidecar. Mirrors Go's removeBitrotSidecars. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * style(ec): align bitrot comments and test setup for merge-cleanliness Match the shared bitrot code (write_ec_files, encode_one_batch, checksum_scrub, the encode sidecar test) to the canonical wording/layout so the volume-server Rust port stays line-aligned across trees, keeping periodic merges conflict-free. No behavior change. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo |