Each erasure block runs its Reed-Solomon encode through
tokio::task::block_in_place on the multi-threaded runtime. That parks the
worker and asks the scheduler to relocate other tasks, but the encode itself
is only ~110µs per 1MiB block (p99 ~542µs) — the profiling in backlog#932
flagged the scheduling disturbance as comparable to the compute it guards.
Call the encode closure inline on the multi-threaded runtime instead. The
CurrentThread (and any other) flavor keeps spawn_blocking so the sole executor
thread is never blocked and block_in_place's multi-thread-only requirement is
respected. Applied to both ingest paths (encode_block / Vec and
encode_block_bytes_mut / BytesMut); no change to encode output, quorum,
shutdown, or error handling.
Adds encode_works_on_multi_thread_runtime to cover the previously-untested
multi-threaded arm for both ingest paths, asserting streaming and batched
encode produce identical shard bytes.
This is the low-risk, correctness-neutral item that backlog#932's adversarial
verification recommended splitting out and doing first; the larger per-writer
pipeline restructure it belongs to stays gated on a Linux multi-disk baseline.
Refs: rustfs/backlog#932 (HP-11), rustfs/backlog#936
Co-authored-by: heihutu <heihutu@gmail.com>
* perf(ecstore): read bitrot hash+data in one pass on the shard path
BitrotReader::read issued two reads per block: one read_exact for the
32-byte hash, then a separate loop for the shard data. On the streaming
disk reader (a raw tokio File whose every read is a spawn_blocking
round-trip) that is two dispatches per block. Since the on-disk layout is
a contiguous [hash][data] run, pull both in a single pass into a reused
scratch buffer and split afterwards, halving the per-block dispatch count
on the streaming path. The no-hash path still reads straight into the
caller buffer with no extra copy, and an in-memory Cursor (inline/mmap)
just does a slice copy.
All existing invariants are preserved: a short read of either the hash or
the data maps to UnexpectedEof before and independent of the hash check
(backlog#799 B2), and the hash-mismatch / InvalidData semantics are
unchanged.
Correctness-only change; the per-dispatch latency win is platform
dependent and its default reliance is left to the warp size-bucket
benchmark gate tracked by backlog#935, per the backlog#933 acceptance
note.
Refs: rustfs/backlog#933 (HP-12 item 2), rustfs/backlog#936
Co-Authored-By: heihutu <heihutu@gmail.com>
* docs: reword bitrot test comment to satisfy typos check
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
Merge the hotpath-rs wall-time instrumentation from the backlog#936 analysis worktree behind an opt-in 'hotpath' cargo feature, keeping the default build at zero overhead and zero dependency.
- hotpath is an optional dependency everywhere (dep:hotpath feature syntax); the default dependency tree contains no hotpath crate at all
- 40+ measurement points across S3 handlers, ECStore/SetDisks object and multipart ops, erasure encode/decode, bitrot, LocalDisk I/O, FileMeta codec, and HashReader
- attribute sites use #[cfg_attr(feature = "hotpath", hotpath::measure)]; async_trait bodies use per-crate hp_guard! macros (ecstore + rustfs bin); rio gates measure_block! behind hp_measure_block!
- feature chain: rustfs -> rustfs-ecstore -> rustfs-rio / rustfs-filemeta, each crate owning its own gate
- hotpath-alloc is intentionally not wired up (hotpath 0.21.x TLS panic on cross-thread guard drop under tokio, see backlog#935); mimalloc stays the unconditional global allocator
- docs/development/hotpath-profiling.md documents building, HOTPATH_* env vars, SIGTERM report flow, and how to reproduce the backlog#936 timing reports
Refs: https://github.com/rustfs/backlog/issues/935 (HP-14, item 2), https://github.com/rustfs/backlog/issues/936
Co-authored-by: heihutu <heihutu@gmail.com>
* feat(ecstore): add stripe-advance handles for deferred bitrot readers
Give DeferredObjectReader a shared pending state and expose a
DeferredReaderStripeHandle that advances the still-unopened source by whole
bitrot blocks using the same bitrot_encoded_range geometry the reader was
created with (identity mapping when hash_size == 0). This lets the GET decode
path open a parity shard aligned to the stripe where a data shard failed
instead of reading every parity shard on every stripe (backlog#923).
An already-opened (or failed) reader rejects the advance so callers retire it
rather than engage it out of alignment; bitrot verification after an advance
checks the advanced stripe's block against that stripe's stored hash.
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(ecstore): read only data shards on healthy lockstep GET behind opt-in gate
PR #4289's lockstep fix made every reconstruction-verifying GET read all
data+parity shards per stripe; the parity blocks are read, bitrot-hashed and
then discarded, a deterministic 2x read-bytes/IOPS/hash-CPU amplification on
healthy 2+2 objects (backlog#923). With the new opt-in gate
RUSTFS_GET_LOCKSTEP_DATA_SHARDS_ONLY_ENABLE=true (default: false, behavior
identical to main):
- read_lockstep keeps only the data slots engaged while the object is
healthy; parity slots stay unopened deferred readers.
- When a data shard is missing or dies at stripe k, parity readers are
engaged mid-object by advancing their deferred stripe handle to stripe k,
preserving the lockstep alignment invariant from backlog#832.
- Degraded stripes engage one parity beyond the decode quorum so
reconstruction verification keeps an extra source to check against
(erasure.rs only verifies when available > data shards); an engaged parity
reader that errors is retired for the rest of the object like any other,
and a parity reader that cannot be realigned is retired instead of being
read out of position.
- fill_deferred_bitrot_readers records stripe handles for deferred slots and,
gate-on only, swaps eagerly opened parity readers for unopened deferred
ones so they remain engageable mid-object; ready/error bookkeeping used by
quorum decisions is untouched.
- Both GET paths (legacy duplex via Erasure::decode_with_stripe_handles,
codec streaming via ParallelReader::with_deferred_parity_handles) carry the
handles from reader setup.
Short-read -> UnexpectedEof -> whole-object retirement and the
inconsistent-source rejection are unchanged in both gate modes; tests lock
the healthy-path data-shards-only call counts, the default read-all-shards
behavior, mid-object parity engagement for streaming and hash_size==0
formats, and mid-stream inconsistent-parity rejection.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
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>
houseme