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.
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
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>
* 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>
* fix(core-storage): fix critical correctness defects from core-storage audit
Fixes verified defects found in a deep audit of the core storage path
(erasure coding, disk persistence, quorum, heal, replication resync):
- ecstore/disk: rewrite live xl.meta atomically (temp+rename) in
delete_versions_internal and write_metadata instead of in-place
truncate, which exposed torn metadata to concurrent readers and
crashes on the DeleteObjects hot path
- ecstore/erasure: allow heal to reconstruct from exactly data_shards
bitrot-verified sources; requiring data_shards+1 made objects
permanently unhealable after losing parity_shards disks
- ecstore/set_disk: direct-memory inline GET applied the erasure
distribution permutation twice (shuffled inputs re-indexed through
distribution), concatenating wrong shards into the response body in
degraded reads; collect from canonical disk-ordered inputs
- ecstore/set_disk: heal now preserves the committed inline layout
instead of recomputing it with a hardcoded unversioned threshold,
which split quorum identity of healed replicas and caused endless
re-heal churn
- ecstore/replication: resync results channel switched from
broadcast(1) to mpsc; a lagged broadcast receiver ended the stats
collector and every subsequent failure went uncounted, letting
failed resyncs be marked completed
- ecstore/replication: ignore an empty persisted resync checkpoint;
resuming with one skipped every object and marked the resync
completed without replicating anything
- ecstore/replication: fix inverted not-found error classification in
replicate_object/replicate_delete logging paths
- ecstore/erasure: guard decode paths against zero block_size or
data_shards from corrupt on-disk metadata (divide-by-zero panic)
- ecstore/disk: os::read_dir no longer consumes the entry limit on
entries it does not return (is_empty_dir misjudgment); create_file
opens with O_TRUNC to avoid stale trailing bytes
- filemeta: treat Some(nil) version id as a null version in
matches_not_strict; disk-loaded headers never store None, so the
mod_time quorum guard for unversioned overwrites never fired and an
interrupted overwrite could displace the committed version in merge
- filemeta: fix msgpack skip lengths for fixext (missed the ext type
byte) and ext16/32 (over-skipped) unknown fields
- filemeta: return FileCorrupt instead of usize underflow when
xl.meta is truncated inside the CRC trailer
- filemeta: surface delete-marker insertion failure in delete_version
instead of reporting success when the data dir is shared
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(replication): drop duplicate cfg(test) etag import from boundary module
The test module already imports content_matches_by_etag locally, so the
top-level cfg(test) import is unused under -D warnings and fails clippy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(http): reduce header timeout log noise
Log peer_addr for HTTP connection errors so idle or probe traffic can be traced back to the source.
Downgrade HeaderTimeout connection logs from warn to info because these timeouts are often caused by incomplete probe traffic rather than a service-side fault.
* fix(ecstore): suppress spurious warn logs for missing trash cleanup sources
Extract `reliable_rename_inner` with a `warn_on_failure` flag so that
`rename_all_ignore_missing_source` can skip the warn log when the source
is absent. This completes the idempotent cleanup fix — the previous
implementation still emitted a `reliable_rename failed` warning before
the NotFound was caught and silenced.
Also restore the `recursive` branching in `move_to_trash` so that
non-recursive deletions use `fs::rename` directly (preserving original
semantics), while recursive deletions use the idempotent helper.
* perf: reduce spawn_blocking contention in PUT path (~23% throughput gain)
Flame graph profiling identified tokio blocking pool mutex contention
as the #1 bottleneck (17.3% of CPU time). Each spawn_blocking call must
acquire parking_lot::raw_mutex to enqueue work. With 16 concurrent PUTs
× 4 disks × 3+ spawn_blocking per disk, this became a serialization point.
Optimizations applied:
- Merge make_dir_all + file write into single spawn_blocking
- Merge read_file + parse + write + rename into single spawn_blocking
for inline objects (small files)
- Optimize reliable_rename to try rename first, mkdir only on ENOENT
- Optimize remove/remove_std to try remove_file first, EISDIR fallback
- Add encode_inline_small fast path for small objects
- Parallelize bitrot writer creation with join_all
Benchmark (4KiB PUT, 4-disk EC, 16 concurrent, 8 rounds, randomized A/B):
Baseline: ~950 obj/s → Optimized: ~1173 obj/s (+23%)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: address review comments
- Return old_data_dir for non-inline rename_data path (was incorrectly None)
- Restore delete_all cleanup of PUT temp data on failure paths
- Fix cargo fmt formatting
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* style: apply rustfmt from stable 1.96.0
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: collapse nested if-let chains for clippy compliance
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: address Copilot review comments
- fs.rs: handle macOS EPERM from remove_file on directories
- os.rs: restore NotFound=Ok(()) semantics on first rename attempt
- local.rs: use try-rename-then-mkdir pattern for inline rename_data
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* test: add unit tests for encode_inline_small fast path
* test: fix comment and line length in encode_inline_small tests
* fix: revert reliable_rename and write_all_internal to match original
Restore the original reliable_rename logic (check parent exists, then
rename in loop) and the original write_all_internal (make_dir_all outside
spawn_blocking). The optimization changes caused a CI-only test failure
in capacity_dirty_scope_test that could not be reproduced locally.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: update comment and fix formatting for CI
- Fix macOS/BSD comment to accurately say macOS only
- Fix encode_inline_small test formatting to match rustfmt 1.96.0
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix: propagate inline rename errors
* fix: retry rename when parent missing
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This commit introduces a significant reorganization of the project structure to improve maintainability and clarity.
Key changes include:
- Adjusted the directory layout for a more logical module organization.
- Removed unused crate dependencies, reducing the overall project size and potentially speeding up build times.
- Updated import paths and configuration files to reflect the structural changes.
Zhengchao An