* fix(ecstore): fsync new object's ancestor dirs to close a power-loss gap
reliable_mkdir_all creates an object's directory (and any missing prefix dirs)
with plain mkdir and never fsyncs the parent chain. The commit-point fsync in
rename_data persists the object dir's *contents* (its xl.meta and data dir),
but not the object dir's own entry in the bucket/prefix directory. So on the
first PUT of an object, a power loss after the write is acknowledged could drop
the whole object directory even though its contents were durable — an
acknowledged write silently lost (rustfs/backlog#922 step 4).
For a new object (no prior xl.meta) under a durability tier that syncs commit
metadata, fsync the ancestor chain from the object dir's parent up to and
including the bucket after the commit rename, so the newly created directory
entries survive power loss. A starts_with guard bounds the walk to the bucket
subtree. Overwrites already have a durable object dir and are unaffected;
relaxed/none accept the wider window like the existing commit fsync.
Durability regressions are invisible to ordinary behavior tests, so the new
tests assert directly (via the fsync_dir recorder) that a first PUT under a new
prefix fsyncs both the prefix and bucket dirs, and that relaxed does not.
Scope: the non-inline (erasure) commit path. The inline branch has the same
gap and is a separate follow-up.
Refs: rustfs/backlog#922 (HP-1 step 4), rustfs/backlog#936
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(ecstore): fsync new inline object's ancestor dirs too
Extend the backlog#922 step 4 mkdir-gap fix to the inline commit branch. Like
the non-inline path, a first PUT of an inline object creates its directory
(and any missing prefix dirs) whose entry in the parent chain reliable_mkdir_all
never fsynced; the commit fsync persists the object dir's contents, not its own
entry. For a new inline object under a commit-metadata-syncing tier, fsync the
ancestor chain up to and including the bucket after the commit rename, using the
same starts_with-bounded walk (via the synchronous os::fsync_dir_std inside the
inline spawn_blocking closure).
Adds a test asserting a new inline object under a new prefix fsyncs both the
prefix and bucket dirs. rename_data now closes the ack'd-write power-loss gap on
both the erasure and inline paths.
Refs: rustfs/backlog#922 (HP-1 step 4), rustfs/backlog#936
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
refactor(ecstore,heal): return the local disk map as an owned read guard (Phase 5 prep)
Prerequisite for the Phase 5 disk-registry migration (backlog#939): the disk
map cannot move from the process global into the per-instance InstanceContext
while callers depend on a `'static` read guard borrowed from the global.
Change `local_disk_map_read` to return an owned guard
(`OwnedRwLockReadGuard`) via `Arc::read_owned` instead of
`RwLockReadGuard<'static, _>`:
- ecstore `runtime::sources::local_disk_map_read` now returns
`OwnedRwLockReadGuard<..>` (holds an Arc clone of the lock), not a `'static`
borrow of `GLOBAL_LOCAL_DISK_MAP`.
- heal's forwarding accessor and its callers (which hold the guard across
`.await` while clearing/writing per-disk markers) keep identical behavior —
the owned guard derefs to the same map, so iteration is unchanged.
This decouples the heal crate from the global's `'static` lifetime so a later
PR can source the map from the current instance's context. Single-instance
behavior is byte-for-byte unchanged; the same read lock is held across the same
awaits.
Verification: cargo test -p rustfs-heal (201 tests green), cargo clippy -p
rustfs-ecstore -p rustfs-heal --all-targets (clean), make pre-commit (pass).
Refs: backlog#939 (Phase 5, disk-registry prerequisite)
shard_read_costs_for_empty_disk_set_are_empty was a plain sync #[test], but
shard_read_costs_for_disks consults process-global topology state
(local_endpoint_hosts_for_shard_costs) whose fast-lock manager lazily spawns a
background cleanup task on first access. When this test was the first in a
process to touch that global — as under nextest's per-test isolation — the
tokio::spawn panicked with a TryCurrentError because no runtime was present,
making the test order-dependent flaky in CI (it passes only when some sibling
tokio test initializes the manager first).
Run it under a Tokio runtime like the sibling reservation tests
(#[tokio::test]), so the lazy init's spawn always has a runtime. Test-only
change; no production behavior change.
Verified failing before the change and passing after under both
`cargo test` and `cargo nextest run` in isolation.
Co-authored-by: heihutu <heihutu@gmail.com>
Phase 5 Slice 11 (backlog#939): move the background replication pool and stats —
the last service handles, and the only async ones — out of the process statics
into the per-instance InstanceContext.
- InstanceContext gains `replication_stats: OnceCell<Arc<ReplicationStats>>` and
`replication_pool: OnceCell<Arc<DynReplicationPool>>` (tokio async OnceCell),
with sync read accessors (`replication_stats`/`replication_pool`/
`replication_initialized`) and pub(crate) cell accessors for the async init.
- `init_background_replication` initializes the current instance's cells via the
same `get_or_init(async {…}).await` (workers still spawned once on first
init). The lifecycle owner helpers and the runtime-source accessors keep their
signatures and route through the current instance's context; the two statics
(and the now-unused lazy_static import) are removed. The replication-boundary
arch guard still passes.
Single-instance: init materializes one shared pool/stats via the bootstrap
context — byte-for-byte the same as the eager statics.
Tests: replication state is None until set and independent across instances.
Verification: cargo test -p rustfs-ecstore (22 instance-context tests green),
cargo clippy -p rustfs-ecstore --all-targets (clean), make pre-commit (pass).
Refs: backlog#939 (Phase 5, Slice 11). Stacked on Slice 10 (#4494).
refactor(ecstore): migrate the bucket bandwidth monitor into InstanceContext (Phase 5 Slice 10)
Phase 5 Slice 10 (backlog#939): move the bucket bandwidth monitor out of the
process static into the per-instance InstanceContext.
- InstanceContext gains `bucket_monitor: OnceLock<Arc<Monitor>>` with
`init_bucket_monitor(num_nodes)` (set-once; returns false if already set) and
`bucket_monitor() -> Option<..>`.
- global.rs `init_global_bucket_monitor` / `get_global_bucket_monitor` keep
their signatures and route through the current instance's context; the static
is removed. The ignore-and-warn-on-reinit behavior is preserved (the warn
stays in global.rs).
Tests: the monitor is None until initialized, set-once (second init is a no-op),
and independent across instances.
Verification: cargo test -p rustfs-ecstore (21 instance-context tests green),
cargo clippy -p rustfs-ecstore --all-targets (clean), make pre-commit (pass).
Refs: backlog#939 (Phase 5, Slice 10).
The non-inline rename_data commit path made the tmp xl.meta durable and then
fdatasynced the shard files in two sequential awaits, each its own blocking
round-trip. The two operations touch disjoint paths (the tmp xl.meta under the
tmp bucket vs the shard data dir) and have no ordering constraint between them:
both only need to be durable before the commit renames that follow.
Run them concurrently with tokio::join!, dropping a blocking round-trip from
the PUT commit critical path (backlog#922 step 2). The commit ordering is
unchanged — both futures complete before any rename, and a failure in either
aborts before the rename exactly as the sequential version did (tmp-meta error
is still surfaced first). Payload and metadata durability semantics are
identical under strict and relaxed tiers.
Validated by the rename_data crash-consistency harness (backlog#935): a crash
at any pre-commit point still reopens as old-or-new, never mixed. Existing
rename_data / durability-tier / disk::os tests are unchanged and pass.
Refs: rustfs/backlog#922 (HP-1 step 2), rustfs/backlog#936
Co-authored-by: heihutu <heihutu@gmail.com>
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>
The rename_data commit sequence is the highest-risk durability path in the
store, and the HP-1/HP-4/HP-5 work (fsync coalescing, group commit, relaxed
durability tiers) all need a standing gate that proves a power loss mid-commit
can never leave a mixed or corrupt object. There was one graceful-rollback
failpoint but no crash-consistency coverage.
Add a deterministic harness that models a hard power loss — the commit
sequence stops dead at the armed step with no in-process cleanup — and then
reopens the disk to assert the raw on-disk state is coherent:
- Two pre-commit injection points, RenameDataCrashPoint::{AfterDataRename,
AfterBackupBeforeMetaCommit}, constructed at the real commit-path call sites
but gated so the production build compiles them to a const-false no-op
(mirrors the existing should_fail_before_old_metadata_backup pattern).
- A parameterized scenario over {strict, relaxed} durability x {both crash
points} x {overwrite, fresh object}: seed, stage a replacement, inject,
reopen, and assert the object reads back as exactly the old version (or does
not exist when there was no old version) with its data dir intact, never the
half-committed new one. The un-injected run asserts the commit makes the new
version visible. Relaxed is held to the same old-or-new invariant as strict
(only the durability window widens), which is exactly the property the
durability-relaxation work must not break (rustfs/backlog#878 hard rule).
- Wire an explicit `ecstore-crash-consistency` gate into the destructive
profile of run_ecstore_validation_suite.sh so it runs in the standing suite
(coordinates with the #878 destructive profile rather than building a
parallel one).
Production behavior is unchanged: the injection guards are no-ops outside
tests, and the existing rename_data rollback tests still pass.
Refs: rustfs/backlog#935 (HP-14 crash-consistency harness), rustfs/backlog#896
(test plan), rustfs/backlog#878 (ECStore validation suite), 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>
Phase 5 Slice 4 (backlog#939): move the S3 region — a write-once identity
scalar — out of the GLOBAL_REGION process static into the per-instance
InstanceContext, so two instances can serve different regions.
- InstanceContext gains `region: OnceLock<Region>` with `set_region()` /
`region()`. `set_region` keeps the write-once fail-fast contract: a second
write panics, exactly as the process global did (not downgraded to a warn).
- global.rs `set_global_region` / `get_global_region` keep their signatures and
forward to the current instance's context; the GLOBAL_REGION static is
removed. Single-instance: startup writes the bootstrap context (which the
ECStore adopts), so reads are unchanged.
Tests: set/get round-trip, two contexts hold distinct regions, and a second
set_region panics (fail-fast preserved).
Verification: cargo test -p rustfs-ecstore (9 instance-context tests green),
cargo clippy -p rustfs-ecstore --all-targets (clean), make pre-commit (pass).
Refs: backlog#939 (Phase 5, Slice 4). Stacked on Slice 3 (#4417).
* fix(scanner): scope long walk timeouts
* fix(scanner): bound IAM config walks
---------
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com>
fix(ecstore): stop ListMultipartUploads from returning one upload past max-uploads (backlog#954)
The result-collection loop in `SetDisks::list_multipart_uploads` pushed an
upload into the page and only then checked `ret_uploads.len() > max_uploads`,
so each page returned up to max_uploads + 1 uploads and pointed
`next_upload_id_marker` at that surplus entry, violating the S3
ListMultipartUploads max-uploads contract.
Move the cap check before the push (MinIO-style) so a page never exceeds
max_uploads, and derive `is_truncated` from the post-marker cursor position
(`upload_idx < uploads.len()`) rather than comparing the page length against
the full listing length. The old length comparison mis-reported truncation on
the final page whenever a marker had skipped earlier entries, which prevented
marker-based pagination from terminating.
Add a regression test that starts more in-progress uploads than a page holds
and asserts a single page returns exactly max_uploads with a correct
next-marker, that the exact-boundary case is not marked truncated, and that
paginating one upload at a time enumerates every upload once with no loss,
duplication, or non-termination.
Incidental: add the missing `ctx` field to the `new_multipart_lock_test_store`
cfg(test) helper so the ecstore test build compiles after the #4413/#4437
merge collision.
Refs: https://github.com/rustfs/backlog/issues/954
`create_file` writes erasure shard / multipart part data through the page
cache, so the commit-point `sync_dir_files` fdatasync in `rename_data` has to
flush the whole shard's dirty pages (profiling: create_file avg 67.52ms vs
1.51ms nosync; BitrotWriter::write p95 40.86ms), and concurrent PUTs on one
device stall each other's writeback inside the rename critical section.
Add an opt-in Linux O_DIRECT streaming writer that reuses the direct-io read
infrastructure (statx DIOALIGN probe, aligned bounce buffer, supported latch,
one-time fallback log). Incoming bytes are staged into an aligned buffer;
full aligned batches and the shutdown tail are flushed on the blocking pool
(same offloading posture as the buffered writer it replaces). The
sub-alignment tail is written buffered after clearing O_DIRECT via fcntl
(MinIO's recipe). Shard bytes reach the device during the write phase, so the
unchanged commit-point `sync_dir_files` fdatasync degrades to a cheap
metadata/device FLUSH instead of flushing ~2MiB of dirty pages.
Gated by `RUSTFS_OBJECT_DIRECT_IO_WRITE_ENABLE` (default false), Linux-only,
and durability-neutral: `sync_dir_files` still fdatasyncs every shard at the
commit point, so the strict/relaxed/none durability tiers keep their exact
guarantees. Filesystems that reject O_DIRECT (tmpfs, overlayfs, 9p) latch the
path off and fall back to buffered writes; an O_DIRECT open/write EINVAL is
never surfaced as `InvalidInput` (which `to_file_error` maps to `FileNotFound`
and would masquerade as a missing shard, triggering a spurious EC rebuild).
Unit tests cover the env gate, the alignment/staging-capacity and tail-split
math, an end-to-end `create_file` round trip across sizes crossing the
alignment and multi-batch boundaries (buffered fallback on macOS / O_DIRECT on
a block device), and the writer state machine over a plain file on Linux. Real
O_DIRECT throughput on Linux NVMe (ext4/xfs) is deferred to the azure 4-node
bench per the issue's GO/NO-GO gate; the development host is macOS and cannot
exercise O_DIRECT.
Refs: rustfs/backlog#927, rustfs/backlog#936
Co-authored-by: heihutu <heihutu@gmail.com>
perf(ecstore): per-bucket durability tier overrides (HP-5 phase 2)
Let a bucket override the process-wide RUSTFS_DURABILITY_MODE with its own
strict/relaxed/none tier, stored as a durability.json extension entry in the
bucket metadata file and resolved at commit points via effective_durability.
System-critical buckets (.rustfs.sys, .minio.sys) can never carry an override
and stay pinned to strict; the legacy full-off switch keeps its historical
semantics and per-bucket overrides do not apply under it. Overrides are
published and cleared through the existing bucket metadata cache-invalidation
path, and an admin GET/PUT handler exposes the configuration. Default behavior
is unchanged: with no override a bucket follows the global mode, which defaults
to strict and stays byte-for-byte identical to before.
Refs: https://github.com/rustfs/backlog/issues/938, https://github.com/rustfs/backlog/issues/936
Co-authored-by: heihutu <heihutu@gmail.com>
* fix(swift): replace assert!(false) with panic! in expiration_worker test
clippy::assertions_on_constants fails the swift clippy CI job under -D warnings.
* fix(ecstore): add missing ctx field to multipart lock test store
The multipart list-parts lock test (#4437) constructs ECStore without the
ctx field added by the Phase 5 InstanceContext work (backlog#939). Both
landed on main independently, leaving a semantic conflict that breaks the
ecstore lib-test build (E0063). Adopt the process bootstrap context,
matching the existing bootstrap_ctx() test in store/mod.rs.
fix(ecstore): validate erasure distribution values to avoid shuffle index panic (backlog#949)
The element values of `erasure.distribution` read from `xl.meta` were never
range-checked. `FileInfo::is_valid()` and `MetaObjectV1::valid()` only verified
`distribution.len()` and the `erasure.index` bound, not that each distribution
value is a valid 1-based slot in `[1, N]`. The metadata shuffle helpers then use
these values directly as `distribution[k] - 1` indices, so a corrupt or
adversarial `xl.meta` carrying a `0` (usize underflow) or a value greater than N
(out-of-bounds) triggers a panic in the shuffle path, turning bad-disk metadata
that erasure coding is meant to tolerate into a request/task crash.
Fix, two layers:
- Validate distribution values at metadata acceptance: `is_valid_distribution`
now requires the distribution to be a permutation of `1..=N` (correct length,
every value in range, no duplicates). `FileInfo::is_valid()` and
`MetaObjectV1::valid()` use it, so `find_file_info_in_quorum` rejects corrupt
metadata and it surfaces as a clean `ErasureReadQuorum` error instead of an
index path.
- Defensive indexing in the shuffle helpers (`shuffle_disks_and_parts_metadata`,
`_by_index`, `_by_index_owned`, `shuffle_parts_metadata`, `shuffle_disks`,
`shuffle_check_parts`): out-of-range distribution values are skipped via
`checked_sub(1)` + bounds-checked slot access instead of a bare `idx - 1`
index, matching the existing pattern in
`collect_inline_data_shard_fileinfos_by_index`.
Regression tests: `is_valid_distribution`/`is_valid`/`valid` reject distributions
containing `0`, values greater than N, duplicates, and wrong length while
accepting valid permutations; the shuffle helpers no longer panic on corrupt
distributions and preserve output length.
Refs: https://github.com/rustfs/backlog/issues/949
* refactor(ecstore): add per-instance InstanceContext, migrate erasure setup type
Phase 5 of the global-singleton consolidation (backlog#939): begin moving
runtime identity state out of process globals so multiple ECStore instances
can coexist in one process. Isolation is carried by the object graph
(ECStore -> Sets -> SetDisks holding an Arc<InstanceContext>), not a
task-local, which does not propagate across the many internal tokio::spawn
boundaries in the data/background paths.
This first slice migrates the erasure setup type -- previously three
independent process-global bools -- into a single per-instance
RwLock<SetupType> that derives is_erasure / is_dist_erasure / is_erasure_sd,
removing a triple source of truth that could drift out of sync.
- New runtime::instance module: InstanceContext + process bootstrap context.
- The legacy free-function facade (is_erasure/update_erasure_type/...) keeps
its signatures and forwards to the current instance's context, falling back
to the bootstrap context before a store is published.
- ECStore gains a pub(crate) ctx field and setup_is_* accessors; its
constructors adopt the bootstrap context (never mint a fresh one) so startup
writes and post-construction reads share one cell -- single-instance
behavior is byte-for-byte unchanged.
Tests: erasure predicate derivation vs the legacy behavior, object-graph
carrier isolation across two ECStore instances, and bootstrap adoption.
Refs: backlog#939 (Phase 5, Slice 1), backlog#653 (item 8)
* refactor(ecstore): thread InstanceContext down the object graph (Phase 5 Slice 2) (#4415)
* refactor(ecstore): source the namespace lock manager per-instance (#4417)
refactor(ecstore): source the namespace lock manager per-instance (Phase 5 Slice 3)
Phase 5 Slice 3 (backlog#939): give each instance its own lock namespace by
sourcing SetDisks' lock manager from the instance context instead of the
process singleton. This removes the false cross-instance mutual exclusion (and
attendant ABBA risk) that a shared GlobalLockManager would cause once multiple
instances coexist.
- InstanceContext gains a `lock_manager: Arc<GlobalLockManager>`. `new()` mints
a fresh manager (independent per-instance); `bootstrap_ctx()` aliases the
process singleton via get_global_lock_manager(), so a single-instance
deployment keeps exactly one shared namespace.
- SetDisks::new sources `local_lock_manager` from `ctx.lock_manager()` (the ctx
it already adopts), not `runtime_sources::global_lock_manager()`. Single
instance: same Arc as before, so behavior is unchanged.
- Remove the now-unused `runtime_sources::global_lock_manager()` wrapper.
Tests: bootstrap lock manager aliases the process singleton; two fresh contexts
own distinct managers; a SetDisks' lock manager is the one from its context and
aliases the global singleton in a single-instance build.
Verification: cargo test -p rustfs-ecstore (10 Phase 5 + set_disk locking
regressions green), cargo clippy -p rustfs-ecstore --all-targets (clean),
make pre-commit (pass).
Refs: backlog#939 (Phase 5, Slice 3). Stacked on #4415 (Slice 2).
fix(lock): fence write commit on lock loss (backlog#899 Phase 2)
Phase 0+1 (#4388) made object write locks refreshable and marks the guard
lost when the heartbeat can no longer refresh a quorum, but does not act on
it. Under a partition a long write's lock can expire on an unreachable node
and be reclaimed, letting a third party re-acquire it; the original writer
keeps going and both commit -- a double write.
Expose the loss signal through NamespaceLockGuard::is_lock_lost() and
ObjectLockDiagGuard::is_lock_lost(), and fence the commit in put_object and
complete_multipart_upload: immediately before rename_data (the atomic commit
point), abort with a retryable NamespaceLockQuorumUnavailable (503) if the
lock was lost. In multipart the check precedes cleanup_multipart_path so a
lost lock leaves the upload intact and retryable. A write that already
reached rename_data Ok is durable and never aborted.
The loss criterion is unchanged (reacts to Phase 1's signal). Heal and the
long-GET read side are deferred follow-ups.
Replication deletes dropped the S3 `versionId` query parameter for every
replication request, conveying the version only via the internal
x-*-source-version-id header. A generic (non-MinIO/RustFS) S3 target ignores
that header, so a version purge degenerated into a delete-marker creation while
the source still stamped VersionPurgeStatus=Complete — a silent divergence.
Only omit the query `versionId` when propagating a delete-marker CREATION
(the target must mint its own marker); version purges, delete-marker purges and
force deletes now address the exact version. Extracted into
resolve_delete_api_version_id with unit coverage.
Fixesrustfs/backlog#857
* fix(ecstore): make post-commit old data dir cleanup best-effort (backlog#898)
A write is authoritatively committed once rename_data returns Ok (the new
version is durable on >= write_quorum disks and immediately readable). The
subsequent reclamation of the now-dereferenced old object/<data_dir> is pure
space reclamation, yet commit_rename_data_dir propagated a below-quorum GC
failure via `?` into ErasureWriteQuorum -> 503, producing a false-negative
ACK for an already-persisted write. This is a deliberate divergence from
MinIO (erasure-object.go:1577), which couples the two; the divergence is
justified by durability semantics, not parity.
Changes:
- commit_rename_data_dir now returns a structured OldDataDirCleanup receipt
and never returns Err. Adds an old==committed-dir anti-misdelete guard and
a committed_data_dir parameter. Classification is extracted into pure
functions (classify_old_data_dir_cleanup / map_cleanup_join_result /
is_cleanup_not_found) so it is unit-testable. Task panic/cancel is mapped to
a non-ignored DiskError::other (never DiskNotFound), and not-found is
normalized to reclaimed.
- object.rs / multipart.rs consume the receipt instead of `?`. The result
reverts to Ok, so the invalidate_get_object_metadata_cache self-heal and the
capacity/compression accounting that a `?` early-return previously skipped
now run on the cleanup-failure path too.
- On residue, report_old_data_dir_cleanup emits leak metrics and enqueues an
object heal over the existing heal channel (disk-health signal replacing the
503). heal_object -> reclaim_orphan_data_dirs already reclaims unreferenced
local data dirs, closing the loop end to end.
- Adds rustfs_old_data_dir_* counters (attempted/reclaimed/leaked/below_quorum)
as the operator-visible backstop for leaked residue.
- Adds a test-only (#[cfg(test)]) delete fault-injection seam; in production it
inlines to a no-op None and has no behavioral effect.
Tests: pure-function A/C group + join-error mapping + actions decision; A5/A5b
real-disk guard/reclaim integration; end-to-end overwrite returning 200 while
old-data-dir cleanup fails. #864 rollback guard test remains green.
* fix(ecstore): resolve merge conflicts with origin/main in io_primitives.rs
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: houseme <housemecn@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>
fix(lock): renew distributed locks via heartbeat; wire server refresh (backlog#899)
A held distributed write lock had a fixed 30s TTL and was never renewed, so
any operation exceeding it got its per-node lease reclaimed and stolen by a
contender, causing split-brain writes. This implements Phase 0 + Phase 1 of the
#899 design (Phase 2 abort-on-loss is deferred and tracked in code comments).
Phase 0 (server refresh wiring, P0):
- node_service handle_refresh was a no-op stub that parsed args then always
returned success=true. Extract a testable `refresh_lock` free function that
actually delegates to the node's lock backend. Not-found maps to
success=false with no error_info, so RemoteClient::refresh keeps its
Ok(resp.success) semantics and yields a real not_found signal to the
coordinator heartbeat. Without this, client heartbeats were silently no-ops.
Phase 1 (client heartbeat + safe interval + observability):
- DistributedLockGuard spawns a heartbeat that refreshes every per-client lease
on a derived interval; interval is derived without Duration::clamp
(entries<=1 or interval>=ttl => no spawn), fixing the sub-second-ttl panic.
- Add LockLostSignal: declare the lock lost when not_found exceeds
entries.len() - refresh_quorum; RPC errors are not counted (absorbed by the
ttl > interval margin). Expose is_lock_lost()/lock_lost() for observers.
- disarm(), release(), and Drop now abort the heartbeat before releasing so no
refresh races the unlock (refresh only extends, never creates, a lease).
- Reclaim path stays behaviorally unchanged but now warns with owner/resource/
lease age and records a metric (#698 scavenger preserved).
- Add DEFAULT_LOCK_REFRESH_INTERVAL, LockRequest.refresh_interval (serde default
for RPC back-compat) + builder, and lock lifecycle metrics.
Open questions adopt the design's documented defaults (marked TODO in code):
refresh not-found -> Ok(false)/error_info=None; lost-quorum base entries.len();
DEFAULT_LOCK_REFRESH_INTERVAL=10s.
Tests: heartbeat keepalive/quorum-loss/jitter/boundary/disarm (lock crate),
server refresh delegation (rustfs), and end-to-end survives-past-ttl plus
crashed-owner-reclaim regressions (namespace).
* perf(ecstore): move speculative tmp cleanup off the PUT hot path
On a successful PUT, rename_data has already moved the data dir out of the tmp workspace, so the delete_all(RUSTFS_META_TMP_BUCKET) at the end of SetDisks::put_object is a speculative no-op safety net. It was awaited inline on the response path, where profiling (backlog#924 / HP-3) showed the same-disk queueing behind fsync-heavy load turns a ~49us no-op into ~9ms average (p99 77ms, macOS F_FULLFSYNC amplified) added to every PUT.
Run that cleanup on a spawned task instead, keeping it as a real backstop (rename_data's remove_std only removes empty dirs and silently ignores failures). The failure path (quorum loss / rollback) keeps the cleanup inline so a failed PUT never returns with tmp shards still on disk. If the process dies before the spawned task runs, cleanup_stale_tmp_objects (24h expiry, 5-minute loop) reclaims the entry.
Scope note: ops/multipart.rs delete_all on RUSTFS_META_MULTIPART_BUCKET is intentionally untouched; it removes real leftovers and deferring it would widen CompleteMultipartUpload/Abort races.
Regression tests (hermetic SetDisks on formatted local disks, no global state): PUT success drains the tmp workspace (polling the spawned task), and PUT failure (missing bucket volume, rename_data quorum error after tmp shards were written) cleans the workspace inline before returning.
Ref: https://github.com/rustfs/backlog/issues/924
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(ecstore): do not retry NotFound in reliable_rename_inner
reliable_rename_inner blindly retried the rename once on any error. A NotFound retry cannot succeed: nothing recreates the missing source or parent directory between attempts, so the second rename fails identically and speculative cleanup renames (e.g. move_to_trash on an already-removed tmp path) always paid for two syscalls.
Extract the retry decision into should_retry_rename: NotFound returns immediately, any other error keeps the existing single retry. This helper is shared by the rename_data commit path via rename_all, so behavior there is covered by a new rename_all success regression test alongside the retry-predicate tests.
Ref: https://github.com/rustfs/backlog/issues/924
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
Step 1 of 4 for rustfs/backlog#922 (HP-1): write_all_internal previously
coupled file-content durability (fdatasync) with directory-entry
durability (parent dir fsync) behind a single bool. For tmp files that
are immediately renamed away, the tmp parent dir fsync contributes
nothing to crash consistency: the safe-rename recipe only needs file
content fdatasync -> rename -> fsync of the destination parent, because
the rename removes the tmp directory entry and a crash before the
rename means the PUT was never acknowledged.
Replace the bool with a SyncMode enum (None / FileAndDir / FileOnly)
and use FileOnly at exactly the two write-then-rename tmp write points:
the tmp xl.meta write in the non-inline rename_data path and the tmp
write inside write_all_meta. write_all_public (format.json etc.) and
the old-metadata rollback backup keep FileAndDir since those files stay
where they are written. The rest of the commit sequence (shard
sync_dir_files, rename, destination parent fsync) is untouched, and
behavior with drive sync disabled is unchanged.
This saves one directory fsync per disk per non-inline PUT (4 on a
4-drive set). Unit tests assert, via a test-only fsync-dir recorder,
that tmp write points no longer fsync their parent while the public
write point, the rollback backup, and the commit-rename destination
parent still do.
Co-authored-by: heihutu <heihutu@gmail.com>
Under a full nextest run on loaded machines the legitimately serialized
cross-disk commits in concurrent_resend_same_part_commits_one_generation
exceed the acquire deadline by themselves: observed at the 5s default,
at the 30s production default (#4370), and on CI even at 60s — which is
a hard ceiling, because fast_lock clamps every requested timeout to
MAX_ACQUIRE_TIMEOUT (60s) while the Timeout error still reports the
requested value, so raising the env override higher is a no-op.
Request the full 60s ceiling explicitly and cap the queue depth at three
concurrent resends, bounding the last waiter to two serialized commits
(~12s each on the slowest observed CI runner). Three resends still race
the streaming phase and contend on the commit lock, which is all the
generation-mixing regression (backlog#853) needs; every correctness
assertion is unchanged.
Co-authored-by: heihutu <heihutu@gmail.com>
MetaObject::into_fileinfo indexed part_sizes[i]/part_actual_sizes[i] by
part_numbers.len() without checking the arrays are the same length,
unlike the adjacent part_etags/part_indices which are length-guarded.
decode_from pushes the three arrays independently and the xl.meta CRC
only covers bytes, so a CRC-valid but internally inconsistent xl.meta
(foreign writer / MinIO interop) triggers an out-of-bounds panic on the
GET/HEAD/LIST decode path.
Guard the three arrays for equal length and return Err(FileCorrupt) so a
divergent shard is skipped and quorum uses the other disks, instead of
panicking the request task. Cascade into_fileinfo to Result across its
callers, and fix io_primitives early-return to derive the version id from
the merged header and fall into the per-disk loop (single-disk survival +
heal). The 2118 merge-first path is left as a documented follow-up.
Refs backlog#900 (filemeta-01).
test(interop): real-MinIO read + migration parity, Phase 1/2 (backlog#580)
Capture authentic on-disk fixtures from MinIO RELEASE.2025-07-23 (a bucket with
versioning, object-lock, lifecycle, tagging, quota, a public policy, SSE-S3
encryption, a webhook notification target, and a replication rule, plus inline /
versioned / multipart objects and a delete marker) and prove RustFS reads and
migrates them losslessly:
- filemeta parses_real_minio_object_xlmeta: small inline, two-object-version +
delete marker, and multipart object xl.meta parse to the expected FileInfo.
- ecstore parses_real_minio_bucket_metadata_blob_without_loss: the MinIO
.metadata.bin msgpack decodes via the PascalCase field names and
parse_all_configs loads all ten config types present (policy, lifecycle incl.
<ExpiryUpdatedAt>, object-lock, versioning, tagging, quota, notification,
encryption/SSE-S3, replication incl. DeleteMarkerReplication /
ExistingObjectReplication) without loss.
- ecstore reads_minio_inline_bucket_metadata_via_bitrot: MinIO inlines an object
body as [HighwayHash256 32B][body]; RustFS's BitrotReader with HighwayHash256S
verifies and yields the exact blob (the "inline_data 前缀不同" is that prefix).
- ecstore migrates_real_minio_bucket_metadata_end_to_end: on a throwaway 4-drive
local ECStore, a real MinIO .metadata.bin seeded under .minio.sys is migrated
into .rustfs.sys byte-identically for every config, exercising the Phase 2
source adapter (MIGRATING_META_BUCKET = ".minio.sys") through the object layer.
All four run as ordinary crate tests (nextest CI). Phase 4 (MinIO re-reading a
RustFS drive) is documented as out of scope for one-way migration.
Refs rustfs/backlog#580
* refactor(ecstore): extract LocalIoBackend trait behind LocalDisk
Model the LocalDisk per-file I/O hot path as a LocalIoBackend trait
(pread_bytes / open_read_stream / open_full_read / open_write) and move
the existing method bodies verbatim into a default StdBackend. LocalDisk
holds Arc<dyn LocalIoBackend> and the DiskAPI methods forward to it.
This is a behavior-preserving refactor: no logic, error-mapping, metrics,
or cfg-branch changes. It creates the seam for an alternative
runtime-probed io_uring backend (rustfs/backlog#894) without touching
DiskAPI callers. Commit-point durability (fdatasync -> rename ->
fsync-dir in rename_data) deliberately stays outside the trait.
Add a differential test asserting all four read shapes return identical
bytes across page-boundary and file-tail ranges.
Tracking: rustfs/backlog#891 (parent rustfs/backlog#897)
Co-Authored-By: heihutu <heihutu@gmail.com>
* feat(ecstore): true O_DIRECT read path with per-disk graceful fallback (#4366)
* feat(ecstore): true O_DIRECT read path with per-disk graceful fallback
Implement a real O_DIRECT positioned read inside StdBackend::pread_bytes
(Linux only) and wire up the previously dead RUSTFS_OBJECT_DIRECT_IO_READ_*
knobs, which had zero call sites.
Open with rustix OFlags::DIRECT, probe the DIO alignment once per disk via
statx STATX_DIOALIGN (4096 fallback), read the aligned superset into an
alignment-allocated bounce buffer with a short-read loop, and slice out the
exact logical range so padding never reaches BitrotReader.
Any O_DIRECT failure falls back to the buffered read methods; EINVAL or
EOPNOTSUPP (tmpfs, overlayfs, 9p) latches the path off per disk with one
warning. O_DIRECT errors never surface: EINVAL maps to FileNotFound in
to_file_error and would trigger spurious EC rebuilds.
Default behavior is unchanged (knob off). macOS keeps F_NOCACHE.
Tracking: rustfs/backlog#892 (parent rustfs/backlog#897)
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(ecstore): add P1.5 benchmark gate harness for O_DIRECT reads (#4369)
* test(ecstore): add P1.5 benchmark gate harness for O_DIRECT reads
Add an ignored, Linux-only release-mode test (direct_read_bench_gate)
that measures DiskAPI::read_file_mmap_copy through a real LocalDisk with
cold-cache enforcement (fadvise DONTNEED between rounds), reporting
p50/p95/p99/mean latency, wall time, process CPU time, and throughput as
one JSON line for A/B diffing between the buffered baseline and the
O_DIRECT candidate selected by the production env knobs.
Content is verified byte-for-byte before any timing starts.
Tracking: rustfs/backlog#893 (parent rustfs/backlog#897)
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(ecstore): add concurrency dimension to P1.5 bench harness
Model the EC GET shape (FuturesUnordered over concurrent shard reads)
via RUSTFS_BENCH_CONCURRENCY (default 1, sequential as before). Needed
to evaluate blocking-pool pressure for the io_uring gate decision.
Tracking: rustfs/backlog#893
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
* ci: retrigger after cancelled required check
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(ecstore): use is_multiple_of in O_DIRECT bench cache-drop check
clippy's manual_is_multiple_of (rust 1.96) fails -D warnings on the benchmark helper's `done % file_count == 0`.
Verification:
- cargo clippy -p rustfs-ecstore --all-targets
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
houseme