Commit Graph

229 Commits

Author SHA1 Message Date
terem42 cb93ac5df1 fix(ecstore): purge the stale destination data dir on healing rename_data commits (#5822)
* fix(ecstore): purge the stale destination data dir on healing rename_data commits

Heal commits reuse the version's existing data_dir, so when repairing
in-place corruption (bitrot) the destination directory still exists and
holds the corrupt shard files. rename(2) cannot replace a non-empty
directory (EEXIST on XFS, ENOTEMPTY on ext4), so the commit failed on
every attempt — including all scheduler retries — and in-place bitrot was
detected and reconstructed but never repaired.

Purge the stale destination data dir (move_to_trash) before the commit
rename, for healing commits only: fresh PUTs mint a new data_dir and can
never collide, and a non-healing collision keeps failing loudly. Adds the
FileInfo::is_healing() reader for the marker set_healing() already writes.

* style(ecstore): emit the heal purge failure as a structured event

The new warning was the only sentence-style log in `rename_data`'s commit
path — it sat ten lines above `info!(event = EVENT_DISK_LOCAL_RENAME_REJECTED,
component = ..., subsystem = ...)` and interpolated its values into the
message instead of carrying them as fields, so it is invisible to any operator
query keyed on `event`.

Give it the shape the rest of the file uses: a named
`EVENT_DISK_LOCAL_HEAL_PURGE_FAILED`, `component`/`subsystem`, `dst_path` and
`error` as fields, and a short label as the message. Level stays `warn` — the
purge is best effort and the rename below fails closed — and the condition,
the branch, and the control flow are unchanged.

---------

Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
2026-08-07 23:10:53 +00:00
Zhengchao An 96d24bc006 docs(agents): make the structured-logging rule reachable and enforceable (#5828)
The RustFS event shape (`event`/`component`/`subsystem`/`result` + context,
message last) is specified only in
`.agents/skills/rustfs-logging-governance/SKILL.md`, and nothing routes a
change to it:

- `AGENTS.md`, which is what an agent actually loads by default, never
  mentions logging. Its only related line is "log unknown fields at `warn`"
  under Serde Safety, which is about level, not shape.
- The skill's `description` says "use when editing or reviewing RustFS logs",
  so a bugfix that adds one log line in passing — how most new log sites enter
  this repo — never matches it.
- `scripts/check_logging_guardrails.sh` is a blocklist: 500+ `rg -F` literals
  that retire log lines which already shipped. It cannot see a newly written
  one. For `crates/ecstore/src/disk/local.rs` the only check is that
  `#[tracing::instrument]` is TRACE-only; `warn!`/`info!` shape is unchecked.

PR #5822 landed `warn!("heal rename_data: purging ... {:?} failed: {}", ...)`
in `disk/local.rs` — sentence-style, no fields, directly beside `info!(event =
EVENT_DISK_LOCAL_RENAME_REJECTED, component = ..., subsystem = ...)` — with
every check green. That is the gap, not an authoring mistake.

Close all three:

- `AGENTS.md`: a Logging section stating the field shape, the level policy,
  the reuse-the-file's-constants rule, and that it applies to any `tracing`
  macro added in passing, not only to log-focused changes.
- Skill `description`: trigger on adding or editing any `tracing` macro,
  naming the single-line-added-in-passing case explicitly.
- Guardrail: assert the event shape positively on the already-governed disk
  files — `error!`/`warn!`/`info!` must open with fields or a `target:`, never
  a bare string. Commented-out macros are excluded; `debug!`/`trace!` stay out
  of scope as targeted diagnostics. Self-test fixtures cover both directions.

`crates/ecstore/src/disk/mod.rs` carried the one live violation in that file
set (`conv_part_err_to_int`), so it is converted here; the guardrail would
otherwise fail on an untouched file.

Verification:
- `./scripts/check_logging_guardrails.sh` — passes
- Negative control: re-inserting PR #5822's exact `warn!` line into
  `disk/local.rs` makes it exit 1 pointing at that line
- `cargo fmt -p rustfs-ecstore -- --check`, `cargo check -p rustfs-ecstore`
2026-08-07 23:05:57 +00:00
Zhengchao An 8c9e884cf2 fix(ecstore): make inline-rollback reclamation file-precise to keep #5703's child-key safety (#5732)
#5724 reclaimed the synthetic inline-rollback dir after a committed rename with delete_data_dir(recursive: true), which has no notion of object metadata: for unversioned objects the synthetic UUID is a fixed, publicly-known constant, so object/<rollback-dir> can simultaneously be a legitimate child key's directory, and recursively deleting it reopens the authorization bypass #5703 closed (PutObject on K destroying K/<uuid> without DeleteObject permission).

Replace the recursive pass with a file-precise one: after quorum commit, delete exactly object/<rollback>/xl.meta.bkp with a non-recursive delete on every disk whose rollback dir is not also the cleanup dir. The parent-rmdir walk removes the dir only when the backup was its sole content, so the BucketNotEmpty leak fix is preserved (#5724's regression test passes unchanged) while a child key at the same path keeps its metadata. The undo path's restore_metadata_backup now also reclaims the emptied synthetic dir, mirroring restore_delete_rollback.
2026-08-05 03:32:41 +00:00
Zhengchao An 42af6e3b63 fix(ecstore): isolate inline rollback cleanup (#5703) 2026-08-04 23:20:29 +08:00
Zhengchao An e64ed14fb0 fix(ecstore): address review comments for batch shard pread (#5680)
fix: address review comments for batch shard pread
2026-08-03 22:36:49 +00:00
rdiperri-wasabi de8cb5f26c perf(ecstore): batch local EC shard preads on GET (#5679)
Collapse per-shard blocking-pool round-trips into one spawn_blocking
pread batch when all online shards are local and mmap-read is enabled.

Co-authored-by: ba <ba@ubuntu-server.alpha30.bos16>
Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
2026-08-04 05:53:22 +08:00
Zhengchao An 98d3619613 fix: address rc.1 release blockers (#5648)
* fix: address rc.1 release blockers

* fix: route release guards through architecture boundaries

* fix: close remaining rc.1 regression gaps

* refactor: group multipart listing options

* fix: resolve rc.1 CI regressions

* fix(ecstore): keep bucket-config writes off the caller's stack

A bucket-config write nests incarnation resolution (which can drive legacy
migration and a peer fan-out), a full metadata load, and `save` — itself an
object PUT that pulls in the whole erasure write path. Every request that
mutates bucket config is already several futures deep, so inlining all of
that into one state machine overflows the 2MiB worker stack in debug builds.

Two CI lanes aborted with SIGABRT on this:

  ILM Integration (serial)
    rustfs app::lifecycle_transition_api_test::
      compensation_driven_complete_multipart_upload_still_transitions
  Test and Lint (swift)
    rustfs-protocols::swift_metadata_persistence::
      swift_metadata_writes_are_durable

Neither test file is touched by this branch and both lanes are green on
main. Stack-pointer probing showed ~780KiB consumed between
`metadata_sys::update` and the config read alone, with single hops of
363KiB (`update` -> `acquire_config_write_guard_for_incarnation`), 125KiB
and 105KiB.

Box the deep sub-futures on both read-modify-write paths (`update` /
`update_checked` and `update_config_with` / `update_config_with_checked`)
so each guard's own state machine stays small. Behaviour is unchanged;
`update` -> guard drops to 253KiB and both tests pass on the default stack.

* fix(lifecycle): unbreak restore under the bucket generation fence

The ILM lane aborted on a stack overflow before reaching these, so they
were never reported; with that fixed, four restore tests fail. All four
are green on main and none of their test files are touched by this branch.

1. RestoreObject and ListMultipartUploads hard-required
   `opts.expected_bucket_incarnation_id`, but `apply_bucket_generation_guard`
   deliberately leaves it unset when no guard extension is present — only the
   S3 access layer installs one. Every direct caller therefore got
   `InternalError: ... bucket generation guard is missing`. Resolve the
   current generation instead, the way the copy path already does. The fence
   is unaffected: RestoreObject still re-reads the incarnation from disk and
   compares before admitting the restore, and the multipart listing is
   filtered by the value it resolves.

2. `restore_expiry_snapshot_matches` (new on this branch) rejected every
   restored-copy expiry whose `restore_expires` had not already elapsed.
   Whether the restored copy is due to expire is the ILM evaluator's
   decision, made when it emitted DeleteRestoredAction; re-deriving it in
   the set layer only adds a way for a legitimate action to be rejected.
   The stale-event risk it appears to guard is already covered by the
   surrounding snapshot match — a re-restore rewrites `restore_expires`,
   so a replayed event fails the equality check. Drop the clause; the
   fifteen identity clauses are unchanged.

Fixed:
  rustfs app::lifecycle_transition_api_test::
    restore_object_usecase_accepts_exactly_one_of_two_concurrent_restores
    restore_object_usecase_completes_suspended_null_version_in_place
    restore_object_usecase_reports_ongoing_conflict
  rustfs-scanner::lifecycle_integration_test serial_tests::
    test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore

Verification: the CI ILM lane filter now runs 53/53 green locally.

* chore: address review follow-ups on this branch

Four items from the adversarial review that were still open.

- Restore the assertion `test_bucket_replication_replayed_delete_marker_
  preserves_source_mtime_without_source_restart` is named for. The branch
  had replaced the backlog#867 mtime check with `assert_replication_
  converged`, which any successful replication satisfies, and deleted the
  two helpers it needed — so the regression the test exists to catch would
  now pass. This matters here specifically because the branch changes the
  flag feeding `replication_delete_remove_options` and routes replay
  through a new file and ordering.

- Drop `read_config_no_lock_preserve_empty`: zero production callers (the
  one real consumer calls the `_with_metadata` variant directly). Its test
  stanza now exercises that variant, so the coverage moves to live code
  rather than being deleted.

- Revert the `bytesize` bump. It is a no-op: `Cargo.lock` already pinned
  2.7.0 before this branch and is untouched, so the caret range already
  resolved there. Nothing in the diff uses the crate.

- Split the AGENTS.md "Adversarial Validation" policy change out of this
  branch. The edit is defensible on its own, but it relaxes the review gate
  that this branch has to pass, so it should land as its own PR reviewed on
  its own merits rather than bundled with the change that benefits from it.
  The reverted hunks are unchanged and ready to re-apply.

Not changed, deliberately: the missing-sidecar path still fails closed.
`missing_bucket_incarnation_sidecar_for_new_metadata_fails_closed` pins
that on purpose, and serving a non-authoritative Object Lock state would
be the wrong trade. The residual concern stands and is recorded in review
— a crash between the two writes in `persist_new_and_set` leaves the
bucket unloadable until DeleteBucket+CreateBucket, and the repair branches
in `migrate_legacy_metadata` and `make_bucket` are unreachable dead code
for that case. Resolving it needs the read path and the (transaction-lock
holding) repair path to be separated, which is more than a follow-up edit.

* test(ci): serialize the new bucket-incarnation tests

The five tests this branch adds around the incarnation / lifecycle fence
drive `init_bucket_metadata_sys` and `bucket_metadata_sys_of` — process-global
OnceLock state that `serial_test`'s `#[serial]` cannot protect across
nextest's process boundary — and they delete+recreate buckets, the shape that
raced into InsufficientWriteQuorum in backlog#937.

Add them to the `ecstore-serial-flaky` group in both the default and ci
profiles (nextest evaluates a named profile's own overrides list, so the
ci mirror is required). Preventive serialization only, no retries.

Not a full fix for the review comment: `bucket_delete_waits_for_config_
mutation_fence` still proves liveness with a fixed 200ms sleep plus
`assert!(!delete.is_finished())`. Turning that into readiness polling needs
a production-side signal to wait on — asserting "still blocked" is inherently
a negative. Serializing the group removes the parallel-load pressure that
makes the window fragile; the sleep itself is left for a follow-up.

* test(ecstore): pin that a drained bucket is actually deletable

`DeleteBucket`'s emptiness check is `has_xlmeta_files`, a raw scan of the
bucket directory on local disks — not an S3-level listing. So "the client
drained the bucket" and "the bucket is deletable" are two different
contracts, and only the first one was covered.

That gap is what the `S3 Implemented Tests` lane is failing on: 219 cases,
all `BucketNotEmpty` on `nuke_prefixed_buckets`, with every test body
passing. The first one is `test_versioning_obj_suspend_versions`, reported
by pytest as PASSED followed by ERROR at teardown.

Add the missing assertion for the unversioned path: PUT, client DELETE,
then assert no `xl.meta` survives and `DeleteBucket` succeeds. It passes —
which is itself a result: the plain delete path leaves no residue, so the
s3-tests failure is not there.

The versioning-suspended path is the remaining suspect (the client DELETE
leaves a null delete marker, and draining means purging it by
`versionId=null`). It is not covered here: `BucketVersioningSys` resolves
through the ambient `get_bucket_metadata_sys()` OnceLock, which this unit
env cannot set, so the bucket never actually reports as suspended. That
repro belongs at the e2e layer where a real server owns the versioning
state.

* fix(ecstore): let an explicit null-version delete purge its delete marker

Root cause of the `S3 Implemented Tests` lane: 219 cases, all
`BucketNotEmpty` on `nuke_prefixed_buckets`, every test body passing.

On a versioning-suspended bucket a client DELETE leaves a null delete
marker — correct S3 semantics, and an `xl.meta` on disk. Draining the
bucket therefore means purging that marker as `?versionId=null`, which is
what `nuke_bucket` does before `DeleteBucket`. That purge was rejected:

    explicit null-version purge of the null delete marker must succeed,
    got [Some(MethodNotAllowed)]

so the marker survived, and `DeleteBucket`'s emptiness check — a raw
`has_xlmeta_files` scan of the bucket directory, not an S3 listing — kept
reporting the bucket as non-empty.

The two sides of the version comparison in the batch delete loop are in
different namespaces. `goi.version_id` is the client-facing identity, where
`from_file_info` synthesizes `Some(Uuid::nil())` for a null version on a
versioned *or versioning-suspended* bucket. `version_id` is the storage
identity, where `delete_file_info_version_id` maps an explicit
`?versionId=null` to `None`. Comparing them raw makes the purge look like a
version mismatch, so `explicit_delete_marker` is false and the
`MethodNotAllowed` from the lookup is recorded as a delete failure.

This only became reachable on this branch: previously `check_opts` did not
carry `dobj.version_id`, so `set_disk_delete_creates_delete_marker` was
true, `object_lock_check_required` was false, and the lookup that produces
`MethodNotAllowed` never ran. Adding the version id to `check_opts` lit up
a comparison that was already wrong.

Normalize both sides through `delete_file_info_version_id`.

The regression test injects a real Suspended bucket-config snapshot — the
delete path reads versioned/suspended from that snapshot, not from `opts`,
so without it `from_file_info` never synthesizes the null version id and
the branch is not reached. Mutation-checked: restoring the raw comparison
fails the test with the exact `MethodNotAllowed` above.

* fix(app): drop the now-needless struct update

Reverting `crates/replication` to main removed the extra `MrfReplicateEntry`
fields, so this literal specifies every field again and `..Default::default()`
trips `clippy::needless_update` under `-D warnings`.

Caught by CI, not locally: I had run `cargo check --workspace --all-targets`,
which does not see clippy-only lints. Ran `cargo clippy --workspace
--all-targets -- -D warnings` here — clean.

* test(e2e): assert the fresh-volume classification

four_node_empty_legacy_volumes_start_as_fresh only started the cluster and
listed buckets — no assertion, so any classification path that still permits
startup left it green without proving the pre-created empty `.minio.sys`
directories were treated as fresh volumes.

Pin what that classification actually leaves behind: no buckets adopted into
the namespace, `.rustfs.sys/format.json` written on every drive, and the empty
legacy directory left untouched rather than migrated into.

* fix(bucket): apply the requested Object Lock to existing buckets

Site replication replays make-with-versioning against the destination,
carrying the source's `lockEnabled`. When the destination bucket already
exists it takes `force_create`, and the whole option-application block was
gated on `confirmed_missing` — so the call returned success while the replica
stayed unlocked. Replicated versions could then be deleted without the
retention the source enforces.

Object Lock enable is one-way, so applying it to an existing bucket is safe:
move it out of the creation-only gate, keeping `created` and versioning-only
options creation-scoped as before.

An existing authoritative bucket takes the `cache_bucket_metadata_in` branch,
which only caches, so the enable would have been dropped on restart. Persist
instead when the enable actually changed something.

Mutation-checked: restoring the creation-only gate fails the new
`force_create_enables_object_lock_on_an_existing_bucket` with "Object Lock
must be enabled on the existing bucket".

cargo nextest run -p rustfs-ecstore --lib: 3633 passed.

* fix(ecstore): box the generation-checked config mutation paths too

The earlier stack fix boxed `update` and `delete`, but an authorized
bucket-config mutation carrying an incarnation takes `update_if_incarnation`
/ `delete_if_incarnation` instead — which were still inlining the whole
resolve/load/save chain into an already-deep request future. Same overflow,
sibling path.

* fix(restore): keep the nil-version normalization the strip removed

Reverting the replication subsystem to main took `set_disk/replication.rs`
with it, but one line in that file was this branch's own fix rather than
replication work:

    -  self.version_id.filter(|v| !v.is_nil()) == fi.version_id.filter(|v| !v.is_nil())
    +  self.version_id == fi.version_id

For a versioning-suspended object the expected version is `Some(Uuid::nil())`
while the read-back `FileInfo` carries `None`, so the raw compare reports
every suspended restore as "restored object changed before restore metadata
finalization" and the copy-back never commits. Same nil-vs-None mismatch as
the null delete-marker purge fixed earlier on this branch.

Caught by `Test and Lint (rio-v2)`, not by my local runs: the test lives in
`transition_commit_failure_tests`, gated behind `feature = "test-util"`, so
the 3633-test suite I had been running never included it. Re-ran with
`--features rio-v2,test-util`: 3722 passed.
2026-08-03 19:25:43 +00:00
Henry Guo b563230782 fix(ecstore): allow Windows renames under guarded parents (#5663)
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com>
2026-08-03 16:11:37 +08:00
houseme a8574d0104 fix(metrics): close dimension review gaps (#5656)
* fix(metrics): close dimension review gaps

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(metrics): cover dimension review gaps

Co-Authored-By: heihutu <heihutu@gmail.com>

* test(metrics): cover failed disk info UUID fallback

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-08-03 04:12:31 +00:00
houseme 035ce5d784 feat(obs): add bounded metrics dimensions (#5645)
* feat(obs): add drive topology detail metrics

Expose additive drive info, topology, state, and per-drive API metrics while preserving the existing drive metric label sets.

Backlog: rustfs/backlog#1655

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): preserve suspect drive runtime state

Keep suspect as a bounded drive runtime state and avoid all-zero runtime_state samples for that storage health state.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): skip unknown drive inode samples

Avoid exporting zero inode gauges for missing or stale drive snapshots and ignore zero-count API latency buckets.

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add scanner source work detail metrics

Expose additive scanner source and cycle work metrics with bounded server/source/state labels while leaving the existing aggregate scanner metrics unchanged.

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add ilm action detail metrics

Expose additive ILM action/state task metrics with a server label while preserving the existing aggregate ILM series.

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add delivery target server metrics

Expose additive audit and notification delivery target metrics with server labels and extend removed-target tombstones for the server-aware series.

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add replication target flow metrics

Expose additive bucket replication target sent and failed-flow metrics while preserving existing bucket aggregates and target backlog series.

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add request server metrics

Expose additive API request metrics with server labels while preserving the existing request and traffic metric label sets.

Co-Authored-By: heihutu <heihutu@gmail.com>

* style(obs): apply rustfmt to metrics changes

Apply rustfmt output to the metrics dimension changes without altering behavior.

Co-Authored-By: heihutu <heihutu@gmail.com>

* style(obs): reuse audit target label constant

Use the exported audit target_id label constant for legacy audit target metrics.

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): populate drive disk metrics

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add scanner bucket drive result metrics

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(obs): add replication proxy server metrics

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): address metric liveness review

Use checked division for drive API latency aggregation and keep recovered drive, scanner current-cycle, replication flow, audit target, and notification target series from retaining stale values.

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): address metric dimension review

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): address additional metric review

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): count drive calls at start

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): address metrics dimension review

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(metrics): address dimension review gaps

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(metrics): address scanner review follow-ups

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(metrics): address runtime review follow-ups

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(metrics): reduce disk metric contention

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(metrics): address runtime review follow-ups

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(metrics): retire stale dimension series

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-08-03 09:03:34 +08:00
houseme d5c6ba99d5 fix(ecstore): harden HotPath profiling boundaries (#5555)
* test(hotpath): gate mimalloc heap test by platform

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): attribute HotPath CPU measurements to impls

Co-Authored-By: heihutu <heihutu@gmail.com>

* feat(ecstore): trace raw shard I/O with HotPath

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): redact profiler and OPA endpoint diagnostics

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): settle encoded queue accounting

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): abort encoder producer on cancellation

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-08-01 08:49:48 +00:00
Zhengchao An f5348d5cc4 fix(ecstore): settle lease-deferred data-dir deletes before bucket removal (#5516)
A streaming GET holds snapshot leases on the object's data directories,
and DeleteObjects defers their physical cleanup until the leases are
released. A DeleteBucket issued inside that window passes the xl.meta
emptiness check but fails closed in the non-force delete_volume tree
removal on the leftover part files, returning BucketNotEmpty for a
logically empty bucket.

This is what intermittently failed the s3tests
test_encryption_sse_c_multipart_bad_download teardown in CI: the test
never reads its 30MiB GET body, so the server-side stream (and its
leases) stays alive until the connection drops, racing the teardown's
DeleteObjects + DeleteBucket sequence. With the body held open the
failure reproduces 5/5 locally; after this change it passes 20/20, and
the real s3-tests case passes 20 consecutive runs.

delete_volume now executes the registry-tracked pending deferred
deletions for the volume before removing the directory tree. Only data
dirs whose logical delete already committed are touched; unknown files
still fail closed with VolumeNotEmpty.
2026-08-01 03:26:58 +00:00
houseme 7051a5ce41 feat: add opt-in hotpath profiling (#5488)
* feat: add opt-in hotpath profiling

Co-Authored-By: heihutu <heihutu@gmail.com>

* test: fix vault kms client construction

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-31 01:49:19 +08:00
cxymds 8601179c39 fix(ecstore): quarantine rejected format members (#5463) 2026-07-30 04:09:08 +00:00
Zhengchao An 422e0ad768 test(ecstore): fix rename_all WARN flake from callsite-interest poisoning (#5448)
rename_all_missing_source_still_warns and rename_all_real_failure_still_warns
assert that `warn_reliable_rename_failure` emitted its WARN, but that is a
single production callsite shared with tests that call rename_all *without*
installing a subscriber — rename_all_missing_source_returns_file_not_found,
two tests above, is one of them.

tracing caches each callsite's Interest process-globally and the first thread
to reach a callsite fixes that value; while at most one dispatcher is
registered, tracing-core derives it from the registering thread's own
subscriber, and registration is once-only. When the subscriber-less sibling
wins, the callsite is cached as Interest::never() and the WARN never fires,
so the assertion sees empty output:

    ordinary missing-source failures must keep the WARN, got:

Reproduced at 3/25 with `disk::os::tests::rename_all_missing_source` (both
tests), against 0/20 for the victim alone. Fixed by pinning callsite interest
inside warn_capture(), so every current and future user of that helper is
covered rather than just the two tests that happen to fail today.

pin_callsite_interest_for_test() moves from cluster::rpc::background_monitor
to a new crate-level test_tracing module: it is domain-neutral and now has
consumers in two unrelated subsystems, and disk::os should not have to reach
into a cluster::rpc test helper.

Verified: repro filter 0/30 (was 3/25); disk::os:: 0/12; cluster::rpc:: 0/12
and its poisoner pair 0/15, confirming the moved helper still holds.

Follow-up to #5438. Closes the last item in #5439.
2026-07-30 07:26:12 +08:00
Henry Guo abc5f2e818 fix(scanner): persist portable usage cache keys (#5444)
* fix(scanner): persist portable usage cache keys

* fix(scanner): validate complete bucket cache graphs

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: houseme <housemecn@gmail.com>
Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-29 23:22:30 +00:00
Zhengchao An 2ed28f9c5f fix(ecstore): prevent recursive delete after empty bucket scan (#5453) 2026-07-29 21:06:28 +00:00
cxymds cc24ef173c fix(ecstore): defer delete cleanup for snapshot reads (#5408)
* feat(ecstore): add local snapshot leases

* feat(ecstore): add remote snapshot lease RPCs

* feat(ecstore): protect streaming GETs with snapshot leases

* fix(ecstore): defer version cleanup for snapshot reads

* fix(ecstore): cover batch snapshot cleanup safely

* fix(e2e): stub snapshot lease RPCs in lock mock

* fix(e2e): stub snapshot lease RPCs in lock mock

* fix(ecstore): bind deferred delete cleanup intents

* fix(rpc): keep snapshot lease checks CI-compatible
2026-07-29 22:23:01 +08:00
cxymds d9efd6b853 feat(ecstore): add remote snapshot lease RPCs (#5389)
* feat(ecstore): add local snapshot leases

* feat(ecstore): add remote snapshot lease RPCs

* fix(rpc): keep snapshot lease checks CI-compatible
2026-07-29 15:06:18 +08:00
Zhengchao An 90d1a15d13 fix(ecstore): classify peer RPC failures by gRPC status code (#5400) 2026-07-29 11:35:09 +08:00
cxymds 3d80578abd feat(ecstore): add local data-dir snapshot leases (#5388)
feat(ecstore): add local snapshot leases

Co-authored-by: houseme <housemecn@gmail.com>
2026-07-29 02:42:35 +00:00
cxymds 547c678eed fix(filemeta): reject positive size without parts (#5354)
* fix(filemeta): reject positive size without parts

* test(ecstore): keep optimized read fixture valid

* test(ecstore): keep listing fixtures valid
2026-07-28 17:35:53 +08:00
cxymds 7af92b4f54 fix(ecstore): handle pre-epoch disk health clocks (#5383) 2026-07-28 17:22:26 +08:00
cxymds b432f31c2c fix(multipart): preserve retried parts on quorum failure (#5363)
* fix(multipart): preserve retried parts on quorum failure

* style(multipart): format transaction rollback

* fix(proto): regenerate multipart transaction RPCs

* fix(multipart): import rollback marker constant

* fix(multipart): export transaction action

* fix: import multipart transaction test requests
2026-07-28 17:04:46 +08:00
harry han 03af8e472b fix(lifecycle): stop tier free-version recovery walk-timeout loop (#5194)
The background tier free-version recovery walk pinned a hardcoded 60s
total wall-clock timeout that overrides every operator knob, so any
bucket whose healthy full walk exceeds 60s fails forever; the failed
run's duration was also subtracted from the next 60s tick, restarting
the walk immediately and pinning CPU and disk I/O.

- Drop the total wall-clock budget on the recovery walk
  (walkdir_timeout: Duration::ZERO) and inherit the operator-tunable
  drive stall budget (RUSTFS_DRIVE_WALKDIR_STALL_TIMEOUT_SECS) for
  per-call progress, so hung disks still fail fast.
- Back off failed runs from completion time: 60s doubling to a 600s
  cap, reset on success; never subtract the failed run's duration.
- Add RUSTFS_TIER_FREE_VERSION_RECOVERY_ENABLED (default true) to opt
  out of the recovery worker on deployments with no remote tiers;
  invalid values warn and fail open.

Fixes #5130

Co-authored-by: claude <claude@ehdtn.com>
2026-07-26 02:54:33 +00:00
Henry Guo a63b79004c fix(scanner): make distributed usage convergence authoritative (#5151)
* fix(scanner): make distributed usage cycles authoritative

* fix(scanner): close distributed refresh races

* fix(config): align scanner reload integration

* fix(admin): scope config test helpers

* fix(scanner): harden distributed usage convergence

* fix(scanner): preserve rolling activity compatibility

* fix(admin): expose non-secret optional config values

* fix(scanner): acknowledge distributed dirty usage

* fix(ecstore): make bucket mutations cancellation safe

* fix(scanner): preserve pending dirty acknowledgements

* test(obs): account for superseded scanner metric

* fix(api): reject excess detached bucket mutations

* test: close scanner convergence coverage gaps

* fix(scanner): make path tracking cleanup one-shot

---------

Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com>
Co-authored-by: houseme <housemecn@gmail.com>
2026-07-25 18:45:16 +08:00
Henry Guo 9d1b10144f perf(ecstore): avoid redundant leaf object scans (#5176)
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com>
2026-07-24 22:25:03 +08:00
houseme 20c4ea864a fix(ecstore): fix wide-prefix listing stalls and decode downstream logging (#5198)
* fix(ecstore): handle list stall and downstream decode logs

* docs(ecstore): keep wide-directory stall context for list_dir
2026-07-24 14:05:27 +00:00
houseme a8e7cce5e1 feat: expose list read-dir amplification metrics (#5103)
Record local read_dir entry counts and duration for live-walker ListObjects scans so wide root/prefix amplification can be measured below the cross-set merge layer.

Add a focused LocalDisk scan_dir test showing a page limit of one still observes the whole parent directory enumeration, plus metric helper coverage.

Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-22 08:15:41 +08:00
houseme 7805cf5ae6 fix(cluster): clarify peer health and listing timeouts (#5086)
* fix(cluster): surface observed peer health

Refs rustfs/backlog#1387

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(admin): distinguish unreported peer health

Refs rustfs/backlog#1388

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): decouple metacache peek timeout

Refs rustfs/backlog#1389

Co-Authored-By: heihutu <heihutu@gmail.com>

* docs(ops): diagnose metacache listing timeouts

Refs rustfs/backlog#1390

Co-Authored-By: heihutu <heihutu@gmail.com>

* refactor(cluster): clarify observed peer health

Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): route capability state through contract

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-21 11:11:24 +00:00
houseme 26573622bc test(ecstore): cover post-apply rollback error (#5054)
Refs rustfs/backlog#1355

Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-20 17:05:53 +08:00
cxymds b0c6c4cbce fix(storage): resolve erasure parity per pool (#4977)
* fix(filemeta): add state-aware file info validation

* fix(filemeta): validate shard arithmetic and delete paths

* fix(ecstore): add fallible erasure construction

* fix(ecstore): resolve storage parity per pool

* fix(storage): report heterogeneous erasure layouts

* fix(admin): publish prepared storage config atomically

* fix(storage): harden per-pool parity boundaries

* fix(storage): address pre-PR validation findings

* test(ci): fix strict-topology validation fixtures

* fix(heal): preserve delete markers during repair

* refactor(filemeta): drop unused ValidatedFileInfo witness

ValidatedFileInfo wrapped an unread `_file_info` reference alongside an `Option<ValidatedErasureLayout>`, but only the layout was ever consumed. Return the layout directly from `FileInfo::validate` so the sole production consumer (`LocalDisk::check_parts`) and the two unit tests read it without the extra witness type and lifetime.

No behavior change.

* fix(filemeta): keep compressed and MinIO-migrated tiered objects readable

The new decode-path validation rejected several legitimate on-disk shapes that older RustFS and MinIO-migrated data carry, turning readable objects into FileCorrupt:

- Compressed objects written with an unknown upload size persist a negative per-part actual_size (the documented "unknown size" sentinel that ObjectInfo::get_actual_size already tolerates). validate_collection_contents rejected it via usize::try_from; now a negative actual_size skips shard validation and only real, non-negative sizes are checked.
- MinIO-migrated objects transitioned to a versioned remote tier store the tier version id as a UUID string, not 16 raw bytes. MetaObject::into_fileinfo returned FileCorrupt (main tolerated it as None), making all versions of the object unreadable; MetaDeleteMarker free-version records took a Some(nil) sentinel path with the same effect, which also breaks free-version expiry (remote-tier leak). Both now decode through a shared transitioned_version_id_from_meta_sys helper: 16 raw bytes or a UUID string are accepted, anything else is tolerated as None instead of failing the read.

Regression tests updated to assert the readable/compat behavior, with new tests covering MinIO string-form recovery.

* fix(scanner): build the delete-marker test fixture without erasure geometry

get_size_counts_delete_markers_separately_from_versions built its delete marker with `FileInfo::new(object, 1, 1)`, which attaches erasure geometry (data=1/parity=1/distribution). This PR classifies versions by shape via `is_storage_delete_marker()` (no geometry) rather than the raw `deleted` flag, so a geometry-bearing "delete marker" is correctly serialized as a purge-pending payload Object and counted as a version — CI saw summary.versions=3, expected 2.

Real delete markers carry no erasure geometry (delete paths build them as `FileInfo { deleted: true, ..Default::default() }`), so construct the fixture the same way. It then classifies as a storage delete marker and the counts (versions=2, delete_markers=1) hold. This keeps the PR's more-correct classification, which prevents a purge-pending object's geometry from being dropped when serialized as a bare delete marker.

* docs(changelog): note per-pool parity fix and storage-class startup upgrade caveat

Records the #4801 per-pool erasure parity fix under Fixed, and documents the upgrade behavior where a persisted storage class that a small or heterogeneous pool cannot satisfy now fails startup — with the RUSTFS_STORAGE_CLASS_STANDARD recovery steps. Docs-only; covers R4 from the on-disk compatibility audit.

* fix(heal): report parity from erasure geometry, not is_valid()

heal_object set HealResultItem.parity_blocks via `if lfi.is_valid()`, which was missed by the migration of the other quorum/metadata predicates. With the new `is_valid()` semantics (full payload validation; delete markers now return false), a delete marker or a geometry-bearing version with a benign collection quirk would misreport parity as the pool default instead of its own. Use `has_valid_erasure_geometry()` — the narrow "does this carry erasure geometry" predicate the rest of the migration uses — so reporting matches the object's actual layout. Reporting-only; no data-path change.

* fix(filemeta): do not silently serialize a non-canonical deleted FileInfo as an Object

`From<FileInfo> for FileMetaVersion` classifies by `is_storage_delete_marker()` (shape), which correctly routes canonical delete markers to Delete and purge-pending payloads (deleted=true with real erasure geometry) to Object. But a `deleted` FileInfo that is neither a canonical marker nor a valid erasure payload would silently serialize as a zero-geometry MetaObject that later fails `validate_for_metadata_read`. Write paths validate first (`validate_for_erasure_write` / `validate_for_metadata_read`), so this is a caller bug; `From` is infallible, so surface it with a structured `warn!` on the malformed branch instead of writing corrupt metadata silently. Legitimate purge-pending objects (valid geometry) are unaffected — the guard only fires for `deleted && !has_valid_erasure_geometry()`.

* test(filemeta): assert real historical xl.meta versions pass metadata-read validation

Empirical companion to the code-reasoned decode-tolerance invariants (docs/architecture/erasure-coding.md §11) and the rolling-upgrade / MinIO-migration compatibility concern: the tightened `validate_for_metadata_read` runs on every local disk read and peer-RPC-decoded FileInfo, so it must accept every version of real historically-written xl.meta, never reject it as FileCorrupt.

Loads five real fixtures — MinIO small-inline, MinIO versioned (two object versions + a delete marker), MinIO large multipart, a legacy V1 (xl.json-derived) object, and a legacy meta_ver 2 object — decodes every version with parts materialized, and asserts validate_for_metadata_read() is Ok for each. Reverting the tolerant handling (delete-marker shape, legacy per-part checksums, string/short transitioned-versionID, negative actual_size) turns this red.

* fix(ci): remove duplicate storage test re-exports

---------

Co-authored-by: overtrue <anzhengchao@gmail.com>
2026-07-19 21:52:31 +08:00
Zhengchao An 7f5873dac8 fix(ecstore): resolve erasure parity per pool (#4801) (#5015)
* fix(ecstore): add fallible erasure construction

(cherry picked from commit bd148b20f7)

* fix(ecstore): resolve storage parity per pool

(cherry picked from commit c05c2cb24b)

* fix(ecstore): keep carved per-pool parity core self-contained on main

Fixups so the cherry-picked fallible-erasure + per-pool-parity core builds standalone on current main without the excluded scope-creep commits:
- runtime/sources.rs: re-add backend_storage_class_parities (removed by the per-pool commit; its rebalance caller was updated in an unrelated reporting commit that was left out). Reimplemented over the snapshot API, behavior-identical.
- config/mod.rs: rename the storage-class publish test module (main independently added a mod tests, so the cherry-pick collided).
- rustfs storage_api.rs + startup_storage.rs: route the storage-class ENV consts through the startup storage facade and use a local const for the erasure-set-drive-count env name, satisfying the layer/facade guardrail (main's guardrail is stricter than when the core was authored).

* fix(ecstore): use struct-init in erasure test helper to satisfy clippy field_reassign_with_default

The cherry-picked fallible-erasure commit's `erasure_with_invalid_dimensions` test helper built `Erasure` via `default()` then reassigned fields, which trips `clippy::field_reassign_with_default` under `-D warnings` (only surfaced by `--all-targets`, which lints test code). #4977 fixed this in a later commit that was not part of the carved core. Use struct-init with `..Default::default()`, matching #4977's final form.

* fix(rustfs): gate the test-only storage-class ENV facade re-export behind cfg(test)

The ENV constants (INLINE_BLOCK_ENV/OPTIMIZE_ENV/RRS_ENV/STANDARD_ENV) re-exported through the startup storage facade are only consumed by a #[cfg(test)] test in startup_storage.rs, so in a non-test lib build the re-export is unused and trips -D unused-imports under clippy --all-targets. Gate it with #[cfg(test)], matching #4977's final form.

---------

Co-authored-by: cxymds <cxymds@gmail.com>
2026-07-19 03:06:46 +00:00
Zhengchao An 569fa3ec87 fix(ecstore): tolerate illumos/Solaris EEXIST for non-empty directory removal (#4995)
POSIX lets rmdir report a non-empty directory as either ENOTEMPTY or EEXIST. Linux/macOS/Windows use ENOTEMPTY (ErrorKind::DirectoryNotEmpty); illumos/Solaris return EEXIST (errno 17), which Rust surfaces as ErrorKind::AlreadyExists and which a DirectoryNotEmpty match never catches.

LocalDisk::delete_file removes xl.meta and then recurses to rmdir the object directory, tolerating only NotFound and DirectoryNotEmpty. Since #4300 (transactional delete rollback-staging, new in beta9) the object directory still holds the rollback backup dir when that rmdir runs — the caller removes it only after write quorum is confirmed — so the rmdir legitimately reports "not empty". On Linux that is tolerated; on Solaris it is EEXIST, which fell through to the catch-all arm and became FileAccessDeniedWithContext. That failed the delete commit, rolled the metadata back, and left the object undeletable, so the client retried indefinitely with a spurious FileAccessDenied and no EACCES anywhere (rustfs/rustfs#4978). The same Linux-errno assumption also broke non-force DeleteBucket on a populated bucket on Solaris.

Add a portable is_dir_not_empty_error classifier (DirectoryNotEmpty kind plus raw ENOTEMPTY/EEXIST), mirroring MinIO's isSysErrNotEmpty, and use it at the two directory-removal sites via is_benign_object_rmdir_error (delete_file) and classify_delete_volume_error (delete_volume). The classifier is applied only at rmdir/remove_dir_all sites, where EEXIST unambiguously means "not empty", so EEXIST keeps its normal meaning everywhere else. rmdir never returns EEXIST on Linux/macOS/Windows, so the new raw-errno branch is unreachable there and the change is a strict no-op off illumos/Solaris.

Adds unit tests for the classifier (DirectoryNotEmpty/ENOTEMPTY/EEXIST match, EACCES/ENOENT reject, real non-empty rmdir against the host errno) and call-site decision tests that make a Solaris EEXIST regression detectable on Linux CI.

Fixes #4978
2026-07-18 02:37:43 +00:00
Zhengchao An 814682d6bb fix(ecstore): accept illumos non-empty rmdir errors (#4991) 2026-07-18 06:01:25 +08:00
Zhengchao An 3674f5f56e fix(ecstore): bound remote shard writers with a progress deadline so one black-hole peer cannot pin write quorum (#4925)
A PUT that fans out erasure shards to remote peers awaited every shard writer to completion on both the per-block write and the final shutdown, and the remote HttpWriter had no progress deadline. A peer that accepts the TCP connection but never drains the request body (or never sends a response) therefore wedges the writer forever once the bounded buffers fill, pinning an otherwise-healthy write quorum indefinitely — a cluster-level write-availability hazard triggered by a single bad peer (rustfs/backlog#1319, https://github.com/rustfs/backlog/issues/1319).

MultiWriter now wraps each shard write and each shard-writer shutdown in a forward-progress deadline. The budget is re-armed on every block, so it bounds a stall rather than the total transfer time of a large object: a slow-but-honest writer that keeps completing shards is never killed, while a writer that makes no progress within the budget is failed and its disk dropped before commit. An optional absolute per-object cap (disabled by default) backstops a slow-drip peer that dribbles just enough progress to reset the per-block timer without ever converging; it is off by default so a legitimate large upload over a slow link is not killed on total time alone. Both knobs come from RUSTFS_OBJECT_DISK_WRITE_STALL_TIMEOUT (default 30s) and RUSTFS_OBJECT_DISK_WRITE_ABSOLUTE_CAP (default 0 = disabled); setting the stall timeout to 0 restores the previous wait-forever behavior for a conservative rollback.

The deadline enforcement lives in MultiWriter (writer-agnostic), so it covers local and remote writers alike and keeps the existing control-flow shape: a timed-out shard is marked failed (Error::Timeout, which is not an ignored error) and excluded from the write quorum exactly like any other shard write failure, and the unchanged nil_count/quorum check then continues on quorum or fails cleanly. This deliberately stays out of the MultiWriter lifecycle / commit-coordinator territory owned by rustfs/backlog#1312.

When a stalled writer is dropped to fail its shard, the remote HttpWriter must stop holding the connection and its buffered body. HttpWriter previously left its spawned request task running on drop; it now aborts that background task in Drop (it is no longer pin-projected, since every field is Unpin and the AsyncWrite impl already used get_mut). Bytes already handed to the transport cannot be unsent, but they land only in this upload's unique tmp path and are reclaimed by tmp GC — they never touch a committed object.

Tests, all on a paused virtual clock so they are deterministic and non-flaky:
- one black-hole writer still meets a 3/4 write quorum without hanging; two black holes fail the quorum cleanly (both for the per-block write and the shutdown paths).
- a slow-but-honest writer that keeps making progress within the stall budget is never failed across many blocks.
- the absolute cap bounds a slow-drip writer within a finite budget while the healthy writers keep quorum.
- the default policy is armed by default and honors 0 as disabled.
- HttpWriter aborts its background request task on drop against a hanging peer.

The toxiproxy/black-hole 4x4 end-to-end acceptance depends on black-box test facilities from rustfs/backlog#1325, which are not built yet; that acceptance is deferred to #1325 and intentionally not faked here.
2026-07-16 16:41:38 +00:00
houseme 1badff3923 fix: resolve moved-value diagnostics (#4905)
Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-16 18:46:56 +08:00
Zhengchao An f78f146c35 test(ecstore): extend crash-point injection to multipart complete and xl.meta update paths (#4853) 2026-07-15 16:10:14 +08:00
cxymds 7ece747eab fix(ecstore): suppress missing rollback rename warnings (#4792) 2026-07-13 19:05:09 +08:00
Zhengchao An 6096bb189d fix(ecstore): demote reliable_rename NotFound WARN to debug (#4789) 2026-07-13 12:07:49 +08:00
houseme 1553dc3f62 Address P2 follow-ups from the 2026-07-10..12 merged-PR review (backlog#1210-1220) (#4783)
* fix(obs): open cleaner compression source with O_NOFOLLOW

The compressor opened the source log via File::open, which follows a
symlink at the final path component. Between the scanner selecting a
regular file and this open, an attacker with write access to the log
directory could swap the entry for a symlink (TOCTOU) pointing at, say,
/etc/shadow, whose contents would then be copied into an archive. Open
the source with O_NOFOLLOW on Unix so such a swap fails with ELOOP; the
temp/archive path already refused symlinks, this closes the source side.

Refs rustfs/backlog#1210
Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(obs): recompress instead of trusting leftover cleaner archives

archive_header_ok only checked the first 2-4 magic bytes before treating
an existing .gz/.zst as a completed prior result and letting the caller
delete the source log. A file with valid magic but a truncated or forged
body passes that check, so an attacker with write access to the log
directory (or a crashed prior run) could plant such a stub and make the
cleaner delete the real log without ever producing a usable archive —
silent audit-data loss.

Chosen fix: stop trusting cross-process leftovers entirely and always
recompress the source in this pass, rather than fully decoding every
leftover to validate it. Full-decode validation would add real CPU cost
and decode-bug surface for a rare crash-recovery case; the existing
atomic create_new+rename already overwrites whatever sits at the archive
path (a planted symlink is replaced, never followed) with a freshly
written, fsync'd archive, so a partial/forged leftover can never gate
source deletion. This is the lowest-regression option.

Refs rustfs/backlog#1211
Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(object-data-cache): cap memory-gate reservation at cache growth headroom

The memory gate subtracts `admitted_since_refresh` from the snapshot's
available bytes so a burst arriving faster than the 5 s refresh cannot
over-allocate. That counter is GROSS: it only rolls over on the refresh and
never rolls back when a fill is later evicted, cancelled, or loses the
invalidation race. Under sustained high-throughput churn (net footprint flat
and far below `max_capacity`) the raw counter balloons past the memory the
cache actually holds, so `effective_available` collapses and the gate reports
false memory pressure — skipping the hottest fills with SkippedMemoryPressure
until the next 5 s refresh. This only lowers hit rate; it never returns wrong
data and self-heals each refresh.

Fix direction 1 (minimal regression): cap the reservation deduction at the
cache's own growth headroom (`max_capacity - weighted_size()`) instead of
letting the unbounded gross counter shrink the system-available budget. The
cache can never hold more than `max_capacity`, so a burst adds at most that
headroom of real memory before moka evicts to stay bounded (net-zero churn
beyond that point) — capping the deduction there keeps the reservation honest
without treating gross churn as growth. Chosen over net-accounting (direction
2, releasing bytes on every failure/cancel/eviction path) because that only
plugs the leak on failed fills and would not address the core defect: churn of
*successful* insert/evict fills over the 5 s window. It also touches only the
gate plus one call site rather than every failure path in moka_backend.

The cap only ever raises `effective_available`, so real memory pressure (a low
snapshot at refresh) still suppresses fills; when the cache is at capacity the
headroom is 0 and the deduction vanishes, correctly reflecting net-zero churn.
`MokaBackend` now stores `max_capacity` and passes the live headroom into
`allows_fill`. Adds targeted gate tests: gross churn far above headroom no
longer falsely suppresses, yet the reservation still bounds a burst while the
cache can genuinely grow.

Refs rustfs/backlog#1212
Co-Authored-By: heihutu <heihutu@gmail.com>

* test(ecstore): assert native O_DIRECT path runs in uring read test

uring_preserves_o_direct_for_eligible_reads only compared bytes through
LocalDisk::read_file_mmap_copy. On a filesystem that rejects O_DIRECT the
read silently degrades to the buffered StdBackend fallback and the byte
check still passes, so the test could go green without the native
read_at_direct path ever executing -- a vacuous pass.

Add a per-disk native_direct_reads counter on UringBackend, incremented
only when pread_uring_direct completes, and rebuild the test to drive a
real UringBackend's pread_bytes and assert the counter is non-zero (every
eligible read went through the native tier). When io_uring or O_DIRECT is
unavailable on the host filesystem (restricted CI runners, tmpfs), the
test skips loudly via eprintln instead of asserting a tautology, while
still checking byte-correctness on whatever tier served the read.

The counter also gives a gray release a positive signal that the O_DIRECT
tier is serving reads, not just a fallback count.

Refs rustfs/backlog#1213
Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(ecstore): warn + count read-time EINVAL on native O_DIRECT reads

classify_direct_read_error is only reached from the read side: the
O_DIRECT open in pread_uring_direct already succeeded (an open-time
refusal is handled earlier as DirectOpenError::ODirectRefused). So an
EINVAL/EOPNOTSUPP arriving here is a read-time error on an fd the kernel
accepted for O_DIRECT -- far more likely an alignment bug in the aligned
read path than an unsupported filesystem. The old code latched the disk's
native path off with only a once-per-disk debug trace, hiding a potential
correctness regression behind a silent buffered-read downgrade.

Diagnostics only: the fallback behaviour is unchanged (the native path is
still latched off and the caller still reads via StdBackend). This adds a
rustfs_io_uring_direct_read_einval_total counter and promotes the
once-per-disk trace from debug to warn so an operator can see an alignment
regression instead of an unexplained latency/CPU shift.

Refs rustfs/backlog#1214
Co-Authored-By: heihutu <heihutu@gmail.com>

* docs(ecstore): document data-blocks-first default and its tail-latency cost

DEFAULT_RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP is true and must stay
true: deferred-parity is the deliberate, already-rolled-out full-object
GET default from backlog#1159/#923. Flipping it back to false in code
would silently revert that rollout for every deployment that has not set
the env var, so this commit only documents -- no behaviour change.

The added notes explain what data-blocks-first does (schedule data shards
up front, engage parity lazily on a missing/corrupt data shard), the known
trade-off (parity is engaged late, so a slow-but-not-dead data drive
raises GET p99 because the faster parity shards are not raced against it
until a data shard is declared missing), and the operational rollback
switch (RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP=false), which is
intentionally an env override rather than a code default change.

No metric was added: the low-risk observability hook for "slow data drive
engaged deferred parity" would live at the deferred-stripe engage point,
which is out of this file's scope; this change stays documentation-only to
avoid touching the hot GET path.

Refs rustfs/backlog#1215
Co-Authored-By: heihutu <heihutu@gmail.com>

* docs(ecstore): document wide-directory walk stall hazard and tuning

list_dir enumerates a whole directory in one os::read_dir call (count =
-1), and the walk caller bounds that entire enumeration with the per-read
stall budget (default 5s) as if it were a single read. For a wide, flat
prefix -- one directory holding millions of immediate children -- a single
readdir can exceed the budget on a healthy disk, trip DiskError::Timeout,
and surface as a ListObjects 500 quorum failure though the drive is fine
(a #2999 sub-class).

This is documented, not rewritten: turning the one-shot readdir into a
streaming/batched enumeration that refreshes the stall deadline between
chunks is an architecture-level change with high regression surface
(ordering, the count contract, quorum merge) and belongs in a separate
follow-up. The supported mitigation today is operational, so the comments
point wide-directory deployments at RUSTFS_DRIVE_WALKDIR_STALL_TIMEOUT_SECS
and the high-latency drive-timeout profile, which widen the budget with no
code change. Notes were added at list_dir, the scan_dir call site, and
get_drive_walkdir_stall_timeout. No behaviour change.

Refs rustfs/backlog#1216
Co-Authored-By: heihutu <heihutu@gmail.com>

* docs(ecstore): document consumer-peek vs producer-stall coupling

In list_path_raw the consumer's peek_timeout is drawn from the same source
and same value (walkdir_stall_timeout, default 5s) as the producer-side
walk stall budget, but the two measure different things: the producer
stall bounds a single drive read, while the consumer peek bounds the gap
between two ADJACENT entries arriving from a reader. Because they share a
value, the consumer cannot wait meaningfully longer for the next entry
than the producer is allowed to spend producing one. Walking a region
dense with non-listable internal items can make a HEALTHY drive miss the
budget between visible entries; the consumer then declares it stalled and
detaches it, dropping a good drive from the merge and capping the "large
prefix succeeds" guarantee.

Documented, not decoupled: giving the consumer peek an independent,
strictly-larger budget would cut these false detaches but equally delays
detaching a genuinely dead drive and shifts listing tail-latency
semantics, so it wants soak data before changing the default. The comment
records the invariant any such follow-up must keep -- consumer peek >=
producer stall, never stricter -- so it can never fail a drive before the
producer would. No behaviour change.

Refs rustfs/backlog#1217
Co-Authored-By: heihutu <heihutu@gmail.com>

* fix(io-metrics): add time-based trigger for low-IOPS latency percentiles

Percentiles were recomputed only every 128 IOs and seeded to 0, so a
low-traffic deployment exported p95/p99 = 0/stale for a long time after
startup. Add a 10s wall-clock trigger alongside the count throttle so the
first recompute can fire before 128 samples accrue. Hot-path per-op mean
update is unchanged.

Refs rustfs/backlog#1218
Co-Authored-By: heihutu <heihutu@gmail.com>

* test(e2e): cover codec-streaming parity under fault injection and NoSuchKey

The codec-streaming compat A/B previously ran only against a healthy
4-disk EC set with successful full GETs: the DiskFaultHarness was
constructed but never faulted, the error path was untested, and the
range assertion silently compared legacy-vs-legacy (ranges always fall
back to the duplex path), overstating what it proved.

Add two genuinely-failable scenarios reusing the existing harness and
fixtures:

- Parity reconstruction A/B: take one data disk offline and re-run the
  full object matrix on both phases while the EC 2+2 set rebuilds each
  large object from the surviving shards. Assert codec == legacy
  byte-for-byte (sha256) and header-for-header, and assert the codec
  phase served the reconstructed objects with zero duplex-pipe fallback
  (the reader gate is drive-health-independent, so the codec fast path
  is really exercised through reconstruction).
- NoSuchKey negative path: compare the HTTP status + S3 error code of a
  missing-key GET across the legacy and codec phases and require them to
  be identical (404/NoSuchKey), guarding against the codec env
  perturbing the error path.

Also clarify the range-phase comment so it is not misread as
codec-range correctness coverage: both sides are served by the same
legacy range path, so the assertion only proves ranges keep working and
keep falling back to legacy with the gates open.

Verified: cargo check/--no-run pass and the test passes locally
(1 passed; dup_codec=0 confirms the codec path ran).

Refs rustfs/backlog#1219
Co-Authored-By: heihutu <heihutu@gmail.com>

* ci(ecstore): exercise native O_DIRECT read path on an ext4 loopback

The uring-integration leg ran on the runner's default TMPDIR, which may sit
on tmpfs/overlayfs where open(O_DIRECT) fails and the native read_at_direct
path silently latches off to the aligned StdBackend fallback. Mount a
dedicated ext4 loopback and point TMPDIR at it so the real io_uring dep
(bumped git->0.1.0->0.2.0->0.2.1) and the native O_DIRECT read path are
actually covered rather than validated only by signature diffing.

Refs rustfs/backlog#1220
Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-12 16:03:28 +00:00
Zhengchao An 418f505a81 fix(ecstore): allow concurrent directory scans (#4778) 2026-07-12 13:17:50 +00:00
houseme 13bdca6762 build(toolchain): switch Rust channel to stable (#4775)
* Change Rust toolchain channel to stable

Signed-off-by: houseme <housemecn@gmail.com>

* style: apply clippy --fix and cargo fix lint suggestions

Run `cargo clippy --fix --all-targets --all-features` and
`cargo fix --lib --all-targets` across the workspace, then resolve the
remaining warnings by hand:

- collapse needless borrows in `format!` args, prefer `?` over explicit
  early returns, and use `.values()` / `.flatten()` iterator adapters
- rewrite the `Md5` scan loop via `manual_flatten` and re-indent the
  `select!` macro body (rustfmt skips macro interiors)
- annotate the intentional dead-code `Md5` inherent methods (constructed
  only by the test factory) with `#[allow(dead_code)]`

Behavior is unchanged.

Co-Authored-By: heihutu <heihutu@gmail.com>

---------

Signed-off-by: houseme <housemecn@gmail.com>
Co-authored-by: heihutu <heihutu@gmail.com>
2026-07-12 18:59:43 +08:00
cxymds d8e69a3adf fix(logging): enforce single-writer sinks and bound tracing (#4765)
* fix(obs): prevent rolling log stdout aliasing

* fix(ecstore): bound hot-path tracing payloads

* test(logging): guard service and disk log invariants

* fix(obs): silence useless_conversion on st_dev for Linux clippy

rustix's Stat.st_dev is u64 on Linux/glibc, making u64::try_from a no-op
that trips clippy::useless_conversion under -D warnings. The conversion is
still needed on macOS/BSD where st_dev is a signed dev_t, so suppress the
lint on that line rather than dropping the portable fallible conversion.

* fix(logging): keep stdout sink validation portable (#4769)

* fix(obs): keep stdout device conversion portable

* docs(logging): update single-writer plan status

---------

Co-authored-by: overtrue <anzhengchao@gmail.com>
Co-authored-by: houseme <housemecn@gmail.com>
2026-07-12 16:03:01 +08:00
houseme 2ddafb4ed9 test(ecstore): bound file sync probe waits (#4767)
Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
2026-07-12 16:01:09 +08:00
Zhengchao An a5765274fc fix: auto-repair test_rename_data_shares_file_sync_limit hang on macOS (#4758)
fix(test): use canonicalized disk root for file_sync_probe in rename_data test

On macOS, tempfile::tempdir returns /var/folders/... while LocalDisk
resolves the root to /private/var/folders/... via dunce::canonicalize.
The file_sync_probe::enter() path check uses starts_with(), so passing
the non-canonical tempdir path caused the probe to never activate,
making wait_for_active() hang indefinitely.

Use disk.root (already canonicalized) for the probe instead.
2026-07-12 06:01:39 +08:00
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>
2026-07-12 01:36:24 +08:00
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>
2026-07-11 17:25:06 +00:00
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>
2026-07-11 15:27:15 +00:00
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>
2026-07-11 13:50:48 +00:00