The codec streaming GET reader divides by erasure.block_size in
build_codec_streaming_part_reader without validating the erasure
dimensions, unlike the legacy multipart path which already rejects
block_size==0 / data_shards==0. FileInfo::is_valid() does not check
block_size, so corrupted on-disk metadata (block_size==0, data_blocks>0)
passes validation and panics the read task with a divide-by-zero.
Add Erasure::has_valid_dimensions() and reject invalid dimensions at the
codec streaming entry before any disk access, mirroring the legacy guard
(which now reuses the same predicate).
Refs backlog#868 (868-1).
Two compounding defects let a truncated shard corrupt a GET (ranged GET could
return HTTP 200 with wrong bytes):
- `BitrotReader::read` returned `Ok(short_len)` when the shard stream hit EOF
before filling the caller's buffer. With `skip_verify` /
`HashAlgorithm::None` / parity=0 there is no hash to catch it, so the short
shard was accepted and every downstream byte shifted.
- `ParallelReader.offset` was set once and never advanced per stripe, so every
stripe after the first reused the first stripe's geometry and the last-stripe
length clamp was wrong.
Fix both (they are mutually required):
- `BitrotReader::read` now errors (`UnexpectedEof`) on a short read, before and
independent of the bitrot hash check, so it fires under skip-verify/no-hash
too. The caller sizes the buffer to the expected per-stripe shard length, so
"buffer not filled" == "shard truncated". A short read routes through the
existing `errs[i]` path, dropping that reader from the stripe so parity
reconstruction engages; with parity=0 the stripe fails read quorum and the GET
errors loudly instead of streaming shifted bytes. Mirrors MinIO's
`parallelReader.Read` (`n != shardSize` -> reader failed).
- `ParallelReader::read` / `read_lockstep` advance `self.offset += shard_size`
per stripe so the per-stripe expected length (incl. the shorter final stripe)
is exact, matching the correct pattern already used by heal.
Updates three bitrot tests that used an obsolete oversized-buffer pattern to
size per-stripe (as the real decode/heal paths do), and adds a regression test
that a truncated shard errors under None/HighwayHash + skip_verify.
Refs backlog#799 (B2), issue rustfs/backlog#851. Design converged by two
independent expert reviews referencing MinIO cmd/erasure-decode.go.
Second TODO-convergence round over the current tree (backlog#646). All
line numbers in the old inventory had gone stale after the set_disk /
diagnostics / cluster refactors, so this re-scans and reduces the marker
count from 144 to 99.
STALE removals (comment describes already-implemented behavior, or dead
commented-out blocks) across ecstore (set_disk ops/core, store,
cluster/rpc, bucket/metadata_sys, services), iam, filemeta, s3select and
rustfs auth/object_usecase. No behavior change.
Safe fills, each verified:
- filemeta: replication_info_equals now also compares
replication_state_internal (function currently has no callers; adds a
regression test).
- bitrot: drop the confirmed-unused `_want` parameter from bitrot_verify
and the now-unused `sum` on LocalDisk::bitrot_verify, removing a
Bytes::copy_from_slice allocation. Streaming verify uses the file's
embedded per-shard hash, never the passed sum.
- signer: rename v4_ignored_headers -> V4_IGNORED_HEADERS and drop the
non_upper_case_globals allow.
- admin/heal: test_decode was #[ignore]d and used serde_urlencoded on a
JSON body (would panic); rewire to serde_json::from_slice to match the
production decode path, add assertions, un-ignore.
Verified: cargo fmt; cargo check on touched crates; tests pass
(filemeta, signer, bitrot, heal::test_decode); arch guardrail scripts
pass.
`MultiWriter` recorded per-writer failures only in a private `errs` vector and
nulled the failed writer locally, but `put_object` never used that to prune the
disk set: `rename_data` ran over the full `shuffle_disks`, so a disk that took a
short/failed write still had its truncated shard renamed into place and counted
as an online disk. The object then claimed N good shards while one was
short/corrupt, so a single later disk failure could drop it below reconstructable
quorum — silent data loss.
- `write_shard`: null the writer on a generic write error too (not only on
`ShortWrite`), so a failed writer is uniformly represented as `None` — matching
`shutdown_writer`, which already does this.
- `put_object`: after encode, drop every disk whose writer failed
(`drop_failed_writer_disks`) from `shuffle_disks` before `rename_data`, and
re-check write quorum over the survivors. `rename_data` already re-checks
quorum and rolls back if too few disks remain, so excluded disks are neither
committed nor counted (MinIO sets failed writers to nil before `renameData`).
Adds unit tests for the exclusion/quorum accounting.
Refs backlog#799 (B3).
fix(ecstore): lockstep stripe read to stop EC GET desync truncation
On the reconstruction-verifying GET path, ParallelReader::read used a
data-first schedule: it read only `data_shards` readers per stripe and
pulled in a parity reader as a substitute on demand. Because the shard
readers are streaming (advanced only by being read, no seek), a parity
reader first used mid-object was still positioned at its stream start
(block 0) and returned an earlier stripe than the surviving data shards.
Every shard passed its own bitrot hash, yet the set was mutually
misaligned, so decode_data_with_reconstruction_verification correctly
rejected it with "inconsistent read source shards" and the large-object
GET truncated mid-stream (client "unexpected EOF").
Add read_lockstep(): on the verify_reconstruction path, read every live
shard reader once per stripe and wait for all of them, so all readers
advance one block per stripe and stay mutually aligned; any reader that
errors is retired for the rest of the object (a stream that failed
mid-block can no longer be trusted to be aligned). The adaptive
data-first path is unchanged for non-verifying callers (e.g. heal).
Adds a regression test reproducing a data shard that dies partway through
a multi-stripe object; it must still reconstruct byte-exact output.
Refs backlog#832.
* fix(rio): propagate http writer shutdown errors
* fix(ecstore): unify remote lock rpc deadlines
* fix(storage): reject corrupt read multiple payloads
* feat(rio): add internode http tuning profiles
* feat(metrics): add internode baseline signals
* feat(ecstore): observe shard locality topology
* feat(ecstore): gate shard locality scheduling
* feat(ecstore): gate batch read version rpc
* feat(ecstore): observe batch processor adaptation
* feat(ecstore): gate batch processor observation
* docs: add get benchmark regression analysis
* docs: add issue 797 execution plan status
* fix(ecstore): require explicit batch rpc support
* fix(ecstore): honor documented batch read gate
* fix(ecstore): keep batch read gate stable per call
* chore: update workspace dependencies
* feat(ecstore): log batch read gate decisions
* feat(ecstore): count batch read gate decisions
* test(issue-797): add local internode A/B runner
* test(rio): fix tuning profile spelling fixture
* fix(protocols): adapt sftp channel open callbacks
* fix(metrics): wrap batch processor observation args
* chore(docs): keep issue notes local only
* fix(storage): address internode review feedback
* fix(storage): address internode data-path review findings
- Run the BatchReadVersion auto-mode unary fallback outside the batch
RPC deadline so each read_version keeps its own per-op timeout and
health accounting instead of racing the whole batch against one
drive timeout.
- Cap adaptive batch-processor concurrency growth at a hard multiple
of the configured baseline so sustained fast batches cannot ratchet
past the configured limit.
- Parse RUSTFS_INTERNODE_HTTP_* tuning, RUSTFS_BATCH_PROCESSOR_ADAPTIVE,
and RUSTFS_METADATA_BATCH_READ once per process instead of re-reading
the environment on hot paths.
- Skip shard read-cost collection in observe mode when stage metrics
are disabled, and cache the local endpoint host list instead of
rebuilding it on every read.
- Allow --warp-extra-args values starting with -- and drop the unused
warp_hosts_csv helper in the issue-797 A/B runner.
* fix(storage): address internode data-path review findings
- Run the BatchReadVersion auto-mode unary fallback outside the batch
RPC deadline so each read_version keeps its own per-op timeout and
health accounting instead of racing the whole batch against one
drive timeout.
- Cap adaptive batch-processor concurrency growth at a hard multiple
of the configured baseline so sustained fast batches cannot ratchet
past the configured limit.
- Parse RUSTFS_INTERNODE_HTTP_* tuning, RUSTFS_BATCH_PROCESSOR_ADAPTIVE,
and RUSTFS_METADATA_BATCH_READ once per process instead of re-reading
the environment on hot paths.
- Skip shard read-cost collection in observe mode when stage metrics
are disabled, and cache the local endpoint host list instead of
rebuilding it on every read.
- Allow --warp-extra-args values starting with -- and drop the unused
warp_hosts_csv helper in the issue-797 A/B runner.
Co-Authored-By: heihutu<heihutu@gmail.com>
* fix(storage): align buffer clamp test with media cap
* fix(ecstore): release optimized read locks before streaming
---------
Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
* fix(core-storage): fix critical correctness defects from core-storage audit
Fixes verified defects found in a deep audit of the core storage path
(erasure coding, disk persistence, quorum, heal, replication resync):
- ecstore/disk: rewrite live xl.meta atomically (temp+rename) in
delete_versions_internal and write_metadata instead of in-place
truncate, which exposed torn metadata to concurrent readers and
crashes on the DeleteObjects hot path
- ecstore/erasure: allow heal to reconstruct from exactly data_shards
bitrot-verified sources; requiring data_shards+1 made objects
permanently unhealable after losing parity_shards disks
- ecstore/set_disk: direct-memory inline GET applied the erasure
distribution permutation twice (shuffled inputs re-indexed through
distribution), concatenating wrong shards into the response body in
degraded reads; collect from canonical disk-ordered inputs
- ecstore/set_disk: heal now preserves the committed inline layout
instead of recomputing it with a hardcoded unversioned threshold,
which split quorum identity of healed replicas and caused endless
re-heal churn
- ecstore/replication: resync results channel switched from
broadcast(1) to mpsc; a lagged broadcast receiver ended the stats
collector and every subsequent failure went uncounted, letting
failed resyncs be marked completed
- ecstore/replication: ignore an empty persisted resync checkpoint;
resuming with one skipped every object and marked the resync
completed without replicating anything
- ecstore/replication: fix inverted not-found error classification in
replicate_object/replicate_delete logging paths
- ecstore/erasure: guard decode paths against zero block_size or
data_shards from corrupt on-disk metadata (divide-by-zero panic)
- ecstore/disk: os::read_dir no longer consumes the entry limit on
entries it does not return (is_empty_dir misjudgment); create_file
opens with O_TRUNC to avoid stale trailing bytes
- filemeta: treat Some(nil) version id as a null version in
matches_not_strict; disk-loaded headers never store None, so the
mod_time quorum guard for unversioned overwrites never fired and an
interrupted overwrite could displace the committed version in merge
- filemeta: fix msgpack skip lengths for fixext (missed the ext type
byte) and ext16/32 (over-skipped) unknown fields
- filemeta: return FileCorrupt instead of usize underflow when
xl.meta is truncated inside the CRC trailer
- filemeta: surface delete-marker insertion failure in delete_version
instead of reporting success when the data dir is shared
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(replication): drop duplicate cfg(test) etag import from boundary module
The test module already imports content_matches_by_etag locally, so the
top-level cfg(test) import is unused under -D warnings and fails clippy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Zhengchao An