mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
0e7b8ea16bbdcd9dd2ee0f92a2869de3bc077452
4495 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ramakrishna Chilaka
|
028ba6a675 |
perf(ecstore): parallelize multipart shard syncing (#4734)
Bound large shard-directory syncs per disk and process while preserving small-directory and durability behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
houseme
|
633c131cef |
perf(io-metrics): cache handles for hot label-less metric emitters (#4750)
The `metrics` macros re-run the recorder's `register_*` on every emission — a `RwLock` read, a name-key hash, and an `Arc` clone — even for a metric that never varies its key. For the hot, label-less recorders on the per-IO path (`record_data_transfer`, `record_io_latency`, `record_io_latency_p95/p99`, `record_io_queue_congestion`) that lookup is pure overhead once observability is on. Add `counter_increment_cached!` / `gauge_set_cached!` / `histogram_record_cached!` that resolve the handle once via `LazyLock` in production and reuse it. Under `cfg(test)` they re-resolve on every call, because the `metrics` crate resolves against a thread-local recorder that `with_local_recorder` swaps per test — a process-global cached handle would bind to whichever recorder was active first and break test capture. The macros only wrap FIXED (label-less) keys, and the `metrics_enabled()` gate still short-circuits before any emission when disabled. Verified: the only callers of these functions are the collector (io-metrics' own cfg(test) tests, which re-resolve) and production code; no cross-crate test captures them. rustfs-io-metrics builds on both cfg paths, 147 unit + 4 doctests pass, clippy clean. Addresses rustfs/backlog#1185 (P3, per-emission handle caching). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
89557a7ffe |
perf(ecstore): cache io_uring fallback root label (#4747)
`record_uring_fallback` is called at multiple sites in the io_uring read path whenever a read falls back to `StdBackend`. It formatted `self.root.display().to_string()` on every call, heap-allocating a `String` from a `Path` that never changes after construction — pure per-read waste when io_uring is degraded. Cache the label once in `UringBackend::try_new` as a `String` field (`root_label`) and clone it per emission. The metric name and the `"root"` label value are unchanged; only the redundant `Path` formatting is removed. The clone is a single alloc of an already-short string. Refs: rustfs/backlog#1185 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
5282c71f86 |
perf(io-metrics): make internode peer-health read-mostly with an RwLock (#4746)
`cluster_peer_should_bypass` is called before every internode RPC when the offline-bypass feature is enabled (remote disk `get_client`, remote locker), and it took a single process-global `Mutex<HashMap>` even for the overwhelmingly common case of an online or unknown peer, which is read-only. That serialized all concurrent internode client acquisitions on one lock. Switch `CLUSTER_PEER_HEALTH` to a `RwLock`: - the hot check takes a shared read lock and returns immediately for unknown or online peers, so concurrent RPCs no longer serialize; - only an offline peer (rare) drops to the write lock to record a re-probe, with a re-fetch/re-check because the state can flip back online between releasing the read lock and taking the write lock; - the write paths (dial reachable/unreachable) and the read-only `cluster_peer_is_offline` move to `write()`/`read()` accordingly. Behavior is unchanged on every path (online/unknown -> not bypassed; offline -> bypass with one re-probe per interval); `Instant::now()` now runs only on the offline path. Poison recovery is preserved. All internode unit tests pass. Addresses rustfs/backlog#1185 (P2). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
53bd4bb300 |
test(e2e): pin GET codec-streaming body/header parity vs legacy duplex (backlog#1183) (#4745)
backlog#1183 tracks flipping the default GET data path from the legacy tokio::io::duplex double-copy to the zero-duplex codec-streaming fast path. That flip is gated behind RUSTFS_GET_CODEC_STREAMING_BODY_COMPAT_CONFIRMED / ..._HEADER_COMPAT_CONFIRMED, which need empirical evidence the two paths are byte- and header-identical. Add an e2e regression net that runs the same object matrix twice against the same on-disk EC shards, changing only the codec-streaming env gates: phase A (default) takes the legacy duplex path, phase B (gates opened) takes codec streaming. It asserts byte-for-byte (sha256) and header-for-header equality across inline / small / multi-block (1.5M/3M/5M+) / multipart objects, plus a ranged GET (which falls back to legacy by gate design). Path confirmation is not assumed: the legacy path logs "Created duplex pipe ..." per full GET, so the test counts that marker per phase and asserts the codec phase created zero duplex pipes, proving the fast path actually ran rather than silently falling back to the path it is compared against. To capture the child server's logs for that assertion, add an optional RustFSTestEnvironment.capture_log_path (default None = inherit stdio, backward compatible) that redirects the spawned server's stdout+stderr to a file. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
763f246f8a |
test(ecstore): add shared MockWarmBackend test utility for lifecycle and tier tests (#4716)
* test(ecstore): extract shared MockWarmBackend into a test-util feature (backlog#1148 ilm-6) The tier/lifecycle integration tests carried two byte-for-byte copies of an in-memory WarmBackend mock — one in crates/scanner/tests and one in rustfs/src/app — plus duplicated register_mock_tier and polling helpers. Both implemented the same ecstore WarmBackend trait. Consolidate them into ecstore behind a new `test-util` feature, exposed via the `rustfs_ecstore::api::tier::test_util` facade: - MockWarmBackend: in-memory WarmBackend with an operation log (for ordering assertions such as "local delete precedes remote remove") and fault injection (FaultConfig): unreachable, HTTP 5xx, credential rejection, injected latency, plus external_remove to simulate an out-of-band remote deletion. - register_mock_tier / register_mock_tier_backend: register the mock into any TierConfigMgr handle (the global manager used by scanner tests or a per-instance one used by the app tests). - xl.meta transition assertion helpers: read_transition_meta, assert_transition_meta_consistent (cross-shard consistency of the status/tier/remote-key/remote-version-id tuple plus free-version count), and free_version_count. - polling helpers: wait_for_remote_absence, wait_for_object_count, wait_for_free_version_absence. Both existing copies now consume this single definition; `rg 'struct MockWarmBackend'` collapses to one. The feature is enabled only from [dev-dependencies], so it never links into the production binary (resolver 3). Designed for downstream ilm-8 (restore lifecycle) and ilm-11 (tier fault injection matrix). Coordinates with #4706 (ilm-2), which adds op-logging to the scanner mock — that op-logging is now part of this shared surface, so #4706 should rebase onto it. Refs rustfs/backlog#1148 (ilm-6), rustfs/backlog#1155. * test(ecstore): fix shared MockWarmBackend usage after main merge - Access stored objects via MockWarmBackend::contains() instead of the now private inner objects map (fixes E0609 after the shared test-util refactor). - Drop dead ReadCloser/ReaderImpl/DiskAPI imports and the unused transition_api test re-exports the mock extraction left behind. - Reword the scanner/rustfs test-util dependency comments so they no longer embed the literal rustfs_ecstore:: path that trips the ECStore architecture-migration guard. |
||
|
houseme
|
ce7d3119b2 |
perf(io-metrics): throttle collector percentile sort off the per-IO path (#4744)
`MetricsCollector::record_io_operation` recomputed P50/P95/P99 on every disk IO by taking a read lock, collecting up to `max_latency_samples` (1000) into a `Vec<u128>` and fully sorting it — an O(n log n) alloc+sort per operation on the GET read path (gated by stage metrics). Both consumers sample only periodically: the autotuner reads `avg_io_latency_us` on its tuning tick, and the P95/P99 values are never read internally — they only feed OTEL export. Nothing needs per-IO freshness. Split the two costs: - The window mean (stored in `avg_io_latency_us`, read by the autotuner) is now maintained in O(1) via a running sum kept in step with the window's push/pop, and refreshed on every op — no sort, no warm-up regression. - The P95/P99 sort is throttled to once per `PERCENTILE_RECOMPUTE_INTERVAL` (128) operations. A bounded lag is invisible to the periodic autotuner tick and OTEL export while the per-op sort cost is amortised ~128x away. Semantics are otherwise unchanged: same sliding window, same percentile indices, same mean value. Tests updated — the percentile test forces a recompute to exercise the math directly, and a new test asserts the mean tracks every op while the percentiles only recompute at the interval boundary. Addresses rustfs/backlog#1185 (P1). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
96e5699568 |
test(ilm): self-managed lifecycle expiry e2e + backdated-mtime helper (backlog#1148 ilm-3) (#4710)
Convert the two reliant lifecycle expiry tests from #[ignore] + a hardcoded localhost:9000 server to self-managed RustFSTestEnvironment tests (random port, isolated temp dir) and wire them into the PR e2e-smoke subset. Time control is chosen per test and documented in-module: - test_lifecycle_expiry_backdated_mtime: mod_time back-dating via the internal source-replication backdoor (new put_object_with_backdated_mtime helper) so a Days=1 rule is already due, with RUSTFS_ILM_PROCESS_TIME=1 shrinking the rounding boundary. Asserts the backdated matching-prefix object expires and a non-matching-prefix object with the same backdating survives (isolates the prefix filter). Proves the backdoor does not trip the replication gate. - test_lifecycle_versioned_current_version_expiry_creates_delete_marker: RUSTFS_ILM_DEBUG_DAY_SECS=2 (ilm-5) accelerates the day length; asserts a latest delete marker is created, the original data version is retained and still readable by version id. - test_lifecycle_zero_day_expiry: Days=0 immediate expiry; matching object deleted, non-matching survives. All three use RUSTFS_SCANNER_CYCLE=1 so the scanner runs every second; tests poll for the terminal state instead of sleeping fixed wall-clock. Ignore count for reliant/lifecycle.rs goes 2 -> 0. CI wiring: added a distinct 'test(/^reliant::lifecycle::/)' clause to the existing profile.e2e-smoke default-filter in .config/nextest.toml (the single sanctioned e2e wiring mechanism per crates/e2e_test/README.md; no new e2e job). Refs rustfs/backlog#1148 (ilm-3), rustfs/backlog#1155 |
||
|
houseme
|
264b2dd480 |
perf(metrics): drop needless per-emission work on hot metric paths (#4743)
* perf(metrics): drop needless per-emission work on hot metric paths Audit of the metrics hot paths surfaced four low-risk wins where emission did work it did not need to: - `record_file_cache_reclaim_success/error` (disk/local.rs) called `.to_string()` on `kind` (already `&'static str`) and on the `"ok"`/`"err"` literals, heap- allocating up to four `String`s per page-cache reclaim window — which runs per read-stream reclaim. The `metrics` macros accept `&'static str` label values directly, so pass them as-is. - `record_read_repair_dedup` (set_disk/core/io_primitives.rs) likewise `.to_string()`-ed an already-`&'static str` `reason`. - `SetDisks::get_object_reader` (set_disk/ops/object.rs) captured `Instant::now()` and emitted the `rustfs.lock.acquire.*` counter and histogram unconditionally on every GET, right beside an already-gated stage timer. Gate them behind `get_stage_metrics_enabled()` too, so an inactive observability config pays no per-GET clock read or recorder lookups. - The per-response-body-chunk counter in server/http.rs re-ran the `counter!` registry lookup on every chunk (a streamed GET emits many). Resolve the label-less handle once into a `LazyLock<metrics::Counter>`; the global recorder is installed at startup before any response streams, so the cached handle binds to the final recorder. No metric names or label values change. The only behavior change is that the `rustfs.lock.acquire.*` GET-path metrics now follow the GET stage-metrics flag, consistent with the neighbouring stage timings. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(metrics): gate page-cache reclaim metrics behind metrics_enabled() `record_file_cache_reclaim_success/error` run per read-stream reclaim window on large-object reads and emitted unconditionally. When general metrics are disabled the `counter!`/`histogram!` macros still construct three metric keys per call for nothing. Skip the emission behind `rustfs_io_metrics::metrics_enabled()`, matching how the io-metrics free functions self-gate. The serial reclaim-metrics test now enables the flag (save/restore) alongside the existing stage gate. Left ungated deliberately: `record_read_repair_dedup` (rare read-repair path, and its non-serial test would need a global-flag toggle), and the HTTP body-chunk counter (its cached handle already makes the disabled case a no-op increment). Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
e742a540a4 |
test(cache): guard the body-cache eligibility gate against deny-list regressions (#1146) (#4742)
`full_object_plaintext_len` decides whether a body-cache hook hit may serve bytes in place of the erasure read. It is a fail-closed allow-list: it excludes every read whose `ReadPlan::build` applies some other transform (ranged/part, raw/data-movement, restore, encrypted, remote) with an early `return None`, then returns a `Some(..)` length only for the whole-plaintext cases. A newly added `ReadPlan` branch that nobody teaches this gate about falls through to `None` and safely bypasses the cache. Flip it to a deny-list and the same new branch silently serves bytes in the wrong representation — the backlog#1108 / #1109 / #1146 class of bug. The existing unit and e2e tests only cover the branches that exist today. This adds `scripts/check_body_cache_whitelist.sh`, a structural guard wired into pre-commit / pre-pr / dev-check and CI, that asserts every exclusion predicate and a `return None` still precede the first `Some(..)`. Reordering a predicate, dropping one, moving the positive return ahead of the gate, or renaming/removing the function all fail; wording, formatting, and adding a new exclusion in the same gate do not. Mutation-tested against all four regression shapes. This machine-enforces the structural invariant that backlog#1146 was kept open to guard by hand. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
fa27bef532 | test: quarantine embedded delete flake (#4741) | ||
|
houseme
|
1ab66e124c |
test(ecstore): pin the remaining io_uring fd-cache invariants (#1180) (#4739)
Close the three test-completeness items in rustfs/backlog#1180 that the earlier hardening (rustfs/rustfs#4726, #4729) left unpinned; the other two of the five (sharded cancel routing, bailout-handle error) already landed in rustfs/uring. - rename_data end-to-end invalidation: drive the real `LocalDisk::rename_data` commit (non-inline part, so `invalidate_part_paths` is non-empty) and assert the destination part descriptor is dropped — not merely `rename_file`/`delete`. Both production `rename_data` call sites share this invalidation, so a "fix one copy, miss the other" regression is now caught. The test is non-vacuous: it seeds the cache, removes the on-disk data dir out of band (the cached fd keeps the old inode alive and clears the path for the directory rename `rename_data` performs), and asserts a read still returns the OLD bytes before the commit — which fails outright if the cache is off, so it cannot pass without a live cache. - FD_CACHE_TTL backstop: an injected short TTL proves the cache self-evicts a descriptor with no explicit invalidation; a static check pins the 5s value. - zero-length read bounds parity on the cache-HIT path: a `length == 0` read past EOF must be rejected identically to the miss path and StdBackend, pinning the #1173 fix against regression. Refactors `FdCache::new` to delegate to a private `with_ttl(ttl)` helper so the TTL backstop can be exercised with a short TTL instead of a multi-second wait. Verified: `cargo test -p rustfs-ecstore` on Linux with real io_uring (seccomp=unconfined, RLIMIT_NOFILE raised, RUSTFS_URING_TESTS_MUST_RUN=1 so a degraded skip fails rather than passing vacuously). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
313d282880 | fix(ecstore): gate gauge import on Linux (#4737) | ||
|
houseme
|
793b2a06e2 | perf(ecstore): snapshot per-IO env config on local disk read/write paths (#4736) | ||
|
houseme
|
9d64d71bd7 | fix(ecstore): reduce GET reader setup shard fanout (#4735) | ||
|
houseme
|
a8eed1c44d |
perf(io-metrics): add a global switch to gate general metric emission (#4733)
The io-metrics crate already had per-stage GET/PUT switches, but the request headers/summaries and ~40 general recorders (I/O scheduler, bytes-pool, zero-copy, bandwidth, system-resource, error/timeout/retry) emitted unconditionally, paying their label allocations and arithmetic even when no metric recorder is installed. Add `METRICS_ENABLED: AtomicBool` (default false) with `set_metrics_enabled()` / `metrics_enabled()`, isomorphic to the existing stage switches, and gate: - GET headers/summaries via `get_stage_metrics_enabled()`: record_get_object_request_start / _request_started / _request_result / _timeout / _completion / _total_duration_with_path, plus legacy record_get_object. - PUT headers via `put_stage_metrics_enabled()`: record_put_object_request_start / _request_result, plus legacy record_put_object. - 41 general recorders via `metrics_enabled()`. Startup enables all three switches together under `observability_metric_enabled()` (startup_observability.rs), with a passthrough in startup_runtime_sources.rs. The global recorder is itself only installed when observability metric export is on, so gating off is pure cost savings when export is disabled and a no-op when it is enabled. Deliberately NOT gated (would break function, not just observation): the EC encode in-flight and GET buffered-bytes accounting guards (add/remove_ec_encode_inflight_bytes, track_get_object_buffered_bytes) whose values are read back by current_*; and the stateful sibling modules (lock/deadlock/backpressure/autotuner/adaptive_ttl/collector/internode). The console realtime metrics endpoint (/admin/v3/metrics) samples system state and internode metrics directly, so it does not depend on the gated recorders. Adds a toggle test exercising both the enabled and disabled paths, serialized on the existing METRICS_FLAG_LOCK to stay robust under nextest's shared-process model. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
c79366d42c |
refactor(ecstore): tidy io_uring read backend (docs + fd-cache handle) (#4732)
Two behavior-preserving cleanups to the io_uring local read backend that accumulated as later work layered onto it: - Reunite the `UringBackend` struct doc comment. The backlog#1145 fd-cache constants were inserted into the middle of the struct's doc block, so its opening sentences were orphaned onto `ENV_RUSTFS_IO_URING_FD_CACHE` and the struct itself was documented by a sentence fragment starting mid-clause with "through rustfs-uring's...". Move the opening back onto the struct and give the constant its own one-line doc. - Fold the fd-cache handle and its lookup key into a single `Option<(&FdCache, FdKey)>` in `pread_uring`, so the get and the insert_if_fresh sites stop re-deriving `self.fd_cache.as_ref()` and re-matching presence. Semantics are identical, including the backlog#1176 generation guard (`gen_at_open` snapshot before open, insert only when the generation is unchanged). No functional change. The borrow/move pattern was validated against a host-compilable reduction (the cfg(linux) path cannot be built from macOS); CI compiles the Linux backend. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
4f29dcdcec |
chore(deps): bump rustfs-uring to 0.2.1 (#4731)
chore(deps): bump rustfs-uring to 0.2.1 from crates.io rustfs-uring 0.2.1 routes the driver's runtime diagnostics through `tracing` with structured fields instead of `eprintln!` (rustfs/uring#13). No public API change — UringDriver::probe_and_start_sharded, read_at, and read_at_direct keep their signatures — and the read path / cancel-safety ownership model are untouched, so this is a drop-in patch bump of the version requirement plus the lockfile entry. 0.2.1 adds a `tracing` dependency; it is recorded in the rustfs-uring lock entry. tracing is already in the workspace graph, so nothing new is pulled in. The Cargo.lock change is scoped to the rustfs-uring package only; unrelated lockfile drift is left for its own change. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
ffca98cdbf |
fix(ecstore): harden io_uring integration (#4726)
* fix(ecstore): close the fd-cache open-then-insert race with a generation guard (rustfs/backlog#1176) pread_uring's miss path opened a descriptor on the blocking pool and only then inserted it into the moka cache. moka's invalidations cover only entries present at call time, so a heal/delete commit that invalidated between the open and the insert could not stop the just-opened stale inode from being cached afterwards — serving the pre-heal/pre-delete inode for up to the TTL and defeating the heal. Add an invalidation generation to FdCache, bumped by invalidate_exact and invalidate_under before they touch moka. The read path snapshots the generation before opening and inserts via insert_if_fresh, which refuses the insert if the generation moved during the open and, with a post-insert re-check, removes the entry if an invalidation raced the insert itself. Reads that never miss are unaffected. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): invalidate the fd cache on the primary object-delete paths (rustfs/backlog#1175) The fd-cache invalidation contract was only wired into DiskAPI::delete, rename_file and rename_data, but object deletion almost never goes through LocalDisk::delete — DeleteObject(s) reach delete_version, delete_versions -> delete_versions_internal, and delete_paths, all of which remove a version's data dir (move_to_trash / rename_all staging) with no invalidation. A cached io_uring descriptor kept the deleted part.N inode readable for up to the TTL, so a GET in that window could still return deleted data. Invalidate every cached fd under the removed data dir at each site: in delete_version and delete_versions_internal the data_dir uuid and object path are in hand (invalidate_cached_fds_under(volume, "{path}/{uuid}")); delete_paths invalidates under each removed path. A later rollback that restores a data dir just causes the next read to re-open it. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): close remaining fd-cache invalidation gaps (rustfs/backlog#1177) Three residual paths could keep serving a stale descriptor: - delete_volume removed the whole bucket tree (remove_dir_all/remove_dir) with no invalidation, and the cache-hit read path skips the volume-access check, so a cached fd kept a removed object readable. Add invalidate_cached_fds_for_volume (a per-volume moka predicate) and call it after the bucket is removed. - A retired LocalDisk instance (renew_disk on reconnect builds a fresh one) kept its populated cache alive while still referenced by in-flight ops, so invalidations through the new instance never reached it. close() now clears the backend's cache via clear_cached_fds. - rename_data's post-commit rollback (a commit-metadata fsync failure under strict durability) restored the old data dir without dropping fds cached during the committed window; the streaming branch now invalidates the dst part fds on those rollback paths. The inline branch's rollback runs inside spawn_blocking and is left to the TTL backstop. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): narrow the io_uring latch classes to match StdBackend (rustfs/backlog#1171) The runtime degradation classification reused the probe-time restriction errnos, which the driver's C7 contract explicitly warns against, so a single per-file error could latch a whole disk off io_uring: - is_io_uring_unsupported no longer includes EACCES: at read time on an already-open fd it is per-file (an LSM hooks security_file_permission on every read) and StdBackend hits the same denial, so falling back masks nothing and a full-disk latch would be wrong. ENOSYS and EPERM (seccomp/LSM applied after startup) remain. EOPNOTSUPP is now classified per-path by the caller. - pread_uring_direct's read-error arm now mirrors StdBackend: an O_DIRECT-shape error (EINVAL/EOPNOTSUPP) latches only direct_uring.supported so eligible reads take StdBackend's aligned path, instead of over-latching the whole io_uring backend or never latching a read-side EINVAL at all. - try_new only negative-caches genuine restriction-class probe failures in URING_UNSUPPORTED_DISKS; an unexpected (possibly transient) probe failure now falls back without latching, so the next reconnect re-probes. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(ecstore): log when a disk latches io_uring off at runtime (rustfs/backlog#1172) A probe-gated gray release was flying blind: the permanent per-disk `active` latch flipped with no log and no metric, so the only message operators ever saw was the startup "io_uring read backend enabled" line — which stayed true on dashboards even after the very first read latched the disk back to StdBackend forever. Add latch_active_off, which flips the latch with `swap` and logs the true->false transition exactly once at warn with a dedicated event constant, disk root, and errno. Both the buffered and O_DIRECT read paths use it. A fallback/latch metric counter and periodic export of the driver StatsSnapshot (cq_overflow, cancel_already) remain as follow-ups that need rustfs_io_metrics plumbing. Co-Authored-By: heihutu <heihutu@gmail.com> * chore(audit): correct the stale rustfs-uring license-allow rationale (rustfs/backlog#1181) The dependency-review allow said rustfs-uring is "pulled as a git dependency", but ecstore now pins it from crates.io. Update the rationale and scope the allow to the exact pinned version (pkg:cargo/rustfs-uring@0.1.0) so a future version bump forces a conscious re-review of the license/provenance claim instead of being waved through on an outdated justification. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): offload io_uring driver teardown off the tokio worker (rustfs/backlog#1170) UringBackend held Arc<UringDriver> and had no Drop, so when the last LocalDisk reference dropped in async context (disk reconnect via renew_disk, or shutdown), UringDriver's own Drop ran on that thread — sending Shutdown and joining each shard thread, which can block up to the bounded-drain timeout (5s) on a hung / D-state disk, stalling a tokio worker. Wrap the driver in ManuallyDrop (deref is transparent, so read call sites are unchanged) and add a Drop that takes the Arc and, when a runtime is present, drops it on a blocking thread so the potentially-blocking join never runs on a runtime worker. Off-runtime it drops inline. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): restore StdBackend read parity on the uring paths (rustfs/backlog#1173) Two byte-for-byte parity breaks against StdBackend on the io_uring read paths: - A zero-length read on an fd-cache hit returned Ok(empty) without any bounds check, while StdBackend and the uring miss path return FileCorrupt for an offset past EOF. Fstat the cached descriptor on the length==0 path and match. - reclaim_read_range fadvise(DONTNEED)'d the raw unaligned [offset, offset+len) range, but fadvise only drops fully-covered pages, so the head partial page stayed resident — whereas StdBackend's mmap path reclaims the page-aligned superset. Bitrot shards' 32-byte block headers keep offsets off page boundaries, so this diverged on the common case. Page-align the reclaim window to match the mmap path exactly. (The third parity item from the audit — a failed reclaim fadvise failing the read — is already parity: StdBackend's mmap path propagates the same fadvise error with `?`, so no change is needed.) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): bound worst-case in-flight memory by chunking huge uring reads (rustfs/backlog#1174) The driver's backpressure permits count operations, not bytes, and it zero-fills a full-size buffer per op, so a single unbounded read could pin ~length bytes per permit (128 permits x shards x up to ~2 GiB). ecstore passes a whole part's shard range as one pread_bytes with no upstream chunking. On the buffered path, split reads larger than URING_MAX_OP_LEN (128 MiB) into sequential chunks, awaited one at a time, so worst-case in-flight memory is bounded by permits x URING_MAX_OP_LEN per shard. The threshold is high enough that ordinary shard reads keep the single-op, zero-copy fast path unchanged. The O_DIRECT path (opt-in, alignment-constrained) is left for a follow-up. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): gate the io_uring fd cache on RLIMIT_NOFILE headroom (rustfs/backlog#1178) The fd cache holds up to FD_CACHE_CAPACITY (512) descriptors per disk, but try_new cannot know the disk count and nothing checked the process fd budget. On a bare-metal / non-systemd run with the common 1024 soft RLIMIT_NOFILE, two disks would already exhaust fds with EMFILE surfacing on reads and probes. Check the soft limit at try_new: enable the cache only with ample headroom (>= 16384), otherwise log a warning once and fall back to open-per-read. The packaged systemd unit sets 1,048,576, so tuned deployments are unaffected. Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): make io_uring test skips visible and gate non-vacuity (rustfs/backlog#1179) The ecstore io_uring tests degrade to a silent pass when io_uring is unavailable (bare `return`s or plain eprintlns), so a CI leg on a restricted runner never exercises the real UringBackend/FdCache/latch paths yet still goes green — an integration regression could merge unseen. Add uring_test_skip: it emits a grep-able `SKIP <name>` line and, when RUSTFS_URING_TESTS_MUST_RUN is set (a CI leg that guarantees io_uring, e.g. a seccomp=unconfined container), panics instead of skipping. Route the silent-skip sites through it. Wiring a dedicated CI leg that sets that env on a capable runner is tracked in the issue; this provides the enforcement mechanism. Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): cover delete_paths fd-cache invalidation (rustfs/backlog#1180) Add an end-to-end test that seeds the descriptor cache with a read, removes the part via disk.delete_paths (one of the primary object-delete entry points that does not go through LocalDisk::delete), and asserts the next read no longer returns the removed inode — pinning the invalidation added in #1175. The sharded cancel-routing half of #1180 is covered in the rustfs-uring PR. Co-Authored-By: heihutu <heihutu@gmail.com> * io_uring audit follow-ups: O_DIRECT chunking, inline invalidation, metrics, CI leg (backlog#1160) (#4729) * fix(ecstore): chunk large O_DIRECT reads too, bounding in-flight memory (rustfs/backlog#1174) The buffered read path already splits reads above URING_MAX_OP_LEN into sequential chunks; do the same for the O_DIRECT path, which was left for a follow-up. read_at_direct aligns each chunk's sub-range internally, and chunk sizes are a multiple of URING_MAX_OP_LEN so a boundary re-read is at most one block. Extract classify_direct_read_error so the single-op and chunked paths share one copy of the EINVAL/EOPNOTSUPP-vs-subsystem latch classification rather than duplicating it. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): invalidate cached fds on the inline rename_data rollback (rustfs/backlog#1177) The streaming rename_data branch invalidates cached part fds on its post-commit rollback paths, but the inline branch runs its commit and rollback inside a single spawn_blocking closure where the async invalidate cannot be called, so it was left to the TTL backstop. Capture the closure's result instead of `??`-propagating it: on error (a commit-metadata fsync failure under strict durability rolls the committed rename back), invalidate the dst part paths at the async level before returning. Inline objects keep their data in xl.meta rather than separate part inodes, so this is largely defensive, but it removes the caveat and keeps the two branches consistent. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(ecstore): export io_uring latch/fallback and driver stats metrics (rustfs/backlog#1172) Complete the gray-release observability. Beyond the warn log added earlier, emit metrics so a dashboard can answer "how much traffic is on io_uring vs falling back, and is any disk degrading": - rustfs_io_uring_latch_off_total — a disk latching io_uring off at runtime. - rustfs_io_uring_read_fallback_total — each io_uring -> StdBackend read fallback (latched-off short-circuit, O_DIRECT error, buffered error). - a low-frequency per-disk exporter of the driver StatsSnapshot as gauges (in_flight, cq_overflow, cancel_already), spawned in try_new. It holds only a Weak reference so it never keeps the driver alive, and drops any temporary strong reference on the blocking pool so a last-reference UringDriver::Drop join never runs on an async worker (rustfs/backlog#1170). submit_errors is deliberately not exported yet: it is a field added in the unreleased rustfs-uring 0.2.0, and ecstore still pins 0.1.0. It lands once the dependency is bumped (rustfs/backlog#1181). Co-Authored-By: heihutu <heihutu@gmail.com> * ci: add a real-io_uring integration leg on ubuntu-latest (rustfs/backlog#1179) The existing self-hosted sm-standard runners cannot guarantee io_uring is available (a container seccomp filter can block io_uring_setup), so the ecstore uring tests degrade to a silent skip and never exercise the real UringBackend/FdCache/latch paths in CI. Add a job on GitHub-hosted ubuntu-latest, which runs a recent kernel with no container seccomp filter, running the uring-named ecstore tests with RUSTFS_IO_URING_READ_ENABLE=true and RUSTFS_URING_TESTS_MUST_RUN=1 — the non-vacuity gate makes the leg fail rather than skip if io_uring is unavailable, so an integration regression can no longer merge green behind a vacuous pass. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
3f1acfe8a7 |
fix(docker): warn instead of rejecting default or missing credentials (#4728)
* fix(docker): warn instead of rejecting default or missing credentials The entrypoint hard-reject from #4278 broke the container-first UX and the repo's own scheduled e2e lanes (e2e-s3tests, mint boot rustfs-ci images with default credentials and died at startup). Maintainer decision: ship no baked-in credentials, warn instead of block. - missing credentials: warn and start; wording accounts for the RUSTFS_ROOT_*/MINIO_* alias sources the entrypoint does not inspect - default rustfsadmin via env/CLI/file: warn and start; the warning notes all-default pairs cannot derive an internode RPC secret - malformed config stays fatal: source conflicts, unreadable files, empty or whitespace-only values, flags missing their argument - present-but-empty env vars now hit the empty-value hard failure instead of running the binary with an empty root credential - empty/default checks trim CR and blanks like the binary; files without a trailing newline are no longer falsely rejected as empty - the no-baked-credentials guard covers all four Dockerfiles, and the test harness refuses hosts where /usr/bin/rustfs exists - e2e-s3tests/mint move to non-default credentials (rustfsadmin-ci), which also restores RPC-secret derivation for the multi-node lane GHSA-68cw fail-closed RPC derivation (#4402) is untouched; helm stays fail-closed by design. * chore(docker): reword entrypoint comment flagged by typos check |
||
|
houseme
|
ab60244d7d |
chore(deps): bump rustfs-uring to 0.2.0 (#4730)
chore(deps): bump rustfs-uring to 0.2.0 from crates.io rustfs-uring 0.2.0 is published on crates.io. It carries the read-driver hardening from the backlog#1160 adversarial audit (cancel-safety and graceful-degradation fixes on the Linux io_uring read path). The public API ecstore consumes is unchanged — UringDriver::probe_and_start_sharded, read_at, read_at_direct all keep their 0.1.0 signatures — so this is a drop-in bump of the version requirement plus the lockfile entry. The Cargo.lock change is intentionally scoped to the rustfs-uring package only; unrelated lockfile drift is left for its own change. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
67a88f3feb | test(replication): cover multipart fanout integrity (#4727) | ||
|
Zhengchao An
|
bec5227018 |
test(e2e): add negative presigned URL SigV4 coverage (#4714)
test(e2e): negative presigned-URL SigV4 suite (backlog#1151 sec-2) Add crates/e2e_test/src/presigned_negative_test.rs, the query-string SigV4 sibling of the header-SigV4 suite (sec-1, #4708). Presigned URLs were only exercised on the happy path, so nothing pinned our end-to-end enforcement of expiry or query-signature verification. Against a live single-node RustFSTestEnvironment (random port, isolated temp dir), the suite asserts HTTP status + S3 error <Code> for: - expired presigned GET -> 403 AccessDenied (Request has expired) - tampered X-Amz-Signature -> 403 SignatureDoesNotMatch - wrong secret key -> 403 SignatureDoesNotMatch - tampered target key (swapped after signing) -> 403 SignatureDoesNotMatch - tampered presigned PUT -> 403 SignatureDoesNotMatch + object not stored - positive controls: valid presigned GET (body match) and PUT (HEAD verify) Expiry is controlled without real waiting via the AWS SDK presigner's start_time (sign as of one hour ago with a 60s window). s3s checks expiry before the signature, so the expired case surfaces AccessDenied. Wired into the e2e-smoke nextest profile (fast, single-node, no external deps) so it runs on every PR per the crates/e2e_test/README.md admission criteria. Refs rustfs/backlog#1151 (sec-2), rustfs/backlog#1155. |
||
|
Zhengchao An
|
d6e3aa9140 |
test(admin-auth): add unit and e2e coverage for the admin authorization gate (#4717)
test(admin-auth): unit + e2e coverage for the admin authorization gate Adds the first tests for the central admin authorization gate `rustfs/src/admin/auth.rs`, which previously had zero coverage (backlog#1151 sec-4, master plan #1155). Unit tests (rustfs/src/admin/auth.rs): - Refactor the two `validate_admin_request*` entry points to share a single generic decision core `evaluate_admin_actions<S: Store>` / `check_admin_request_auth<S: Store>` (removes the duplicated action-loop and makes the gate testable without a running cluster). - Cover: owner/root credential allowed; authenticated non-admin credential denied with AccessDenied; missing credential denied; and the multi-action loop grants on any permitted action, denies when none pass. Backed by an in-memory empty IamSys so the owner path short-circuits to allow and every other principal resolves to deny. E2E (crates/e2e_test/src/admin_auth_test.rs, raw SigV4 over HTTP, no awscurl dependency): - non_admin_credential_denied_on_admin_api: a limited IAM user gets 403 AccessDenied on GET /rustfs/admin/v3/info while root succeeds. - root_credential_rotation_takes_effect: restart with rotated --access-key/--secret-key; new credential works and the stale one is rejected, on both the admin plane and the S3 data plane. - default_credentials_emit_startup_warning: booting with the default rustfsadmin credentials emits the default-credentials warning. Refs rustfs/backlog#1151 (sec-4), #1155. |
||
|
houseme
|
2ebe8e561b |
fix(replication): allow loopback replication targets under an explicit test opt-in (#4725)
* fix(replication): allow loopback replication targets under an explicit test opt-in
Commit
|
||
|
Zhengchao An
|
f63af3df63 |
chore: retire completed-migration scaffolding, wire orphaned boundary check (#4719)
The ecstore/global-state migrations are done (backlog#815, #939, #1052 all closed). Review of every migration-era test/gate measure found three things actually retirable or broken — everything else is a live anti-regression guard and stays. Remove: - scripts/check_metrics_migration_refs.sh — guards a migration that finished: rustfs_metrics:: has zero hits, the metrics crate no longer exists, and the script was never wired into CI or make (only reference was one line in config-model-boundary-adr.md, also removed). - crates/obs init_metrics_collectors — the "backward-compatible alias kept during migration" the removed script was guarding. Zero callers; pure delegate to init_metrics_runtime. Archive (docs/superpowers/plans/, continuing the 2026-07 convention, with the standard archived banner): - startup-timeline.md, scheduler-baseline.md, profiling-numa-capability-inventory.md, kms-development-defaults-inventory.md — one-shot snapshots whose only consumer is the already-archived migration-progress ledger (their same-dir links there start resolving again after the move); zero script pins; fed the closed backlog#660/#665 architecture-review ledger. Fixed the one outbound link (startup-timeline -> readiness-matrix) that the move would have broken — check_doc_paths.sh deliberately does not scan plans/, so nothing else would have caught it. Wire (found orphaned by the same review): - scripts/check_extension_schema_boundaries.sh guards a live contract crate but was never invoked anywhere. Add lint-fmt.mak target, include in pre-commit/pre-pr/dev-check, add ci.yml Quick Checks step (job already installs ripgrep), sync the CONTRIBUTING.md enumerated list, and harden the script against a silently-passing rg probe when src/ is missing. Keep (verified live, documented so the next cleanup pass does not repeat this analysis): - scripts/check_architecture_migration_rules.sh — added a header stating it is a permanent boundary guard, not retirable migration scaffolding; 'migration' in the name is historical. - check_migration_gate_count.sh + floor, delete-marker e2e proof, all pinned docs, compat-cleanup-register sync, remaining inventories (referenced by live docs). Verification: all 7 guard scripts pass, actionlint clean, cargo check --workspace (excl e2e) clean, cargo fmt --check clean. Adversarially reviewed by two independent skeptic passes; their 7 findings (alias left behind, broken outbound link, missing banners, wrong backlog attribution, CONTRIBUTING drift, rg exit-2 hole, missing header rationale) are all folded in. |
||
|
Zhengchao An
|
a97f3a9c52 |
fix(test): isolate health tests from minimal-response env var race (#4702)
Three health handler tests assert on payload fields (degradedReasons, details) that are absent when RUSTFS_HEALTH_MINIMAL_RESPONSE_ENABLE is true. The minimal-mode sibling test sets that env var via temp_env::with_var, which leaks across parallel test threads. Wrap the three affected tests with their own with_var guard pinning the env var to false so they are deterministic regardless of thread order. |
||
|
Zhengchao An
|
5650dcdc5d |
ci: pull replication tests out of e2e-smoke (loopback targets fail every PR) (#4724)
ci: pull replication tests out of e2e-smoke — loopback targets fail every PR repl-1 (#4712) added 20 bucket-replication admin-path tests to the e2e-smoke PR lane. They set a remote replication target at a loopback endpoint (127.0.0.1, a second local server), which RustFS's target SSRF guard rejects by default ('outbound URL host 127.0.0.1 is not allowed: loopback address'). repl-1's local verification was inconclusive under machine load, so this shipped broken and failed End-to-End Tests on every PR based on post-repl-1 main (#4707 etc). Move all replication e2e tests to the e2e-repl-nightly lane (now the full replication_extension_test set, no allowlist split) to un-block PRs. The nightly job needs loopback-target-allow server config for these to pass; tracked as a repl-1 follow-up (backlog#1147). e2e-smoke returns to the 17 functional modules from ci-4 (63 tests). |
||
|
houseme
|
9bf102f965 |
fix(cache): reserve admitted bytes in the memory gate to bound burst overshoot (#4718)
The fill gate compared each request against a snapshot refreshed at most every 5 s, with no accounting for what it had already let through. A burst arriving while the snapshot still read high therefore all passed the same check-then-act test and over-allocated far past the real headroom before the next refresh — a gap the burst stress test could expose but not close. Track admitted bytes since the last refresh in the shared snapshot cell and subtract them from available memory in `allows_fill`, reserving the request's size on each admission. Cumulative admission is now bounded to the real budget even though every fill reads the same stale snapshot; the refresh resets the counter because the fresh reading already reflects those allocations. The `min_free_memory_percent == 0` opt-out still short-circuits first, and the already-low-memory path is unchanged. New test `moka_backend_gate_reservation_bounds_burst_under_stale_snapshot`: 20 concurrent 40 KiB fills against a 500 KiB stale-high snapshot (300 KiB budget) admit only a bounded handful, not the whole storm. Passed 10/10 runs; mutation-verified — dropping the reservation admits all 20 and fails the test. Refs: backlog#1107 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
05fae6f939 |
test(ecstore): add TierConfigMgr state-machine unit coverage (#4713)
* test(ecstore): unit-test TierConfigMgr add/edit/remove/verify state machine (backlog#1148 ilm-4) Covers the tier config state machine and persistence paths that previously had only 4 codec tests and none for tier_config.rs: - add: non-uppercase name, duplicate name, unsupported type, missing backend payload, and a regression anchor documenting that AWS-reserved names (STANDARD) are not currently rejected. - edit: unknown tier, missing-credentials rejection for RustFS and MinIO. - remove: idempotent unknown-tier no-op, in-use rejection, empty-backend success, force skips the in_use probe, and probe-error surfacing. - verify: unknown tier, healthy backend, unhealthy backend. - pure query helpers (empty/is_tier_valid/tier_type/get/list_tiers). - persistence: JSON marshal/unmarshal roundtrip, external tier-config.bin roundtrip for Azure and GCS payload mapping, truncated/unknown-format/ unknown-version rejection, legacy v1 version-word acceptance, and encode failure on missing payload. Tests are hermetic: error paths return before backend construction, and a MockWarmBackend injected into driver_cache exercises remove/verify without any real remote. Refs backlog#1155. Co-authored-by: overtrue <anzhengchao@gmail.com> * fix(tier): reject reserved names STANDARD/RRS in TierConfigMgr::add (backlog#1148 ilm-4) (#4721) |
||
|
Zhengchao An
|
846aa95c32 |
test(security): GHSA-named regression tests for 3p3x and r5qv (backlog#1151 sec-6) (#4707)
test(security): add GHSA-named regression tests for 3p3x and r5qv (backlog#1151 sec-6) Anchor the two fixed advisories to discoverable, named regression tests so `rg -i "ghsa|3p3x|r5qv"` finds a guard for each, and future fixes are forced to update pinned behavior (red -> green). GHSA-3p3x-734c-h5vx (constant-time WebDAV/FTPS secret comparison, rustfs#4403): - ftps_core.rs: new `assert_ftps_ghsa_3p3x_wrong_credentials_rejected` drives the `ct_eq` reject branch in FtpsAuthenticator::authenticate; asserts wrong password and unknown user are both rejected (530) and indistinguishable. - webdav_core.rs: the auth-failure block now sends a valid access key with a wrong secret (exercising the `ct_eq` branch, not just the unknown-access-key path) plus an unknown user, asserting both 401 and indistinguishable. - Module doc comments map advisory -> tests -> fix PR on both files. GHSA-r5qv-rc46-hv8q (internode RPC fail-closed, rustfs#4402): - http_auth.rs: renamed the default-fallback rejection test to `ghsa_r5qv_resolve_shared_secret_rejects_default_fallback` (and broadened it to cover default env secret + blank secrets), and added `ghsa_r5qv_verify_rpc_signature_fails_closed_on_missing_or_invalid_auth` pinning the exact advisory scenario (missing/forged/cross-URL signature is rejected; a correctly signed request still passes). File-level doc maps the advisory. Docs: new docs/testing/security-regressions.md with the advisory -> test mapping table and where each layer runs; linked from docs/testing/README.md. sec-14 will formalize the written policy in AGENTS.md. The unit-level ghsa_r5qv_* tests run in the default CI pass. The WebDAV/FTPS e2e live in the protocols suite (fixed ports, --test-threads=1); they cannot join the e2e-smoke profile and are wired into CI by sec-5. Refs: rustfs/backlog#1151 (sec-6), rustfs/backlog#1155 |
||
|
Zhengchao An
|
4b83efaf36 |
ci(ilm): add gated s3-tests lane for lifecycle expiration cases (#4715)
* ci(ilm): move first batch of 5 s3-tests lifecycle expiration cases into a gated behavior lane (backlog#1148 ilm-10)
The 20 lifecycle cases in excluded_tests.txt were labeled "vendor-specific"
but the real blocker was the absence of a Ceph lc_debug_interval equivalent.
RUSTFS_ILM_DEBUG_DAY_SECS (ilm-5) now provides that, so Days>=1 expiration
behavior is testable in seconds.
These cases assert that objects/versions/uploads are actually removed by the
background scanner and the stale-multipart cleanup loop. They cannot join the
default single-server s3-implemented-tests gate: it disables the scanner, and a
global RUSTFS_ILM_DEBUG_DAY_SECS also shrinks the x-amz-expiration header that
the already-passing test_lifecycle_expiration_header_* cases assert on. So this
adds an isolated lane instead of putting them in implemented_tests.txt.
First batch (5), each verified against the pinned upstream source and the
RustFS evaluator/scanner/stale-multipart execution paths:
- test_lifecycle_expiration (prefix Days=1/Days=5, scanner delete)
- test_lifecyclev2_expiration (same via ListObjectsV2)
- test_lifecycle_noncur_expiration (NoncurrentVersionExpiration)
- test_lifecycle_deletemarker_expiration (ExpiredObjectDeleteMarker cascade)
- test_lifecycle_multipart_expiration (AbortIncompleteMultipartUpload)
Dropped from the batch, kept excluded with reason:
- test_lifecycle_expiration_days0: RustFS accepts Expiration{Days:0} (returns
200; only Days<0 is rejected in crates/lifecycle/src/core.rs validate()),
while AWS/this test expect InvalidArgument. Real validation gap.
Changes:
- scripts/s3-tests/lifecycle_behavior_tests.txt: new exact-node-id run set.
- scripts/s3-tests/run.sh: honor an IMPLEMENTED_TESTS_FILE override so a lane
can point at an alternate whitelist.
- .github/workflows/ci.yml: new s3-lifecycle-behavior-tests PR-gate job that
starts rustfs with RUSTFS_ILM_DEBUG_DAY_SECS=10, scanner enabled (cycle 2s,
no start delay) and a 2s stale-multipart cleanup interval, running the new
list serially.
- scripts/s3-tests/report_compat.py: recognize the behavior list so the weekly
scope=all sweep (plain server) does not misclassify expected failures.
- scripts/s3-tests/excluded_tests.txt: remove the 5 enabled cases; re-annotate
the remaining 15 lifecycle exclusions with real reasons + batch-2 plan.
- docs/architecture/s3-compatibility-matrix.md: sync counts (implemented 451,
excluded 274, behavior 5) and document the lane.
Refs backlog#1148 (ilm-10), master plan backlog#1155.
* fix(lifecycle): reject zero-day expiration/noncurrent/abort rules (backlog#1148 ilm-10) (#4722)
|
||
|
houseme
|
4de73c0653 | fix(docker): use non-default credentials in cluster compose/scripts (#4720) | ||
|
Zhengchao An
|
abe6c41227 |
test(e2e): negative header-SigV4 rejection suite (backlog#1151 sec-1) (#4708)
test(e2e): add negative header-SigV4 rejection suite (backlog#1151 sec-1) RustFS delegates SigV4 verification to the s3s dependency, so nothing in this repo pins OUR end-to-end wiring of it. Add negative_sigv4_test.rs sending REJECTED header-SigV4 requests against a live RustFSTestEnvironment and asserting the HTTP status plus the S3 error code XML: - valid_header_sigv4_request_succeeds (positive control so the negatives cannot pass for the wrong reason) - tampered_signature_returns_signature_does_not_match -> 403 SignatureDoesNotMatch - wrong_secret_key_returns_signature_does_not_match -> 403 SignatureDoesNotMatch - tampered_payload_is_rejected (body != signed x-amz-content-sha256) -> not 200 - skewed_date_returns_request_time_too_skewed (>15min) -> 403 RequestTimeTooSkewed - malformed_authorization_header_returns_clean_4xx (present-not-missing) -> 4xx, never 5xx Signatures are hand-built via rustfs_signer::request_signature_v4 primitives (get_signing_key/get_signature/get_scope) so the test controls the timestamp, secret, signed payload hash, and final signature bytes. Missing-credential negatives are intentionally not duplicated (covered by multipart_auth_test / anonymous_access_test). Refs backlog#1151 (sec-1), master plan backlog#1155. |
||
|
Zhengchao An
|
ac646cfbe4 |
test(e2e): large-object degraded-read EOF truncation regression net (dist-13) (#4709)
Adds an S3-API-level e2e regression net proving that a large-object GET on a
degraded EC set never returns a silently truncated body under a full
Content-Length -- the historical "unexpected EOF" bug fixed on main by
rustfs#4594 (short-body GetObject stream -> UnexpectedEof), rustfs#4560
(in-place per-part legacy degradation for the lazy multipart reader), and
rustfs#4585 (DARE package-boundary truncation detection). Those fixes each
ship a *unit* regression; this covers the layer they do not -- the full HTTP
GET path streaming a real body reconstructed from real on-disk EC shards.
New file crates/e2e_test/src/degraded_read_eof_regression_test.rs, single-node
4-disk (EC 2+2) DiskFaultHarness, three scenarios:
(a) one disk offline: a 6 MiB single object (>=2 EC stripes) and a 3x5 MiB
multipart object GET back byte-identical with the correct Content-Length.
(b) mid-stream bitrot within quorum: 2 of 4 shards corrupted mid-file on a
large multipart object still reconstructs the full, hash-matching body.
(c) beyond read quorum (the heart of the net): 3 of 4 shards corrupted
mid-file -- the read MUST fail cleanly (non-2xx or a mid-stream body
error), NEVER close with a truncated body under the full Content-Length.
The shared get_checked() helper panics on the forbidden outcome (a clean 2xx
whose collected body is shorter than the advertised Content-Length), so the
truncation bug can never be silently tolerated.
CI placement: these spawn a 4-disk server per test and are resource-heavy, so
they stay OUT of the fast PR e2e-smoke filter. A new e2e-reliability nextest
test-group (max-threads=1) serializes them (and the existing
reliability_disk_fault_test) across nextest's process boundary; ci-7's nightly
full-e2e run picks them up. Visible via `cargo nextest list -p e2e_test`.
Refs rustfs/backlog#1150 (dist-13), rustfs/backlog#1155, rustfs#4594,
rustfs#4560, rustfs#4585, rustfs#2955.
|
||
|
houseme
|
d5d6f1160d |
test(object-data-cache): add concurrency stress tests and GET benchmarks (#4711)
* test(object-data-cache): add concurrency stress tests for the fill machinery The race coverage so far pins specific interleavings with an injected barrier. These add sustained, real-parallelism contention to catch what pinned tests cannot — a leaked singleflight leader, a stranded semaphore permit, an index/cache divergence that only surfaces under volume: - moka_backend_concurrency_storm_leaves_no_leaked_state: 48 tasks x 400 mixed fill/lookup/invalidate ops on a 6-key pool over 4 worker threads, then asserts inflight_fills == 0 (no leaked in-flight fill), a clean post-storm fill/hit/invalidate sequence still works (state not corrupted), and clear() drains the cache. - moka_backend_gate_bounds_admission_under_concurrent_burst: a low-memory snapshot must reject an entire 32-fill burst — admission is bounded, not a check-then-act race that lets the burst through. (This does not cover the separate 5s-stale-snapshot overshoot, which needs byte-based reservation.) Both are mutation-verified: neutering invalidate_object fails the storm's "invalidation must win" assertion, and making allows_fill always-true fails the burst's full-rejection assertion. The storm passed 20/20 runs. `rt-multi-thread` is added to dev-deps so the tasks run on real worker threads. Refs: backlog#1107 Co-Authored-By: heihutu <heihutu@gmail.com> * bench(object-data-cache): measure the GET-path cost the audit argued statically Adds a criterion benchmark driving the public ObjectDataCache facade, so the "hot path is clean / high throughput" claim rests on numbers, not analysis: - plan_get ~104-112 ns (per-GET planning + metric label-set hash) - lookup_hit ~143-147 ns (a resident-body hit: moka get + Bytes clone + metric) - fill_distinct ~2.2-2.4 us (cold fill: plan + singleflight + index + moka + inner spawn) A hit at ~143 ns against a multi-millisecond erasure read is the ~1000x saving the cache exists for; the per-GET metric cost is real but ~100 ns, not a bottleneck; the fill cost (incl. the cancellation-safety spawn kept on purpose) is negligible on a background task. Run with `cargo bench -p rustfs-object-data-cache`. Refs: backlog#1107 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
ce53feee87 |
test(lifecycle): regression test for expire/GET race (#3491) (#4706)
Adds a serial-lane ILM integration regression test that pins the local-first ordering contract established by rustfs#3491, where `expire_transitioned_object` deletes local metadata BEFORE any remote tier cleanup so a concurrent GET can never observe live local metadata pointing at an already-removed remote tier version (user-visible `NoSuchVersion`). `serial_tests::test_expire_transitioned_object_never_races_concurrent_get` in `crates/scanner/tests/lifecycle_integration_test.rs`: - Transitions an object to a mock warm tier, then runs a tight concurrent GET loop while calling `expire_transitioned_object`; every GET must return a full, correct body or a clean object/version-not-found -- never a tier-fetch failure. - Deterministic ordering assertion (revert-proof): immediately after expiry the remote tier object is still present and the mock recorded zero remote `remove` calls, proving remote cleanup is deferred to free-version recovery. Reverting to remote-first ordering turns this assertion red. Supporting changes: - Re-export `expire_transitioned_object` from `api::bucket::lifecycle`. - Record remote-tier mutating ops in the scanner test `MockWarmBackend`. - Update tier-ilm-debugging.md to reference the new regression test. Runs in the CI ILM Integration (serial) lane (ci.yml test-ilm-integration-serial), picked up by the existing binary(lifecycle_integration_test) filter. Refs: rustfs/backlog#1148 (ilm-2), rustfs/backlog#1155 |
||
|
Zhengchao An
|
5c7c757a30 |
ci(repl): wire replication e2e suite into nextest profiles (backlog#1147 repl-1) (#4712)
Activate the 36 dormant replication e2e tests in crates/e2e_test/src/replication_extension_test.rs (zero ran anywhere before). Split via the ci-4 nextest profile mechanism, no hand-rolled cargo-test lane: - PR smoke (profile.e2e-smoke, existing e2e-tests job): the 20 fast bucket-replication tests (target-registration / replication-check / list / remove / delete admin paths) that validate config synchronously and never wait for async convergence. Each spawns its own single-node rustfs server(s) on random ports with isolated temp dirs, so parallel-safe by construction (serial_test's #[serial] is a no-op under nextest's process-per-test model; no test-group needed). - Nightly (profile.e2e-repl-nightly + .github/workflows/e2e-replication-nightly.yml): the remaining 16 = 6 slow data-plane tests + 9 _real_dual_node + 1 _real_single_node. Defined as 'replication module MINUS the PR allowlist' so new replication tests default to nightly and are never silently unrun. The nightly workflow builds the binary once, installs awscurl so the STS dual-node test runs (skips gracefully with a visible log line otherwise), and routes scheduled failures through .github/actions/schedule-failure-issue (ci-8). Explicit division of labor with ci-5 e2e-full: these run only here. Counts (cargo nextest list): e2e-smoke 83 (63 + 20), e2e-repl-nightly 16. Docs updated: e2e-suite-inventory.md, e2e_test/README.md. Refs backlog#1147 repl-1, backlog#1155. |
||
|
Zhengchao An
|
c41a813a31 |
docs(e2e): contributor guide for the e2e_test crate (backlog#1153 infra-10) (#4705)
docs(e2e): write contributor guide for e2e_test crate (backlog#1153 infra-10) Turn the e2e_test README skeleton into a dense, link-heavy contributor guide: crate overview + grouped module map, how to run (whole crate / one module / e2e-smoke profile / ILM serial lane / protocols suite), #[ignore] semantics as a pointer to live sources, how to add a test (RustFSTestEnvironment + RustFSTestClusterEnvironment, common.rs/chaos.rs helper inventory, isolation rules, #[serial]-vs-nextest reality), a CI subset map table, and a troubleshooting section (stale binary / RUSTFS_TEST_PORT / orphan processes). Keeps the ci-4 "CI smoke subset" section intact and restructures around it. Adds a reciprocal link from crates/e2e_test/AGENTS.md. Refs rustfs/backlog#1153 (infra-10), #1155. |
||
|
Henry Guo
|
3f25426534 |
fix(ecstore): reject incomplete listing usage refreshes (#4698)
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
houseme
|
7437f99c45 |
fix(cache): key the object body cache on data_dir for write-uniqueness (#4703)
* refactor(object-data-cache): derive Default for ObjectDataCacheGetRequest The GET request literal is hand-listed field-by-field across ~13 test sites in two crates. Adding `mod_time_unix_nanos` in backlog#1111 had to touch every one and still missed a literal, producing a compile error caught only in a later CI lane. Derive `Default` and spread the engine-crate literals so the next field addition is absorbed rather than fanned out. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): key the object body cache on data_dir for write-uniqueness The cache key's correctness rested on being write-unique, but its only write-scoped component was `mod_time` — a wall-clock timestamp that is not monotonic and can be absent. Two writes that collide on MD5 (same etag+size) and land on an equal mod_time (clock skew, same-tick, or absent) derived the same key, so a node that never saw the overwrite could serve the previous body for up to the TTL, and the same collision turned the fill-after-invalidation race into a serving bug. This is the store's strong-read-after-write guarantee leaning on a probabilistic argument. Add `data_dir` — the xl.meta directory UUID ecstore regenerates on every body write — as the primary write-unique anchor: - surface `data_dir: Option<Uuid>` on ObjectInfo, copied from FileInfo in the single GET-path constructor `from_file_info`; - carry it into ObjectDataCacheKey as `data_dir_u128` (held as u128 to keep the engine crate free of a uuid dependency), derived in the one planner site both the ecstore hook and the usecase layer share, so both produce an identical key by construction; - keep `mod_time` as a second anchor and `etag+size` as belt-and-braces; an absent data_dir falls back to the prior behavior — strict improvement, no regression. Two writes distinct only by data_dir now derive different keys even under an MD5 collision with identical mod_time — the case mod_time alone cannot cover. Blast radius is compiler-guarded: ObjectInfo derives Default and every real construction site uses `..Default::default()`, so only from_file_info and one full-literal test needed the field. The three P0 body_cache_hook_e2e regressions, engine (80), and app (36) suites pass unchanged; a mutation that severs the planner wiring fails planner_key_changes_with_data_dir. Refs: backlog#1111, backlog#1118 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): thread data_dir field through the merged mutation-hook test Merging main (which landed the object-mutation-hook work, backlog#1131) brought in a GetRequest test literal that predates the data_dir field. Spread it via `..Default::default()` — the derive(Default) added here means this is the last such hand-listed literal to need touching. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
a35ebb92b8 |
fix(test): add missing mod_time_unix_nanos field in mutation_hook test helper
The recent addition of ObjectDataCacheGetRequest.mod_time_unix_nanos (backlog#1111 / ODC-06) missed the test helper in mutation_hook.rs, causing a compile error under clippy --all-targets. |
||
|
Zhengchao An
|
a32af463ed |
ci: quarantine walk_dir stall-budget flake under the ci profile (#4691)
ci: quarantine walk_dir stall-budget flake under the ci profile (rustfs#4690) First application of the flake policy from docs/testing/README.md: the test failed on a zero-Rust-diff PR (#4674, run 29099382470) — timing windows in the stall-budget accounting stretch under CI load. retries=2 under profile.ci only; local default profile still never retries. Tracked by rustfs#4690 (30-day fix-or-delete deadline). |
||
|
houseme
|
85fd824581 |
feat(object-data-cache): close write-side invalidation gaps and add an admin surface (#4694)
* feat(object-data-cache): close write/delete-side invalidation gaps The object data cache exposed only a single per-(bucket,object) invalidation primitive and no write-side ecstore hook, so several delete paths left dead bodies resident until TTL (hygiene/capacity, not stale-serving: lookups follow a fresh metadata quorum and cannot serve a gone object). This adds the missing primitives and wires them in. ODC-26 (backlog#1131): add an `ObjectMutationHook` trait beside the GET body hook, registered next to it at startup, and call it from the ecstore-internal delete paths (`apply_expiry_on_non_transitioned_objects`, `expire_transitioned_object` including the restored-copy branch, and `delete_object_versions`). The app impl is one `invalidate_object` call under a new `AfterLifecycleExpiry` reason. ODC-27 (backlog#1132): force prefix delete now invalidates the whole prefix, not just the prefix string. `store.delete_object(delete_prefix)` returns no deleted-name list, so this uses a new prefix primitive rather than the batch path. ODC-28 (backlog#1133): DeleteBucket now flushes the bucket via a new bucket-scope primitive (covers force and non-force, which share the delete_bucket call). ODC-C2 (backlog#1143): add `ObjectDataCache::clear()` and two admin handlers (GET stats, POST flush) routed through admin runtime_sources. The starshard identity index gains a single `remove_matching` full-scan API backing prefix/bucket/clear; it is documented as admin/delete-path only and never runs on the GET or fill hot path. New invalidation reasons and metric labels added; outcome (removed/noop) labelling kept correct for every new primitive. Also fixes a pre-existing broken intra-doc link in memory.rs. Co-Authored-By: heihutu <heihutu@gmail.com> * refactor(ecstore): extract the shared HookSlot behind both cache hooks This PR introduced object_mutation_hook.rs by mirroring body_cache_hook.rs, which left two process-global registration slots whose register/get/clear bodies were line-for-line identical except the trait type and the WARN string: a RwLock<Option<Arc<dyn _>>>, an Arc::ptr_eq "different instance" warning, the poison-recovery closure, and the same read-lock-and-clone read. Two copies of the same swap-vs-warn logic can drift apart under maintenance. Hoist it into a generic HookSlot<T: ?Sized> that owns the logic once. Each hook module keeps its `static HOOK: HookSlot<dyn XxxHook>` and its thin, unchanged public wrappers (register_/get_/clear_), so the crate's public surface and every call site are untouched — this is an internal consolidation, not a contract change. The load-bearing #1126 guarantee (newest registration wins, so a rebuilt AppContext is never stranded on a first-wins slot) previously had no direct test — the hook tests only covered register-then-notify. HookSlot now has its own unit tests including re_registration_swaps_to_the_latest_instance; mutation-testing confirms a first-wins regression fails exactly that test. No behavior change: the two hooks' existing tests, the P0 body_cache_hook_e2e regressions, and the app-layer mutation-hook tests all pass unchanged. Refs: backlog#1126, backlog#1131 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(admin): register the object-data-cache routes in the policy inventory This PR added GET /object-data-cache/stats and POST /object-data-cache/flush but did not list them in the two registries that must account for every admin route: the route-policy inventory (route_policy.rs) and the route matrix (route_registration_test.rs). Their coverage tests — route_policy_inventory_covers_registered_routes and test_admin_route_matrix_matches_registered_routes — failed on CI because a registered route had no policy/matrix entry. These two tests are not part of `make pre-commit` (which runs fmt + arch + quick-check, not the full suite), so the gap passed local pre-commit and only surfaced in the CI Test-and-Lint lane. stats is a read (ServerInfoAdminAction, Sensitive); flush mutates (ConfigUpdateAdminAction, High) — matching the actions the handlers already enforce. The MinIO-alias matrix test is unaffected: these are native rustfs endpoints with no MinIO equivalent. Refs: backlog#1143 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
f9874b591a |
[DO NOT MERGE until published] chore(ecstore): switch rustfs-uring to crates.io 0.1.0 (#4701)
* chore(ecstore): switch rustfs-uring from the git rev to crates.io 0.1.0
Prepared ahead of the rustfs-uring 0.1.0 crates.io release (rustfs/uring):
flip ecstore's dependency from the pinned git revision to the published
`0.1.0`, and drop the now-unneeded git-source allowance in deny.toml.
DO NOT MERGE until rustfs-uring 0.1.0 is on crates.io. Until then cargo
cannot resolve `rustfs-uring = "0.1.0"`, so this branch will not build and
CI will be red — that is expected, not a defect in the change.
After the crate is published:
1. `cargo update -p rustfs-uring --precise 0.1.0` to regenerate Cargo.lock
(deliberately not touched here — the registry entry, with its checksum,
cannot be written before the crate exists);
2. push the Cargo.lock;
3. CI goes green; merge.
No code change. The guard `scripts/check_no_tokio_io_uring.sh` is unaffected
(it bans only tokio's io-uring runtime feature, not an explicit rustfs-uring
dependency). `audit.yml`'s `allow-dependencies-licenses` for rustfs-uring is
left in place — it is a first-party Apache-2.0 crate either way.
Co-Authored-By: heihutu <heihutu@gmail.com>
* chore: refresh rustfs-uring lockfile
---------
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
houseme
|
48bdf87e53 |
refactor(obs): drop the dial9 S3 config left stranded by the upload revert (#4700)
#4663 added a full S3 upload path (config fields + env parsing + wiring), then reverted the wiring when its dependency turned out to carry RUSTSEC advisories (D9-14). The revert stopped at enabled.rs and left the config layer half torn down: `Dial9Config` still carried `s3_bucket`/`s3_prefix` with no consumer, plus a dead `s3_upload_requested()`, and `Dial9SessionGuard::config()` was likewise never called. All three are zero-consumer dead code. Since S3 upload cannot be built at all, the config type should not model it. Drop the two fields and both accessors. `from_env` still reads the S3 env vars and warns about them (an operator who set them deserves to know why they do nothing — that warning is deliberate and stays), but drops the values instead of storing configuration nothing reads. While here, collapse `from_env`'s two `record_config` calls into one by extracting `from_env_enabled`, so the enabled/disabled paths share a single publish point. No behavior change: the S3 warning fires identically, and every other field is untouched. Net -13 lines, and `Dial9Config` drops from 8 fields to 6. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
95819cb986 |
docs(object-data-cache): state the correctness boundary and timing channel (#4695)
docs(object-data-cache): state the correctness boundary and the timing channel The crate doc still described itself as "a minimal skeleton for the initial rollout phase", which stopped being true several releases ago, and neither the crate nor the operator-facing env constant said anything about what the cache does or does not guarantee. Record two things a reader has to know. The correctness boundary: a hit is sound because the key matched metadata the caller just resolved from a read quorum, not because invalidation ran. Process-local invalidation is hygiene that frees capacity; the key is what keeps a stale body from being served. Anyone tempted to weaken the key should meet this sentence first. The timing side channel: a hit skips the erasure read, bitrot verify and decode, so it is reliably faster than a miss. A principal authorized to read an object can time a single GET and learn whether someone read that object within the entry's lifetime. This crosses no authorization boundary — the probe needs read access to that exact object, checked before the cache is consulted — but it does disclose a co-tenant's recent access pattern. Say so where operators look: on RUSTFS_OBJECT_DATA_CACHE_ENABLE. Timing noise would cost precisely the latency the cache exists to save, so the mitigation is to leave the cache off for buckets where access-pattern confidentiality matters. Refs: backlog#1139 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
6780140318 |
fix(object-data-cache): make the GET key write-unique and dedup the lookup (#4693)
fix(object-data-cache): make GET body cache key write-unique and dedup lookups Address four object-data-cache GET-path findings (backlog#1107 batch): ODC-06 (backlog#1111): the cache key was content-unique, not write-unique. Extend ObjectDataCacheKey with the resolved version's modification time (i128 unix nanoseconds, None -> 0), derived once in the shared planner so the ecstore hook and the usecase layer produce an identical key. An unversioned overwrite advances mod_time, so a stale node can no longer serve old bytes for up to the TTL under an MD5 collision; etag + size stay as belt-and-braces. ODC-16 (backlog#1121): every cacheable GET planned and looked up twice (once in the ecstore hook, once in the usecase layer), double-counting hits, hit_bytes and lookups. GetObjectReader now carries a GetObjectBodySource marker (Unprobed / HookMissed / HookServed); the hook stamps it, and build_get_object_body_with_cache serves a hook-served body directly and skips its lookup whenever the hook already probed. One hook-served GET now records exactly one lookup. ODC-19 (backlog#1124): ENABLE=true with no explicit mode defaulted to HitOnly, which never fills and keeps a permanent 0% hit rate. Default to FillBufferedOnly, log the resolved mode at startup, and warn when HitOnly is selected explicitly. ODC-24 (backlog#1129): max_entry_bytes above the in-memory GET fill limits was silently inert. Clamp the planner's size eligibility to min(max_entry_bytes, seek-support threshold, 64 MiB buffer cap) so ineligible sizes plan SkipTooLarge instead of being reported eligible, and warn at startup when the excess is inert. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
d0ca14d8df |
test(ecstore): cover post-commit hard-crash in rename_data crash harness (#4689)
The rename_data crash-consistency harness (backlog#935/#878) only armed pre-commit crash points (AfterDataRename, AfterBackupBeforeMetaCommit), which assert the *old* version survives. The other half of the "partial commit -> old or new, never mixed" invariant — a hard power loss *after* the xl.meta commit rename must leave the *new* version — had no crash-point coverage. The existing after-metadata-commit failpoint only exercises the graceful in-process rollback (restores old), a different path from a hard crash (no rollback runs, commit stays on disk). Add a RenameDataCrashPoint::AfterMetaCommit injection right after the commit rename (cfg(test), compiles to a const-false no-op in production, like the existing points) and overwrite/fresh x strict/relaxed tests asserting the object reads back as the new version. The fresh case pins that the point is genuinely post-commit: a pre-commit misplacement would leave no readable object and fail the assertion. cargo test -p rustfs-ecstore crash_consistency: 9 passed. clippy -D warnings clean; fmt and arch guards clean. |
||
|
houseme
|
d715cb5c34 |
refactor(ecstore): single-source the bitrot read/verify path (backlog#1159) (#4697)
P-A (`read_appending`) and P-C (the in-memory fast path) each copied the hashed-read logic, so `BitrotReader` ended up with the hash verification, the short-read error, and the scratch-buffer fill written three times across `read` and `read_appending`'s two branches. That is patch-on-patch: a change to the bitrot contract would have to be made in three places and kept in sync by hand. Collapse the duplication onto three single-source pieces: - `split_and_verify` — a free function that splits `[hash][data]`, verifies (unless skip_verify), and returns the data slice plus the hash time. Free rather than a method so it can run while `self` is borrowed for the block. - `read_scratch_block` — the single-pass fill of `self.buf` with the short-read-to-UnexpectedEof contract. - `short_shard_read` / `begin_read` — the shared error and preamble. `read` and `read_appending` now differ only in what they must: how the block is acquired (caller's slice vs `try_take_block` vs scratch fill) and where the verified shard lands (`copy_from_slice` vs `extend_from_slice`). This also fixes a latent inconsistency the duplication hid: the old `read` did `copy_from_slice` *before* verifying, so on a hash mismatch it left the corrupt bytes in the caller's buffer before returning the error, while `read_appending` verified first. Both now verify before writing, so a shard that fails the hash never reaches the caller's buffer on either method — the stronger of the two behaviors. Net -19 lines; behavior otherwise unchanged. Verified: `erasure::` 215 passed, 0 failed (including the fast-path equivalence and corrupt-shard tests, and the existing `test_bitrot_read_hash_mismatch`); clippy --all-targets -D warnings clean. Co-authored-by: heihutu <heihutu@gmail.com> |