mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-05 21:07:43 +00:00
65953bfdb3d98c570f6b664b166380867450803a
839 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Zhengchao An
|
65953bfdb3 | fix(ecstore): reduce rename data version signatures (#4383) | ||
|
Zhengchao An
|
ddf197ba57 |
fix(lock): refresh object write locks with a heartbeat (#4388)
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). |
||
|
houseme
|
e7cc719c17 |
perf(ecstore): move speculative PUT-tail tmp cleanup off the hot path (#4389)
* 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> |
||
|
houseme
|
062a68d151 |
perf(ecstore): skip tmp parent dir fsync for write-then-rename paths (#4387)
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> |
||
|
houseme
|
9ae4ca5f99 |
test(ecstore): fit concurrent-resend guard inside lock acquire ceiling (#4384)
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> |
||
|
Zhengchao An
|
7efacbdf95 |
fix(filemeta): validate part array lengths in into_fileinfo (#4382)
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). |
||
|
Zhengchao An
|
a91d9cefc6 |
test(interop): add real MinIO read and migration parity tests (#4377)
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 |
||
|
Zhengchao An
|
62a31e4ec4 | fix(storage): address pending metadata and health gaps (#4380) | ||
|
Zhengchao An
|
742a59884d | test(ecstore): add validation suite coverage gates (#4378) | ||
|
Zhengchao An
|
3ed414cdb4 | fix(storage): complete pending metadata and quorum fixes (#4375) | ||
|
houseme
|
5533e080b0 |
feat(ecstore): LocalIoBackend trait and O_DIRECT read with fallback (#4365)
* 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
|
8f4349793e |
fix(ecstore): log peer offline/online transitions for console reporting (#4367)
A node could show OFFLINE in the console with zero logs explaining why (rustfs/backlog#888, reported in rustfs#4304): the consecutive-failure threshold crossing in handle_server_info_failure marked the peer offline silently, the per-call warnings only existed on the observing node and never named the transition, and the PeerRestClient offline flag plus its background recovery monitor ran without any start/success log. Logging-only change, no behavior change: - handle_server_info_failure / handle_storage_info_failure: WARN event="peer_marked_offline" exactly once at the threshold crossing (later failures while already offline stay DEBUG), and DEBUG event="peer_probe_failure" while returning cached state below the threshold. - update_server_info_cache / update_storage_info_cache: INFO event="peer_recovered_online" when a probe succeeds after the peer had been reported offline. - PeerRestClient::mark_offline_and_spawn_recovery: WARN event="peer_connection_marked_offline" when the offline flag is first set (guarded by the recovery_running CAS so repeated failures do not spam), and INFO event="peer_connection_recovered" with the attempt count when connectivity is restored. An "offline then back" episode now leaves a complete, correlatable trace: N probe failures -> peer_marked_offline -> recovery monitor -> peer_recovered_online. Verification: - cargo test -p rustfs-ecstore --lib (notification + peer_rest suites) - make pre-commit Ref: rustfs/backlog#888, rustfs#4304 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
2dfa3d3c36 |
test(ecstore): restore 30s lock-timeout guard for concurrent resend test (#4370)
concurrent_resend_same_part_commits_one_generation keeps failing on CI with Lock(Timeout ... 5s) even after the fast-lock lost-wakeup fix (NOTIFY_WAIT_CAP re-polling) landed — observed on rustfs#4365's Test and Lint runs. Two independent causes exist and both are real: 1. the lost/stolen wakeup stall — fixed in fast_lock::shard; and 2. the six legitimately serialized cross-disk commits exceeding the small 5s default acquire timeout under full-suite CI disk load. The lost-wakeup fix reverted the earlier 30s test override on the assumption that cause 1 was the whole story; CI shows cause 2 stands on its own. Restore the temp_env override (production-default 30s) around the concurrent section so the regression guard asserts the correctness property (exactly one intact generation), not CI disk latency. The NOTIFY_WAIT_CAP fix stays untouched. Verification: - cargo test -p rustfs-ecstore --lib concurrent_resend_same_part_commits_one_generation Ref: rustfs/backlog#882 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
2247823200 |
fix(runtime): remove tokio io-uring feature and add regression guard (#4364)
Drop the [target.'cfg(target_os = "linux")'.dependencies] tokio "io-uring" feature from 6 crates and the rustfs binary. Because .cargo/config.toml enables --cfg tokio_unstable globally, this feature switched every Linux build's file I/O onto tokio's io_uring runtime backend. Restricted Linux environments (Docker default seccomp, gVisor, proot, old kernels) reject io_uring_setup with EACCES/ENOSYS, which tokio surfaced as PermissionDenied and RustFS reported as DiskAccessDenied at startup. Add scripts/check_no_tokio_io_uring.sh so the feature cannot silently return: it fails on any tokio dependency line enabling "io-uring", while still allowing a future application-level io-uring crate dependency. Wire it into make pre-commit/pre-pr/dev-check and CI. Tracking: rustfs/backlog#890 (parent rustfs/backlog#897) Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
717cdd2abd |
fix(migration): decrypt MinIO IAM & server config on drop-in migration (#4358)
* fix(migration): decrypt MinIO IAM & server config on drop-in migration MinIO encrypts IAM identity/service-account files and the server config at rest with a key derived from the root credentials. The drop-in migration paths read those blobs from the legacy `.minio.sys` bucket and parsed them as plaintext JSON, so any encrypted blob failed to parse and was silently skipped with "incompatible format". This is why users migrating from MinIO kept their buckets/objects/policies but lost users and access keys (#2212). The IAM load path already knows how to decrypt these blobs (RustFS master keys plus MinIO-compatible legacy keys derived from the root credentials), but that logic lived behind a private method and was never used by the migration paths. Expose it as `rustfs_iam::try_decrypt_iam_blob` and inject it into both migration paths via a `LegacyBlobDecryptFn` callback (ecstore cannot depend on the IAM crate, so the closure is wired in the binary crate). When a blob cannot be decrypted the raw bytes are used as-is, preserving the previous plaintext-only behavior with no regression. Also improve object-layer migration observability without changing control flow: `try_migrate_format` now distinguishes "no legacy format" (a normal fresh install) from "legacy format present but incompatible", and the caller logs a loud error before initializing a fresh format that would leave the existing MinIO objects unreadable. Topology/version skip reasons are promoted from debug to warn. Fixes a pre-existing test isolation race by marking `test_recovery_falls_back_to_default_config_when_blob_stays_corrupt` serial, since it reads a process-wide env var toggled by a sibling test. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(migration): box FormatV3 in LegacyFormatOutcome to satisfy clippy Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): stabilize concurrent multipart resend lock timeout concurrent_resend_same_part_commits_one_generation spawns 6 same-part resends whose cross-disk commits serialize on the per-uploadId commit lock. Under the full nextest suite the parallel disk load pushes those serialized commits past the small default lock-acquire timeout (5s), producing a spurious `Lock(Timeout ...)` unrelated to the property under test (observed on CI at 5.775s vs ~0.5s in isolation). Raise RUSTFS_OBJECT_LOCK_ACQUIRE_TIMEOUT to the production default (30s) for the concurrent-commit section via temp_env, so the regression guard reflects correctness (exactly one intact generation) rather than disk latency under CI load. The meaningful assertions are unchanged, and #[serial] keeps the process-wide env override isolated. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(lock): bound fast-lock notification wait to prevent lost-wakeup stall The real cause of the concurrent_resend_same_part_commits_one_generation failures was a lost wakeup in the fast-lock slow path, not disk latency: raising the acquire timeout to 30s only delayed the failure (it then timed out at 30s), proving a genuine stall rather than overload. In acquire_lock_slow_path a waiter that reaches the notification phase did a single `timeout(remaining, wait_for_write())` spanning the whole acquire budget, and treated that wait's elapse as a hard `Timeout`. But the release path only notifies when `writer_waiters > 0`, so if the holder releases in the gap after the waiter's `try_acquire` fails and before it registers as a waiter, no notification (and no stored permit, since the pooled `Notify` is gated) is produced. The waiter then blocks until the deadline even though the lock is free and stays free — a spurious lock-acquire timeout. The shared process-wide notify pool makes it worse: a wakeup can be consumed by a waiter of a different lock hashing to the same slot. Bound each notification wait (NOTIFY_WAIT_CAP = 50ms) and, on elapse, loop back and re-`try_acquire` instead of returning `Timeout`; the deadline check at the top of the loop is the single source of truth for timing out. A lost/stolen wakeup now degrades to bounded re-polling (acquire within ~50ms of the lock becoming free) instead of stalling for the whole timeout. Correctness (mutual exclusion) is unchanged — acquisition still only happens via `try_acquire_*`. Add a regression test that reproduces the stall (holder + late waiter across many keys): it times out without the fix and passes in ~1s with it. Revert the earlier acquire-timeout workaround in the multipart test now that the underlying stall is fixed, so it runs under the default timeout again. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
f73880f354 |
fix(startup): survive slow multi-node cold starts (#4357)
* fix(server): make startup readiness wait configurable and raise default (#4264) The startup runtime-readiness wait was a hardcoded 30s constant with no env override. On slow multi-node cold starts (Docker/K8s/Synology NAS) this window is shorter than the internal startup budgets it depends on — the endpoint DNS-retry window (~90s) and the format-load retry loop (~100s worst case) — so readiness times out and the node exits with `startup readiness timed out after 30s: storage_ready=false, lock_quorum_ready=false` before storage/lock quorum can converge, feeding the restart storm reported in the issue. - Add `RUSTFS_STARTUP_READINESS_MAX_WAIT_SECS` (default 120s), documented in rustfs-config health constants. - Resolve the wait at runtime via `startup_runtime_readiness_max_wait()`; a value of `0` falls back to the default instead of timing out instantly. - Repoint `STARTUP_RUNTIME_READINESS_MAX_WAIT` at the shared config default so there is a single source of truth, and cover the getter with unit tests. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): stop peer/disk background monitors on graceful shutdown (#4264) Long-lived peer health/recovery and remote-disk monitors are detached `tokio::spawn` tasks that each hold a `tracing::Span` via `.instrument(..)` for their whole lifetime. Nothing cancelled them at shutdown, so on the normal return path the Tokio runtime was dropped while they were still alive and their `Span`s were dropped during worker-thread thread-local-storage (TLS) destruction. At that point `tracing-subscriber`'s fmt `on_close` can touch an already-destroyed TLS slot and panic with `cannot access a Thread Local Storage value during or after destruction`, which escalates to a panic-during-panic abort (SIGILL / exit 132) — the crash reported on Synology in issue #4264, amplified by the restart storm. - Add `cluster::rpc::background_monitor` with a process-global shutdown token, `spawn_background_monitor()` (races the monitor future against that token so its span drops while the runtime is alive), and public `shutdown_background_monitors()`. - Route every span-holding peer_s3 / peer_rest / remote_disk monitor spawn through `spawn_background_monitor` instead of `tokio::spawn(..).instrument()`. - Expose `rustfs_ecstore::shutdown_background_monitors()` and call it from the graceful shutdown sequence (right after `ctx.cancel()`, before runtime teardown) via the `storage_api` compatibility boundary. Existing recovery-probe span-context tests still pass, confirming log correlation is preserved. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
58114f49f2 |
perf(ecstore): k-way heap merge for ListObjects, drop clone-to-parse (#4347)
* perf(ecstore): replace linear merge scan with k-way heap and drop clone-to-parse (backlog#874 backlog#875) merge_entry_channels advanced the k-way merge with a linear scan over all channel heads (O(entries x channels)) and allocated two fresh Strings per pairwise comparison via path::clean. Every step also cloned MetaCacheEntry values, including entry.clone().xl_meta() clone-to-parse calls. - Introduce MergeHead with a cached cleaned name (allocated only when the raw name is not already clean) and drive the merge with a BinaryHeap of boxed heads: O(log channels) per entry, allocation-free comparisons. - Move entries through the merge instead of cloning; the winner is sent without an intermediate copy. - Remove the dead merge_file_meta_versions block: it only ran for prefix-dir groups whose entries have empty metadata, so xl_meta() always failed; cross-drive version merging happens in the resolve path. - Keep legacy same-name semantics (dir groups collapse, objects shadow prefix dirs, later object candidate wins) and add regression tests for interleaved ordering, dir/object precedence, uncleaned-name grouping, and prefix-dir collapse. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): honor ascending versions_sort in ListObjects walk (#4348) * fix(ecstore): honor ascending versions_sort in walk and document ordering invariant (backlog#876) The walk loop carried a bare `//TODO: SORT` inside the `WalkVersionsSortOrder::Ascending` branch, so the requested ascending order was silently ignored and versions streamed newest-first (the raw FileMeta order). WalkOptions defaults to Ascending, so every default walker -- notably replication resync, which replays versions and needs oldest-first to preserve the version-stack order -- received the exact opposite of the contract. FileMeta maintains versions newest-first (sort_by_mod_time is descending) and into_file_info_versions preserves that order, so ascending emission is the exact reverse of file_info_versions output. Reverse in place when ascending is requested and add a regression test locking the newest-first invariant plus the reversal contract. Key-ordering audit result (no gap found): per-disk walkers emit sorted streams, merge_entry_channels performs an ordered k-way merge, and gather_results only filters by marker/limit, so ListObjects key order is guaranteed upstream and needs no post-sort. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): enable GET metadata early-stop by default (#4349) * perf(ecstore): enable GET metadata early-stop by default with env opt-out (backlog#872) The metadata early-stop fanout (read_all_fileinfo_early_stop) has been implemented and instrumented for a while but stayed behind an opt-in flag, so default GETs always waited for every disk to answer the metadata read even after quorum agreement was reached. Flip RUSTFS_GET_METADATA_EARLY_STOP_ENABLE to default-on. The gate stays conservative: should_allow_metadata_early_stop only admits metadata-only reads (read_data=false) without version_id, healing, or free-version requirements, everything else falls back to the full-wait fanout, and setting the env var to false restores the old behavior entirely. The version-aware gate (RUSTFS_GET_METADATA_VERSION_EARLY_STOP_ENABLE) remains opt-in because versioned reads carry a higher stale-selection risk profile. Also replace the stale "optimize concurrency" TODO in get_object_fileinfo with a pointer to the early-stop implementation and add regression tests for the new default plus the explicit opt-out path. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): lazily construct codec streaming multipart readers (#4350) * perf(ecstore): lazily construct codec streaming multipart part readers (backlog#871) get_object_decode_reader_with_fileinfo opened shard readers for every part of a multipart object before returning the streaming reader, so TTFB paid for parts x disks file opens up front and an early client disconnect wasted the setup work for every unread part. Replace the eager loop with LazyMultipartCodecStreamingReader: the first part is still built eagerly so the dominant fallback conditions (missing shards / read quorum) are detected before any byte is streamed and the whole request can fall back to the legacy duplex path exactly as before. Each subsequent part is built on demand -- when the previous part hits EOF -- via a spawned task handle owned by the reader; dropping the reader aborts an in-flight build so disconnects stop all further IO. If a later part hits a fallback condition mid-stream (a shard vanished after the request started), the reader surfaces an explicit read error with a pipeline-failure metric instead of silently degrading; the client's retry then detects the condition on the eager first-part setup and takes the legacy path cleanly. Adds unit tests for in-order streaming across lazy boundaries, deferred construction (no build when the client stops within part 1), and the mid-stream fallback error path. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): prefetch next multipart part reader setup during decode (#4351) * perf(ecstore): prefetch next multipart part reader setup during decode (backlog#870) get_object_with_fileinfo processed multipart parts strictly serially: the next part's bitrot reader setup (file opens + read-quorum wait across all disks) only started after the current part finished decoding, so large multipart reads paid full setup latency between every part. Overlap the two stages with a depth-one pipeline: right after the current part's readers are obtained, the next part's setup is spawned (shared inputs behind Arc) and joined when the loop reaches that part. The shared setup_multipart_part_readers helper keeps stage-duration metrics semantics identical for both paths; a failed or stale prefetch falls back to the synchronous setup, and the PrefetchedReaderSetup guard aborts the in-flight task on error returns, early breaks, or caller drop so disconnects stop background disk IO. Gate: RUSTFS_GET_MULTIPART_READER_SETUP_PREFETCH (default on, env opt-out). Adds a three-part end-to-end read test covering the prefetch hit path and cross-part content ordering. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): move FileInfo through GET shuffle instead of cloning (#4352) perf(ecstore): move FileInfo entries through the GET shuffle instead of cloning (backlog#873) shuffle_disks_and_parts_metadata_by_index deep-cloned every valid FileInfo (parts, erasure info, metadata map) once per disk on each GET. Add an ownership-taking variant that runs the same by-index consistency check as a read-only first pass and then moves entries into their shuffled slots with mem::take, and switch get_object_with_fileinfo to it -- that call site already owned the parts metadata vector. Disk handles are Arc clones and stay cheap. Scope notes from the backlog#873 audit: - get_object_fileinfo's disks.clone() stays: DiskStore is Arc<Disk>, so the clone is per-slot refcounting and correctly avoids holding the RwLock read guard across the metadata fanout awaits. - get_object_decode_reader_with_fileinfo keeps the borrowing shuffle: its caller must retain files/disks for the legacy fallback path, so an owned variant would just shift the same clone upstream. - The metadata-cache hit path still clones parts_metadata; sharing the cached entry via Arc changes the read-path return types and is left as a follow-up. Equivalence tests cover both the by-index placement and the mod-time fallback against the borrowing variant. Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> * fix(ecstore): gate merge emission on cleaned key and clear clippy redundant_clone Address review + CI findings on the ListObjects/GET optimization PR: - merge_entry_channels gated emission on the raw entry name while the heap orders by the cleaned key, so entries whose cleaned order and raw byte order disagree (e.g. redundant slashes) could be dropped. Gate on the same cleaned sort key the heap uses; add a regression test (`a//c` after `a/b`). - Drop three redundant `.clone()` calls in test code flagged by clippy::redundant_clone (owned-shuffle equivalence tests and the walk ascending-versions contract test) that failed the CI clippy gate. - Document the known mid-stream fallback limitation of the opt-in multipart codec streaming reader (default off) and mark the in-place per-part legacy degradation as a follow-up. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): force full metadata fanout for object tagging writes (backlog#872) put_object_tags reads the object fileinfo with read_data=false and then writes the updated tags to the online-disk set that read returned. With metadata early-stop enabled by default, that read now returns as soon as read quorum is reached, so the online-disk set is only a read-quorum subset. Writing tags to that subset fails write quorum -> ErasureWriteQuorum -> S3 SlowDown, which is exactly the s3-tests tagging failures (PutObjectTagging/DeleteObjectTagging, reached max retries). Thread a caller-controlled `allow_early_stop` gate through read_all_fileinfo_observed/_inner and add get_object_fileinfo_gated; put_object_tags calls it with allow_early_stop=false so the metadata read does the full quorum fanout and returns the complete online-disk set as the write target. Pure-read callers (GET/HEAD/tag read) keep the early-stop fast path unchanged. Extract metadata_early_stop_permitted() as the single gate and add a unit test locking the invariant: caller opt-out (and observe=false, and data reads) never early-stop even with the env flags on. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
c9b976ad46 |
feat(ecstore): reclaim orphaned data dirs during heal (#4356)
* feat(ecstore): reclaim orphaned data dirs during heal (backlog #3231/#3191) Pre-#3510, an unversioned overwrite leaked one UUID-named data dir per PUT. The write path now cleans up going forward, but pre-existing strays stay on disk forever: heal's dangling logic only removes whole objects whose data is missing, never surplus data dirs of an otherwise-healthy object. That leaked space is what eventually made ListObjects scan tens of GB and time out. Add SetDisks::reclaim_orphan_data_dirs, a fail-closed, quorum-safe sweep that mirrors purge_orphan_dir_object: - referenced set is the UNION of get_data_dirs() across every online replica's xl.meta, so a dir named by any replica is kept; - if a disk holds the object dir but its xl.meta is missing or unparseable the object is treated as degraded and nothing is removed; - only UUID-named subdirectories are ever considered. Wire it into heal_object's healthy tail (under the object write lock) as a best-effort step so the background heal scanner and admin heal recover the leaked space automatically; a reclaim failure never fails the heal. Covered by four unit tests: removal of an unreferenced dir, no-op when all dirs are referenced, fail-closed abort on missing metadata, and cross-replica union preservation. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): fix typo unparseable -> unparsable Satisfies the repository typos CI check. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
0ce0388fc0 |
fix(ecstore): fsync inline rename_data rollback backup (#4346)
* fix(ecstore): fsync inline rename_data rollback backup The inline branch of LocalDisk::rename_data writes the previous version's xl.meta rollback backup (<old_data_dir>/xl.meta.bkp) with a bare std::fs::write, fsyncing neither the file nor its new directory entry, even when drive_sync_enabled() is true. The same closure already syncs the new xl.meta and the commit rename directory, and the non-inline branch persists its backup durably. That backup is the sole restore source for delete_version(undo_write=true) on a set-level write-quorum failure. With sync enabled, an inline overwrite plus power loss plus a quorum failure that triggers undo_write could restore a lost or truncated backup and fail to roll back to the prior committed object. Write the inline backup via OpenOptions + write_all and, when sync is enabled, sync_data() it and fsync_dir_std its parent, mirroring the non-inline branch. Extend the inline backup test to assert the .bkp contents equal the previous metadata bytes. Refs backlog#868 (disk-durability-01). * fix(ecstore): preserve to_file_error mapping on inline backup write Address review: the rewritten inline rollback-backup write must keep the same DiskError classification as the previous std::fs::write(...).map_err( to_file_error)? call. Without it, io errors (PermissionDenied/NotFound/ StorageFull/...) would surface as DiskError::Io(_) instead of the specific DiskError variants (FileAccessDenied/FileNotFound/DiskFull/...), since From<io::Error> for DiskError only recovers variants that were wrapped via to_file_error. Map open/write_all/sync_data/fsync_dir_std through to_file_error, matching write_all_internal. --------- Co-authored-by: houseme <housemecn@gmail.com> |
||
|
Zhengchao An
|
5594b18912 |
fix(ecstore): make delete_volume non-recursive by default to prevent bucket-heal wipe (backlog#799 B1) (#4339)
* fix(ecstore): make delete_volume non-recursive by default to prevent bucket-heal wipe (backlog#799 B1) `delete_volume` unconditionally `remove_dir_all`'d the whole bucket tree, and the bucket-heal "remove" branch called it fire-and-forget on every local disk. A mis-classified "dangling" bucket (or a non-force S3 DeleteBucket on a populated bucket) was therefore recursively wiped — a potential whole-bucket data loss. The `VolumeNotEmpty` -> recreate/`BucketNotEmpty` handling already present in both delete_bucket paths was dead code because the primitive never refused. Add an explicit `force_delete` flag to `DiskAPI::delete_volume` and default the non-force path to a non-recursive `remove_dir` (rmdir), which fails atomically with `VolumeNotEmpty` if the bucket still holds any object data. Only an explicit force delete (S3 force bucket delete) removes recursively. Mirrors MinIO's `xlStorage.DeleteVol` (`Remove` vs `RemoveAll`). - Trait + all impls (local behavior, dispatch, disk_store, remote RPC) take the flag; the gRPC `DeleteVolumeRequest` gains a `force` field (proto3 default false → old peers get the safe non-recursive behavior on rolling upgrade). - Heal remove branch passes `false` and no longer discards the result: a `VolumeNotEmpty` refusal is logged (the bucket is not dangling) instead of wiping data. - Both `delete_bucket` paths pass `opts.force`, activating the previously-dead `VolumeNotEmpty` -> `BucketNotEmpty`/recreate handling (correct S3 semantics). Adds a regression test: non-force delete of a non-empty bucket returns VolumeNotEmpty and preserves the data; force delete removes it. Design converged by two independent expert reviews (MinIO-fidelity + defense-in-depth) referencing MinIO xl-storage.go. Refs backlog#799 (B1), issue rustfs/backlog#850. The safety expert's deeper hardening (typed capability instead of a bool, trash-instead-of-in-place for force, quorum re-verification of dangling) is noted on #850 as follow-up. * fix(ecstore): reword 'mis-classified' -> 'misclassified' to satisfy typos (backlog#799 B1) * fix(rustfs): thread force_delete through StorageDiskRpcExt::delete_volume + test literal (backlog#799 B1) |
||
|
Zhengchao An
|
b813fc7739 |
fix(ecstore): reject zero erasure block_size in codec streaming read (#4340)
The codec streaming GET reader divides by erasure.block_size in build_codec_streaming_part_reader without validating the erasure dimensions, unlike the legacy multipart path which already rejects block_size==0 / data_shards==0. FileInfo::is_valid() does not check block_size, so corrupted on-disk metadata (block_size==0, data_blocks>0) passes validation and panics the read task with a divide-by-zero. Add Erasure::has_valid_dimensions() and reject invalid dimensions at the codec streaming entry before any disk access, mirroring the legacy guard (which now reuses the same predicate). Refs backlog#868 (868-1). |
||
|
houseme
|
6f613317f6 |
feat(internode): optimize gRPC transport (#4337)
* feat(internode): P0 gRPC transport tuning, message limits, payload metrics Land the P0 subtask from docs/grpc-optimization: close the client-vs-server transport gaps and add instrumentation to size which unary RPCs need channel isolation in P1. Transport tuning (G3): the client `Endpoint` now disables Nagle and raises the HTTP/2 stream/connection flow-control windows to mirror the server socket, so small lock/health RPCs are not batched and larger metadata responses are not throttled by the 64KiB default window. All env-overridable, 0 opts out. Message-size limits (G1): both `NodeServiceClient` and `NodeServiceServer` set max decode/encode size (default 100MiB) instead of tonic's silent 4MiB cap, so a large multi-version xl.meta or aggregated ReadMultiple no longer fails out_of_range. The server limit is set on `NodeServiceServer` before wrapping in the auth `InterceptedService` (the interceptor type does not expose it). Payload instrumentation (P1 prep): ReadAll/ReadMultiple record a payload-size histogram plus a large-payload counter when a response crosses the configured threshold (default 8MiB), feeding alerting on paths that contend with latency-sensitive control-plane traffic on the shared channel. Threshold-only counter, no per-call hot-path log. Verification: cargo check/test on config, io-metrics, ecstore, rustfs; clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P1 control/bulk gRPC channel isolation (opt-in) Land the P1 subtask from docs/grpc-optimization: physically separate large bytes-carrying unary RPCs from latency-sensitive control-plane RPCs so a big transfer can no longer head-of-line block a lock/health RPC on the shared HTTP/2 connection (G2/G5). Introduce ChannelClass { Control, Bulk } and get_channel_for_class in protos. Control RPCs keep the per-peer connection keyed by the bare address; Bulk RPCs (ReadAll/WriteAll/ReadMultiple/BatchReadVersion, via a new get_bulk_client) are round-robined across a small per-peer bulk pool. Rather than restructuring the global GLOBAL_CONN_MAP (and every consumer), bulk channels are cached under a composite key (addr\0bulk\0idx). The NUL separator cannot appear in a URL, so bulk keys never collide with the control key. This keeps the blast radius small on a consistency-sensitive path. create_new_channel is refactored into build_channel(dial_addr, cache_key) so several physically distinct channels to one peer cache independently while dialing/TLS still use the real address. Gated by RUSTFS_INTERNODE_CHANNEL_ISOLATION (default OFF) so the default build is byte-for-byte the pre-P1 behavior: bulk resolves to the control channel and the switch is a single-env rollback. RUSTFS_INTERNODE_BULK_CHANNELS (default 2, clamped >=1) sizes the pool. On failure, evict_failed_connection drops the whole bulk pool for the peer (round-robin hides which index was used), avoiding half-dead cached channels. Lock RPCs (remote_locker) already use the default Control path, so lock semantics and retry behavior are unchanged. Verification: cargo check/test on config, protos, ecstore, rustfs; new protos tests for bulk key routing and isolation-off passthrough; clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P2 msgpack/JSON codec observability + encode buffer presizing Land the safe, wire-compatible slice of P2 from docs/grpc-optimization: the observability prerequisite for retiring the redundant JSON fields, plus a codec micro-optimization. No proto/wire-format change; JSON is still dual-written. Internode RPCs today dual-encode each metadata value as both msgpack (`*_bin`) and a JSON compatibility string, and decoders prefer `_bin` with a JSON fallback. Before the JSON fields can ever be dropped (a cross-version change), that fallback must be proven unused in production. Add rustfs_system_network_internode_msgpack_json_fallback_total{direction, message}: incremented whenever a decode falls back to the JSON field because the msgpack payload was absent. Wired into both directions — the client decoding peer responses (remote_disk.rs, incl. the list-level read_multiple/batch fallbacks) and the server decoding peer requests (node_service/disk.rs). This counter must read zero across a release window before send paths stop writing JSON and the proto text fields are reserved/removed (the deferred P2-1 steps). Also pre-size the msgpack encode buffers (Vec::with_capacity(512)) on both sides, eliminating the repeated growth reallocations for typical FileInfo payloads with zero added copy. Full thread_local buffer pooling is deferred: it needs either an extra copy (unclear net win) or a send-path buffer-return lifecycle, to be justified by a codec microbenchmark first. Verification: cargo check/test on io-metrics, ecstore, rustfs; new fallback counter smoke test; existing codec decode tests green; clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(internode): add msgpack/JSON convergence observation runbook Runbook driving the observation-gated retirement of the redundant JSON compatibility fields on internode gRPC metadata RPCs (grpc-optimization P2-1). Documents the shipped fallback counter (rustfs_system_network_internode_msgpack_json_fallback_total{direction,message}), the PromQL to confirm it reads zero across a release window, a standing alert, and the staged flip/rollback procedure (env-gated msgpack-only send, then proto field removal in N+1). Includes the verified field -> peer-decoder audit: only fields whose peer decodes _bin first may be converged. Notes DeleteVersion.opts (DeleteOptions) is NOT convergence-ready — its server handler is not _bin-first and must gain a decode_msgpack_or_json path first. This gates the send-side change so it cannot empty a JSON field an old peer still needs. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): env-gated msgpack-only send + DeleteVersion _bin support (P2-1) Implements the send-side lever for retiring the redundant JSON compatibility fields on internode gRPC metadata RPCs, plus the missing `_bin` support on the delete path that it depends on (grpc-optimization P2-1). Default-off: the base build is byte-for-byte the prior dual-write behavior. Gated msgpack-only send (RUSTFS_INTERNODE_RPC_MSGPACK_ONLY, default false): - New rustfs_protos::internode_rpc_msgpack_only() reads the flag. - Client (remote_disk.rs) compat_json() and server (node_service/disk.rs) compat_response_json() emit an empty JSON string when the flag is on, so only the msgpack _bin payload is sent. The _bin field is always sent; decoders keep the JSON read fallback. Applied only to fields with a confirmed _bin-first peer decoder (WriteMetadata/UpdateMetadata/RenameData file_info, UpdateMetadata opts, ReadOptions, ReadMultipleReq, BatchReadVersionReq; ReadVersion/ReadXL/RenameData responses and the ReadMultiple/BatchReadVersion response lists). - Only enable after the P2 fallback counter has read zero across a release window (see docs/operations/internode-msgpack-json-convergence-runbook.md). Single-env rollback; no wire-format break. DeleteVersion(s) _bin support (prerequisite): - The DeleteVersion/DeleteVersions protos had NO _bin fields. Add additive (backward-compatible) bytes file_info_bin/opts_bin (DeleteVersion) and repeated bytes versions_bin + bytes opts_bin (DeleteVersions); regenerate the checked-in prost struct. - Client dual-writes them; server decodes them _bin-first with JSON fallback. - These delete fields are kept OUT of the msgpack-only set (always dual-write) until their own fallback counter reads zero across a window with the new decoders fully deployed. DeleteVersion.raw_file_info stays JSON-only (no _bin field yet). Verification: cargo check/test on protos, config, ecstore, rustfs (incl. the six delete request handler tests and a compat_json default-path test); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P3 cluster peer online/offline health metric Land the safe observability core of P3 (grpc-optimization G6/G8): track each internode peer's reachability and expose the offline count, for parity with MinIO's minio_cluster_servers_offline_total. Pure instrumentation — peer selection and quorum are unchanged. - io-metrics: per-peer PeerHealthState { online, consecutive_failures } registry plus record_peer_reachable/record_peer_unreachable. A peer flips offline after N consecutive failures (dial failures or RPC-triggered evictions) and back online on the next successful dial; the count of offline peers is published to the rustfs_cluster_servers_offline_total gauge. - config: RUSTFS_INTERNODE_OFFLINE_FAILURE_THRESHOLD (default 3, clamped >= 1). - protos: build_channel marks the peer reachable on a successful dial and unreachable on a dial failure; evict_failed_connection feeds the failure signal too. Keyed by the real peer address, so control and bulk channels to one peer share health state. Deferred (documented in docs/grpc-optimization P3): startup prewarm (no clean topology-ready hook yet), the offline fast-bypass in peer routing (consistency- sensitive; must not change quorum), and idempotent-read-only retry. This commit is observability only. Verification: cargo check/test on io-metrics, config, protos (new peer-health state-machine and threshold-clamp tests); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P3 control-channel prewarm + self-healing offline bypass Add the remaining P3 connection-lifecycle levers (grpc-optimization G6/G8), both env-gated and default-off so the base build is unchanged. Prewarm (RUSTFS_INTERNODE_PREWARM, default off): RemoteDisk::new spawns a best-effort background dial of the peer's control channel, deduped per peer address, moving the connect cost off the first RPC. Failures fall through to the existing lazy connect + recovery monitor. Offline bypass (RUSTFS_INTERNODE_OFFLINE_BYPASS, default off): remote_disk get_client/get_bulk_client fast-fail a peer already marked offline instead of paying the connect timeout, so the erasure layer proceeds on quorum sooner. This does NOT change quorum. It is self-healing: cluster_peer_should_bypass lets one request per RUSTFS_INTERNODE_OFFLINE_REPROBE_SECS (default 5s) through to recover the peer even with no background monitor, and the recovery monitor's own probe path calls the client directly so it is never bypassed. io-metrics gains cluster_peer_is_offline / cluster_peer_should_bypass (with a per-peer re-probe timestamp). Scope: data path only — remote_locker (lock RPCs, most consistency-sensitive) is left dual-writing/unbypassed as a follow-up. Verification: cargo check/test on io-metrics, config, ecstore (new self-healing bypass tests; all 105 rpc tests green); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(internode): add A/B benchmark runbook for gRPC optimization stages Reproducible before/after collection procedure for grpc-optimization P0–P3. Since every stage is env-gated, before/after is the same binary with different env — no rebuild. Documents, per stage: the exact env toggles (baseline vs enabled column), which existing bench script to run (run_internode_transport_baseline.sh / run_four_node_cluster_failover_bench.sh), the Prometheus metrics to capture, and the acceptance gates from the design docs (e.g. lock p99 down >= 20% for P1, msgpack fallback counter = 0 before enabling P2, correct rustfs_cluster_servers_offline_total for P3). Live runs require a multi-node cluster + load tool + Prometheus scrape and cannot be produced in a single-process sandbox; artifacts land under target/bench (gitignored) and attach to the PR. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(internode): P3-2 lock-path offline bypass + P3-3 idempotent read retry Extend the offline bypass to the lock path and add opt-in retries for idempotent reads (grpc-optimization P3-2/P3-3). Both env-gated and default-off/zero. Offline bypass (lock path): factor the bypass decision into a shared pub(crate) internode_offline_bypass_reason(addr) and call it from remote_locker::get_client too, so lock RPCs to an offline peer fast-fail (letting dsync reach quorum sooner) instead of paying the connect timeout. Does not change quorum; the self-healing re-probe keeps peers recoverable. Gated by RUSTFS_INTERNODE_OFFLINE_BYPASS (default off). Idempotent read retry (P3-3): add execute_read_with_retry — a bounded, exponential-backoff retry for read-only/reentrant RPCs on transient network errors — and route disk_info through it. RUSTFS_INTERNODE_IDEMPOTENT_READ_RETRIES defaults to 0 (disabled). Write/lock RPCs are never retried (quorum/idempotency safety, per CLAUDE.md); the wrapper requires an Fn closure so only reads that rebuild their request from borrowed inputs qualify. Deferred: grpc.health.v1 (optional ecosystem-compat only; needs a new tonic-health dep and 3-way hybrid-service wiring — internal needs are met by the existing Ping RPC). Verification: cargo check/test on config, ecstore (105 rpc tests green incl. disk_info now via the retry wrapper); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(scripts): one-click internode gRPC A/B benchmark driver Wrap the per-stage env matrix from the benchmark runbook into scripts/run_internode_grpc_ab_bench.sh: given --stage <p0|p1|p2|p3> and --phase <before|after>, it emits the stage/phase RUSTFS_INTERNODE_* server env to <out-dir>/server-env.sh and runs the right underlying bench (run_internode_transport_baseline.sh for p0/p1/p2, run_four_node_cluster_failover_bench.sh for p3) into a labeled target/bench/internode-transport/<stage>-<phase>/. Passthrough args after `--` reach the underlying bench; --dry-run previews the env + command. The script is explicit that RUSTFS_INTERNODE_* are server env, so for the load-driven stages the operator must restart rustfs with the emitted env before the run; the docker four-node (p3) path exports them for a forwarding compose. shellcheck-clean. Runbook updated with a "One-click driver" section. Co-Authored-By: heihutu <heihutu@gmail.com> * chore(compose): forward RUSTFS_INTERNODE_* into the four-node cluster The four-node local-build compose only forwarded a fixed whitelist of env, so the internode gRPC knobs (grpc-optimization P0-P3) never reached the containers and the A/B bench driver's "after" phase was a no-op. Forward the full RUSTFS_INTERNODE_* set with defaults matching the binary defaults, so leaving them unset is a no-op and the A/B driver can toggle a stage per phase. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(internode): address Copilot review — retry health action + poison-safe peer health Two review nits on #4337: - P3-3 idempotent read retry (remote_disk.rs): execute_read_with_retry ran every attempt through execute_with_timeout_for_op, which hardcodes FailureHealthAction::MarkFailure. So the first transient error could flip the disk faulty and short-circuit the remaining retries, and each attempt over-counted the failure. Route all but the final attempt through execute_with_timeout_for_op_and_health_action with IgnoreFailure; only the last attempt marks faulty/evicts. No default impact (retries default 0). - Peer-health helpers (io-metrics): record_peer_reachable/record_peer_unreachable, cluster_peer_is_offline and cluster_peer_should_bypass early-returned on a poisoned mutex, permanently stalling the offline gauge and bypass state after a single panic. Recover via PoisonError::into_inner(). Verification: cargo check/test on io-metrics + ecstore (105 rpc tests green); clippy clean on touched files; make pre-commit green. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
68f048b8fe |
fix(replication): persist delete marker mtime in MRF entries (#4331)
fix(replication): persist original mtime in MRF entries (backlog#867) MRF delete entries did not persist the original delete-marker mtime, so after a restart the recovery replay path reconstructed the delete without a source timestamp. Downstream the replica delete-marker was stamped with the replay time (now()) instead of the source mtime, causing delete-marker timestamp divergence across clusters. Extend the MrfReplicateEntry disk format with an optional deleteMarkerMtime field (persisted as Unix nanoseconds) in both duplicate struct definitions (rustfs-replication and rustfs-filemeta). DeletedObjectReplicationInfo now persists delete_marker_mtime, and start_mrf_processor restores it onto the reconstructed delete so the replica keeps the source timestamp. Backward compatibility: the new key uses skip_serializing_if + serde default, so historical MRF files without it decode to None and replay falls back to the current time (pre-#867 behaviour). No panic or entry loss on old files. Closes rustfs/backlog#867 |
||
|
Zhengchao An
|
3bc8d79fe5 |
fix(multipart): serialize put_object_part per uploadId (#4329)
fix(multipart): serialize the part commit per uploadId to prevent mixed-generation shards (backlog#853) Concurrently re-transmitting the same part could land two shard generations across different disks: each shard is individually bitrot-valid, so the corruption surfaces only as a silent mix at read time. The hazard is confined to rename_part, where two temp parts are moved cross-disk onto the SAME final part path — interleaving there can leave shards from two generations. Each concurrent stream already writes to its own unique temp dir, so the encode/stream phase never conflicts and must stay lock-free: holding a lock across it would serialize slow re-transmits of the same part, break the S3 'last finisher wins' semantics, and cause UploadPart lock-acquire timeouts (ServiceUnavailable). Scope a write lock to the uploadId namespace around only the rename_part commit, so each commit is atomic across disks and the last committer wins consistently. Mirrors MinIO's per-uploadID NS lock; disjoint from the object lock held by complete_multipart_upload (no lock-ordering cycle). Honors opts.no_lock. The pre-delete-before-rename half of backlog#853 was already fixed in #4316; this completes the issue. Adds an end-to-end regression test: six concurrent resends of the same part must all succeed (no lock-acquire timeout), exactly one generation stays visible, and the reassembled object equals one intact generation. Closes rustfs/backlog#853 |
||
|
houseme
|
6d0583872b |
test(ecstore): serialize list_objects mutation-sequence tests (#4338)
Three tests share the global list-objects mutation-sequence state (the LIST_OBJECTS_MUTATION_SEQUENCE atomic and the per-bucket sequence map) and all call reset_list_objects_mutation_sequences_for_test(), but only observed_mutation_persists_namespace_journal_high_water was marked #[serial_test::serial]. The serial lock only excludes tests that also carry the attribute, so the unmarked tests still ran concurrently with it: the journal test's persist of "bucket" -> 11 could land after the recovery test's reset, making it observe 11 instead of the expected 9 (flaky under the default multi-threaded test runner, green with --test-threads=1). Add #[serial_test::serial] to the two unmarked tests: - persistent_key_only_index_load_recovers_mutation_sequence_from_journal - list_objects_mutation_sequence_advances_per_bucket Unique bucket names alone would not fix this: the per-bucket assertions in list_objects_mutation_sequence_advances_per_bucket check exact values produced by the single global atomic counter, which any concurrent observe call perturbs, and every reset zeroes that global state for whichever peer is mid-flight. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
25193ee6cf |
feat(listobjects): add guarded metadata-fast listing (#4311)
* feat(listobjects): add phase 0 observability metrics * perf(listobjects): reduce gather metadata decoding * test(listobjects): add adversarial listing coverage * feat(listobjects): add source-aware cursor fields * feat(listobjects): define index source modes * feat(listobjects): model index rebuild health * feat(listobjects): add opt-in index fallback gate * feat(listobjects): verify index candidate pages * docs(listobjects): add rollout runbook * docs(listobjects): untrack baseline fixtures * feat(listobjects): add opt-in key-only provider * feat(listobjects): add index verification metrics * perf(listobjects): gate list metrics on get stage switch * feat(listobjects): bind provider health generation * feat(listobjects): add persistent key-only provider * feat(listobjects): bind persistent provider lifecycle * feat(listobjects): track persistent index mutations * feat(listobjects): persist index mutation checkpoints * fix(listobjects): aggregate benchmark metric snapshots * feat(listobjects): store journal in system namespace * chore(listobjects): report fallback reasons in bench * test(listobjects): verify metadata-fast stale fallbacks * feat(listobjects): serve metadata-fast snapshots behind guardrails * test(listobjects): add metadata-fast chaos bench * fix(listobjects): attribute metadata-fast fallback metrics * test(listobjects): add metadata-fast fallback chaos probes * test(listobjects): add quorum journal chaos guardrails * fix(listobjects): satisfy pre-pr clippy gate * docs(listobjects): untrack rollout runbook * fix(ecstore): remove redundant heal error conversion * test(filemeta): satisfy replication info clippy ---- Co-Authored-By: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
a2d679cdb6 |
fix(bucket): propagate bucket deletion to peer metadata caches (backlog#646) (#4326)
The peer `DeleteBucketMetadata` RPC handler was a stub that returned success without doing anything, and the delete-bucket flow never sent the notification in the first place. As a result, after a bucket was deleted other nodes kept serving its stale cached metadata. Wire the whole path end to end: - ecstore: add an in-memory `remove_bucket_metadata` (free fn) and `BucketMetadataSys::remove`, the counterpart to `set_bucket_metadata`, and export it through the `api::bucket::metadata_sys` facade. - node_service: `handle_delete_bucket_metadata` now validates the bucket name and actually drops the cached metadata for it. - bucket_usecase: after a successful delete_bucket, notify peers via `notification_sys.delete_bucket_metadata` in the background, symmetric to the existing `notify_bucket_metadata_reload` path. Also update the delete-bucket-metadata unit test to assert the empty-bucket rejection instead of the old always-success stub, and drop an unused `tracing::debug` test import left over from #4322. Verified: cargo fmt; cargo check -p rustfs-ecstore; cargo test -p rustfs --lib --features rio-v2 test_delete_bucket_metadata_empty_bucket; arch guardrail scripts pass. |
||
|
Zhengchao An
|
655c5cb403 |
fix(ecstore): reject short-read shards and advance ParallelReader per stripe (backlog#799 B2) (#4327)
Two compounding defects let a truncated shard corrupt a GET (ranged GET could return HTTP 200 with wrong bytes): - `BitrotReader::read` returned `Ok(short_len)` when the shard stream hit EOF before filling the caller's buffer. With `skip_verify` / `HashAlgorithm::None` / parity=0 there is no hash to catch it, so the short shard was accepted and every downstream byte shifted. - `ParallelReader.offset` was set once and never advanced per stripe, so every stripe after the first reused the first stripe's geometry and the last-stripe length clamp was wrong. Fix both (they are mutually required): - `BitrotReader::read` now errors (`UnexpectedEof`) on a short read, before and independent of the bitrot hash check, so it fires under skip-verify/no-hash too. The caller sizes the buffer to the expected per-stripe shard length, so "buffer not filled" == "shard truncated". A short read routes through the existing `errs[i]` path, dropping that reader from the stripe so parity reconstruction engages; with parity=0 the stripe fails read quorum and the GET errors loudly instead of streaming shifted bytes. Mirrors MinIO's `parallelReader.Read` (`n != shardSize` -> reader failed). - `ParallelReader::read` / `read_lockstep` advance `self.offset += shard_size` per stripe so the per-stripe expected length (incl. the shorter final stripe) is exact, matching the correct pattern already used by heal. Updates three bitrot tests that used an obsolete oversized-buffer pattern to size per-stripe (as the real decode/heal paths do), and adds a regression test that a truncated shard errors under None/HighwayHash + skip_verify. Refs backlog#799 (B2), issue rustfs/backlog#851. Design converged by two independent expert reviews referencing MinIO cmd/erasure-decode.go. |
||
|
Zhengchao An
|
6297d112c3 |
fix: auto-repair breaking changes from 1b3727e2 (#4324)
fix(heal): remove useless .into() conversion in heal error path Clippy flagged a useless_conversion lint at heal.rs:491 where is redundant because is already of type . |
||
|
Zhengchao An
|
31c6859965 |
chore: converge stale TODOs and apply safe fills (backlog#646) (#4322)
Second TODO-convergence round over the current tree (backlog#646). All line numbers in the old inventory had gone stale after the set_disk / diagnostics / cluster refactors, so this re-scans and reduces the marker count from 144 to 99. STALE removals (comment describes already-implemented behavior, or dead commented-out blocks) across ecstore (set_disk ops/core, store, cluster/rpc, bucket/metadata_sys, services), iam, filemeta, s3select and rustfs auth/object_usecase. No behavior change. Safe fills, each verified: - filemeta: replication_info_equals now also compares replication_state_internal (function currently has no callers; adds a regression test). - bitrot: drop the confirmed-unused `_want` parameter from bitrot_verify and the now-unused `sum` on LocalDisk::bitrot_verify, removing a Bytes::copy_from_slice allocation. Streaming verify uses the file's embedded per-shard hash, never the passed sum. - signer: rename v4_ignored_headers -> V4_IGNORED_HEADERS and drop the non_upper_case_globals allow. - admin/heal: test_decode was #[ignore]d and used serde_urlencoded on a JSON body (would panic); rewire to serde_json::from_slice to match the production decode path, add assertions, un-ignore. Verified: cargo fmt; cargo check on touched crates; tests pass (filemeta, signer, bitrot, heal::test_decode); arch guardrail scripts pass. |
||
|
Zhengchao An
|
83edafb11f |
fix(ecstore): reject renewing a mis-mounted drive into the wrong set (backlog#799 B19) (#4321)
fix(ecstore): reject renewing a misplaced drive into the wrong set (backlog#799 B19) `renew_disk` located the drive's (set, disk) position from its own format via `find_disk_index`, but never checked that the resolved `set_idx` matched this `SetDisks`' own `set_index` before inserting the drive into `self.disks`. A drive whose format places it in another set would be claimed by this set too, so two sets could manage the same drive and degrade together. Skip (with a warning) when `set_idx != self.set_index`. Refs backlog#799 (B19), tracked in rustfs/backlog#863. |
||
|
Zhengchao An
|
5c5f8063d8 |
fix(replication): log (not swallow) resync status persistence failure (backlog#799 B23) (#4320)
fix(replication): don't silently swallow resync status persistence failure (backlog#799 B23) After a resync computes a new replication status, it persists it via `put_object_metadata` but discarded the `Err` case (`if let Ok(u) = ...`). A failure left the object's on-disk replication status disagreeing with the resync result with no signal at all. Log the failure at warn level instead. Refs backlog#799 (B23), tracked in rustfs/backlog#863. |
||
|
Zhengchao An
|
1b3727e2c4 |
fix(heal): clean up .rustfs/tmp healed shards on all failure paths (backlog#799 B20) (#4319)
heal_object wrote healed shards to .rustfs/tmp/<uuid>/ and only removed that tmp dir on the success path (the final delete_all). Three early exits after the tmp shards were written leaked the dir: - `erasure.heal(...)?` failing midway, - the `disks_to_heal_count == 0` early return, and - the `?` on the post-rename remote data-dir delete. Add the tmp cleanup before the first two, and downgrade the remote data-dir cleanup failure to a warning (the healed shard is already renamed into place, so that cleanup failing must not abort the heal or leak the tmp shards). Refs backlog#799 (B20), tracked in rustfs/backlog#863. |
||
|
唐小鸭
|
849ea9a122 | fix(site-replication): align IAM and bucket metadata replication (#4318) | ||
|
Zhengchao An
|
8ffe230a89 |
fix(multipart): don't pre-delete the committed part before rename_part (backlog#853) (#4316)
`rename_part` unconditionally ran `cleanup_multipart_path([part.N, part.N.meta])` on every disk *before* the per-disk rename fan-out. The per-disk `rename_part` already replaces `part.N` atomically (std::fs::rename) and rewrites `part.N.meta`, so the pre-delete is redundant — and destructive: it removed an already-committed (ACKed) part on all disks before the new rename landed, so a re-upload of the same part that then failed write quorum destroyed the committed part outright. Remove the pre-rename cleanup and rely on the atomic rename to overwrite in place; the quorum-failure rollback below is unchanged. No behaviour change on the success paths (first upload / retry both end with part.N replaced), verified by the existing multipart suite. Refs backlog#799 (B4). The remaining half of B4 — serializing concurrent same-part uploads with an uploadId-scoped lock so two generations can't be mixed across disks — is a larger concurrency change tracked as a follow-up on the issue. |
||
|
Zhengchao An
|
5e73d59eb6 |
fix(replication): re-derive replication decision when replaying MRF delete entries (backlog#858) (#4315)
MRF delete replay reconstructed the delete with `..Default::default()`, leaving `replication_state = None`. `replicate_delete` derives its target set purely from `replication_state.replicate_decision_str`, so an empty state produced an empty decision -> zero targets: the replayed delete contacted no remote at all, a silent no-op that left replicas permanently diverged after a restart. The MRF entry doesn't persist the decision and the source object is already gone, so re-derive it from the live bucket config at replay time via `check_replicate_delete` — mirroring the object heal path (`get_heal_replicate_object_info`) — and set it on the reconstructed delete's `replication_state`. This is a contained fix with no change to the on-disk MRF format. Refs backlog#799 (B9). Note: the source delete-marker mtime is still not persisted in the MRF entry, so a replayed delete marker is stamped with the replay time on the target. That is a separate, minor consistency nuance (the delete now propagates correctly) and can be addressed by extending the MRF entry format in a follow-up. |
||
|
Zhengchao An
|
f33ae8e9af |
fix(replication): keep MRF persister backlog cumulative so flushes don't drop entries (backlog#859) (#4314)
The MRF persister accumulated overflow entries in `pending`, flushed them with `flush_mrf_to_disk`, and cleared `pending` on success. But `flush_mrf_to_disk` *overwrites* the whole MRF file with exactly the entries passed. After flushing batch A (file = A) and clearing, the next flush wrote batch B and thereby overwrote the file to contain only B — and the MRF file is only replayed (and cleared) at startup, never during the run, so batch A's entries were silently lost. A crash after the B flush lost all of batch A's pending replications. Keep `pending` cumulative (the file must hold the full set of overflow entries for the run) and rewrite the whole set on each flush instead of clearing after success: - flush eagerly once 1 000 *new* entries accumulate since the last write (measured against the flushed length, so a large backlog isn't rewritten on every add), and on the 10s tick when dirty; - bound the in-memory/on-disk backlog with `MRF_PENDING_CAP` (200 000) and log once when the cap is hit rather than growing without limit. Refs backlog#799 (B10). |
||
|
Zhengchao An
|
650a3e5734 |
fix(replication): classify resync verification HEAD errors instead of miscounting (backlog#862) (#4312)
The resync result verification HEADed the target after replicating and counted the outcome with inverted error handling: - for a delete marker, ANY HEAD error (timeout, 5xx, auth, malformed) was counted as replicated (success); - for a versioned object against an AWS-style target, HEAD was sent with the RustFS UUID versionId, which AWS rejects with 400, so a well-replicated object was counted as failed. Classify the error before counting: - delete marker: only a definitive 404/NoSuchKey or 405/MethodNotAllowed confirms the marker propagated (`is_retryable_delete_replication_head_error` == false); any retryable/ambiguous error now counts as failed; - versioned object with a version-id-format rejection: re-verify via `head_object_fallback` (versionId-less HEAD) before deciding — present -> replicated, absent/error -> failed; - all other errors: failed, as before. Reuses the existing, unit-tested classifier helpers. Verified against the existing resyncer suite (24 tests). Refs backlog#799 (B13). |
||
|
Zhengchao An
|
32845e03b0 |
fix(replication): mark version-id fallback ETag match as Completed, not Failed (backlog#860) (#4310)
During resync against an AWS-style target that rejects RustFS UUID versionIds,
the code retries HEAD without a versionId and compares ETags. On a match it set
`replication_action = None` ("already in sync, nothing to copy") but did not
return, so control fell through to `if replication_action != ReplicationAction::All`
— a branch meant only for the unsupported metadata-only case — and stamped the
object FAILED with "metadata-only replication is not implemented". The target
already held an identical object, yet the source recorded FAILED forever, so
AWS-style targets never converged and the MRF kept re-queuing.
Handle `ReplicationAction::None` explicitly before that branch: record it as
Completed (with the resync timestamp/`replication_resynced` bookkeeping for
ExistingObject + reset_id, mirroring the HEAD-success None path) and return.
Only `ReplicationAction::Metadata` now takes the metadata-unsupported failure
branch; `All` still proceeds to the copy. This path is the only way `None`
reaches that point (the HEAD-success None case already returns earlier).
Refs backlog#799 (B11).
|
||
|
Zhengchao An
|
1cc1fc0f83 |
fix(ecstore): exclude failed-shard disks from commit so write quorum isn't inflated (backlog#852) (#4309)
`MultiWriter` recorded per-writer failures only in a private `errs` vector and nulled the failed writer locally, but `put_object` never used that to prune the disk set: `rename_data` ran over the full `shuffle_disks`, so a disk that took a short/failed write still had its truncated shard renamed into place and counted as an online disk. The object then claimed N good shards while one was short/corrupt, so a single later disk failure could drop it below reconstructable quorum — silent data loss. - `write_shard`: null the writer on a generic write error too (not only on `ShortWrite`), so a failed writer is uniformly represented as `None` — matching `shutdown_writer`, which already does this. - `put_object`: after encode, drop every disk whose writer failed (`drop_failed_writer_disks`) from `shuffle_disks` before `rename_data`, and re-check write quorum over the survivors. `rename_data` already re-checks quorum and rolls back if too few disks remain, so excluded disks are neither committed nor counted (MinIO sets failed writers to nil before `renameData`). Adds unit tests for the exclusion/quorum accounting. Refs backlog#799 (B3). |
||
|
Zhengchao An
|
7975f26b90 |
fix(ecstore): reach orphan-dir purge on nil-version delete miss (#4189) (#4307)
PR #4220 added a purge for orphan empty-directory trees (folder keys with no xl.meta) on the delete path, but the guard only accepted object-not-found. Over the real HTTP DELETE path the guard is never reached: `del_opts` pins `version_id = Uuid::nil()` for directory keys, so the missing dir object fails the specific-version lookup with version-not-found (FileVersionNotFound), not object-not-found. The guard short-circuits, the store returns the error, and the API layer turns it into a fake 204 — the ghost folder survives, exactly the #4189 symptom. The existing unit tests passed because they call `purge_orphan_dir_object` directly, bypassing the nil-version lookup. Accept both misses in the guard (extracted as `should_purge_orphan_dir_on_missing`) so the folder delete actually takes effect. Non-directory keys and non-miss errors (e.g. quorum failures) are unaffected; the cross-disk data-safety refusal in `purge_orphan_dir_object` is unchanged. Verified end-to-end against a running 4-disk erasure server: DELETE of a planted orphan `ghost/` tree now purges it on all drives and clears the listing, a real object under the same prefix is untouched, and a prefix holding real data on any drive is refused (all drives preserved). Adds predicate regression tests covering version-not-found / object-not-found on directory keys, and the negative cases. |
||
|
Zhengchao An
|
a9115f729e | fix: block force delete on object lock buckets (#4298) | ||
|
Zhengchao An
|
3e4c15da5d | fix(object-lock): prevent locked version deletes (#4297) | ||
|
cxymds
|
2585855d23 | fix(ecstore): hide deleted versioned folder prefixes (#4296) | ||
|
Zhengchao An
|
e68a52ff59 | fix: replace while-let loop with for-loop to fix clippy lint on main (#4292) | ||
|
Zhengchao An
|
a8d8e56478 | refactor(ecstore): relocate NamespaceLocking + finalize God-Object split (backlog#822) (#4291) | ||
|
Zhengchao An
|
f737b39cfc |
refactor(ecstore): move ObjectIO/ObjectOperations into set_disk::ops::object (backlog#821) (#4290)
P6 of the SetDisks God-Object split (tracking backlog#815, issue backlog#821; depends on P5 #4288). Relocate the core object read/write hot-path contract impls out of the set_disk/mod.rs God-Object into their own module home: - impl ObjectIO for SetDisks (~1,010 lines) -> set_disk/ops/object.rs - impl ObjectOperations for SetDisks (~1,137 lines) -> set_disk/ops/object.rs - Registered pub(crate) mod object; under set_disk/ops. Pure move — zero logic change. Both contracts stay implemented for SetDisks, so their EcstoreObjectIO / EcstoreObjectOperations associated-type bounds are unchanged and the contract-compat tests still guard them. Method bodies are moved verbatim: a whitespace-insensitive token-stream diff of the two impl blocks (with their #[async_trait::async_trait] attributes) against the pre-move mod.rs source is byte-identical (5,754 tokens each) — NO visibility widening was required, because the impls reach SetDisks helpers and the P5 io_primitives through inherent self./Self:: calls that resolve across modules unchanged. The two inherent impl SetDisks blocks that sat between the trait impls (lock batch helpers) remain in mod.rs, verified present exactly once and uncorrupted. The issue's borrow/Arc-clone-avoidance optimization is intentionally deferred: it is a perf-sensitive change requiring the #738 benchmark and would risk the 'byte-level behavior unchanged' acceptance; this PR delivers the relocation. Verification: - cargo check / clippy -D warnings -p rustfs-ecstore --all-targets: clean - cargo test -p rustfs-ecstore --lib: 1841 passed, 0 failed - all five arch guard scripts: pass - token-stream diff of moved impls vs original: identical (mod.rs diff is a pure relocation; ObjectIO/ObjectOperations each defined exactly once post-move) |
||
|
Zhengchao An
|
3e4a7c1f6a |
fix(ecstore): lockstep EC stripe read to fix large-object GET EOF (#4289)
fix(ecstore): lockstep stripe read to stop EC GET desync truncation On the reconstruction-verifying GET path, ParallelReader::read used a data-first schedule: it read only `data_shards` readers per stripe and pulled in a parity reader as a substitute on demand. Because the shard readers are streaming (advanced only by being read, no seek), a parity reader first used mid-object was still positioned at its stream start (block 0) and returned an earlier stripe than the surviving data shards. Every shard passed its own bitrot hash, yet the set was mutually misaligned, so decode_data_with_reconstruction_verification correctly rejected it with "inconsistent read source shards" and the large-object GET truncated mid-stream (client "unexpected EOF"). Add read_lockstep(): on the verify_reconstruction path, read every live shard reader once per stripe and wait for all of them, so all readers advance one block per stripe and stay mutually aligned; any reader that errors is retired for the rest of the object (a stream that failed mid-block can no longer be trusted to be aligned). The adaptive data-first path is unchanged for non-verifying callers (e.g. heal). Adds a regression test reproducing a data shard that dies partway through a multi-stripe object; it must still reconstruct byte-exact output. Refs backlog#832. |
||
|
Zhengchao An
|
166679a723 |
refactor(ecstore): sink write + shared-read primitives into set_disk::core::io_primitives (backlog#820) (#4288)
* refactor(ecstore): sink write/rename/delete primitives into set_disk::core::io_primitives (backlog#820) P5 step 2 of the SetDisks God-Object split (tracking backlog#815, issue backlog#820; follows step 1 #4285). Relocate the entire set_disk/write.rs primitive family into the core/io_primitives.rs module home established by step 1: - Module items: dangling_delete_grace() + its env consts, and the OrphanDirScan scan-result enum. - impl SetDisks write/rename/delete primitives shared across mod.rs and the ops/ operation families: rename_data, commit_rename_data_dir, cleanup_multipart_path, rename_part, eval_disks, write_unique_file_info, update_object_meta(_with_opts), delete_if_dangling, delete_prefix, scan_orphan_dir, purge_orphan_dir_object, check_write_precondition, default_read_quorum, default_write_quorum. - The dangling_delete_grace unit tests move with their subject. set_disk/write.rs is removed and 'mod write;' dropped from mod.rs. Pure move + visibility adjustment, zero logic change. Method bodies are moved verbatim; pub(super) items are widened to pub(in crate::set_disk) so mod.rs / ops still reach them (write.rs's super was set_disk; the deeper module needs the explicit path to preserve identical reach). Callers use inherent self./Self:: calls, so no call sites change. Verification: - cargo check / clippy -D warnings -p rustfs-ecstore --all-targets: clean - cargo test -p rustfs-ecstore --lib: 1841 passed, 0 failed (moved dangling_delete_grace tests run as set_disk::core::io_primitives::tests::*) - all five arch guard scripts: pass - token-stream diff of moved block vs original write.rs: identical modulo visibility tokens (2713 tokens each) * refactor(ecstore): sink shared read primitives into set_disk::core::io_primitives (backlog#820) P5 step 3 (final) of the SetDisks God-Object split (tracking backlog#815, issue backlog#820; follows steps 1 #4285 and 2). Relocate the shared, low-level metadata/erasure READ PRIMITIVE methods out of set_disk/read.rs into the core/io_primitives.rs module home, leaving the object-read operation itself in read.rs. Moved into a new impl SetDisks block in io_primitives.rs (verbatim bodies): - read_parts, read_all_fileinfo (+ _observed / _inner / _full_wait / _early_stop variants), read_all_xl, load_file_info_versions_exact, read_all_raw_file_info, pick_latest_quorum_files_info, read_multiple_files. - The should_allow_metadata_early_stop free helper (called by the moved read_all_fileinfo_observed). Kept in read.rs: the object-read operation and its private helpers (read_version_optimized, get_object_fileinfo, get_object_info_and_quorum, try_get_object_direct_data_shards_with_fileinfo, get_object_with_fileinfo, get_object_decode_reader_with_fileinfo, build_codec_streaming_part_reader) and the metadata-cache helpers/tests. read.rs reaches the moved primitives through the SetDisks core (inherent self./Self:: calls; the sole cross-boundary edge, get_object_fileinfo -> read_all_fileinfo_observed, is handled by widening that one method to pub(in crate::set_disk)). Pure move + visibility adjustment, zero logic change. pub(super) primitives are widened to pub(in crate::set_disk); the three internal-only fanout variants (_inner/_full_wait/_early_stop) stay private; load_file_info_versions_exact stays pub(crate). Method bodies are moved verbatim. Verification: - cargo check / clippy -D warnings -p rustfs-ecstore --all-targets: clean - cargo test -p rustfs-ecstore --lib: 1841 passed, 0 failed - all five arch guard scripts: pass - token-stream diff of the three moved regions vs original read.rs: identical modulo visibility tokens, the new impl-block scaffolding, and rustfmt signature re-wrapping (read.rs diff is pure deletion, zero added lines) |
||
|
houseme
|
76124423a4 |
fix: stabilize s3-tests delete key-limit coverage (#4283)
* fix: tighten list handling and s3 test support * chore: tidy imports and metric updates * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zhengchao An <anzhengchao@gmail.com> * fix: address s3tests review follow-ups --------- Signed-off-by: Zhengchao An <anzhengchao@gmail.com> Co-authored-by: Zhengchao An <anzhengchao@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |