mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-07 13:53:12 +00:00
e279a4f48a6bc74f23cfffe8431249bb5504c4bc
166 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Zhengchao An
|
be2e454d9d |
test(ecstore): rename/commit fan-out pause barrier + background-task introspection (backlog#1325 block 2) (#4936)
test(ecstore): add rename/commit fan-out pause barrier and background-task introspection Second white-box test-infra block for https://github.com/rustfs/backlog/issues/1325 (the first block landed the per-disk call counters in PR#4914). Adds a `#[cfg(test)]` awaitable pause barrier plus in-flight background-task introspection to the rename/commit fan-out in `crates/ecstore/src/set_disk/core/io_primitives.rs`, following the same dual-cfg seam style as the existing `disk_call_counters` and `cleanup_fault_injection` seams. A test arms a barrier for `(object, disk_index, phase)`; the matching spawned fan-out task parks at its checkpoint until the test releases it, and the test awaits the pause through a deterministic `tokio::sync::Notify` handshake (no sleeps). A separate object-keyed task tracker reports how many rename/cleanup background disk tasks are still in flight, so a test can assert "a background disk write is still running" while paused and "no background disk write remains" once the fan-out drains. Both mechanisms live in one process-global registry keyed by object name, so concurrent tests using distinct object names stay isolated. Barriers are placed on the real `rename_data` fan-out (phase `rename`) and the `commit_rename_data_dir` old-data-dir cleanup fan-out (phase `cleanup`). In production the barrier compiles to an immediately-ready `#[inline(always)]` no-op future and the task guard to `()`, so the fan-out control-flow shape and behavior are unchanged; only the `#[cfg(test)]` variants touch the registry. Coordinator lock-holding is asserted by the test at the store/coordinator layer via the guard it already holds; io_primitives has no handle to that namespace lock. Cross-process/black-box fault injection (toxiproxy, blackhole peers, 2-pool) remains a later cluster-harness block. Serves the barrier-style white-box acceptances of #1312 (commit fencing: abort at the first-disk rename barrier, assert no background disk write remains after release), #1319, and #1313. Three demo tests drive the real fan-out functions and double as regression guards: neutralizing the barrier seam makes the pause await time out, and neutralizing the task guard pins the in-flight count at zero, so reverting either seam fails the demos. |
||
|
Zhengchao An
|
4d22ed4465 |
perf(capacity): drop per-PUT global lock and per-disk allocation from write dirty-scope (#4933)
perf(capacity): remove per-PUT global lock and per-disk allocation from write dirty-scope Every successful write recorded its capacity dirty scope by allocating an endpoint/path String per online disk, deduplicating through a HashSet, entering the global dirty-scope Mutex, and — in the app response path — taking a global async RwLock to record the write frequency. Under small-object high concurrency this created a global serialization point and O(disks) allocation on the hot path (https://github.com/rustfs/backlog/issues/1315). This change makes the steady-state write path allocation-free and lock-free without altering capacity accounting semantics: - Memoize the per-set dirty scope. Each set resolves its disks' immutable endpoint/path identity lazily into a slot-indexed cache and reuses a shared `Arc<CapacityScope>`; steady-state writes clone the Arc under a read lock instead of rebuilding String/HashSet. The heal path keeps an ad-hoc scope builder because it passes disks in erasure-distribution order rather than physical-slot order. - Add a monotonic generation to the global dirty-scope registry, advanced only when a non-empty drain removes disks. A set upgrades the global registry mutex only on the first write of each generation and then skips it while the generation is unchanged; the observed generation is read under the registry lock so a concurrent drain forces a re-mark, preventing lost updates. The write commits its bytes before recording the scope, so any drain that could remove the mark is ordered after the commit and the following refresh reads the committed bytes. - Replace the write-frequency `RwLock<WriteRecord>` with lock-free atomics: per-second CAS buckets, an atomic last-write timestamp, and an atomic total counter. The frequency window and debounce semantics the refresh scheduler relies on are unchanged. Capacity marking remains a conservative superset of the disks actually written, so admin/scan totals are byte-for-byte identical: extra dirty marks only trigger a re-read of a disk whose usage is unchanged. White-box tests assert the memoized scope equals the previous ad-hoc construction, that the global registry is upgraded exactly once per generation and re-marked after a drain, and that the lock-free write record is exact under concurrent contention. Ref: https://github.com/rustfs/backlog/issues/1315 |
||
|
Zhengchao An
|
b41bbe2db4 |
fix(ecstore): split rename_data signature from heal-convergence decision (#4926)
CompleteMultipartUpload enqueued a normal-priority heal whenever `rename_data` returned a `Some(versions)` signature. But the per-disk signature is produced for every object with <=10 versions, and a healthy quorum reduces to `Some` as well, so the `Option<Vec<u8>>` return value conflated two distinct facts — "a version signature exists" and "the committed replicas need heal". The result: nearly every healthy MPU completion self-enqueued a heal, while >10-version objects (signature `None`) did not — an algorithmic heal amplification on the healthy path (rustfs/backlog#1321). Replace the overloaded `Option<Vec<u8>>` second element of `SetDisks::rename_data` with an explicit `RenameConvergence` classification computed after the write-quorum gate: - AllSuccessIdentical — every attempted disk committed with an identical, known signature (no heal). - PartialCommit — write quorum met but a disk failed/offline; a committed replica is missing or stale (heal). - SignatureDivergent — all committed but signatures diverge, or mix signed (<=10-version) with unsigned (>10-version) disks (heal). - Unknown — all committed, no signature produced (>10 versions); latent divergence is left to the scanner backstop, not self-enqueued. `RenameConvergence::needs_heal()` is the single decision point. The version signature is now only comparison material; it no longer doubles as a heal flag. The old `select_rename_data_versions` / `reduce_common_versions` / `rename_data_versions_key` machinery that carried the conflation is removed. The heal submission in `complete_multipart_upload` moves off the ACK critical path into a detached task: it runs after the object lock is dropped and after the durable `rename_data` commit, survives cancellation of the completion future, and coalesces through the existing bounded / deduplicated / observable heal-channel admission (one submit per degraded completion, at most). A completion cancelled in the narrow window between the durable commit and reaching the enqueue is scanner-backstopped, as is the Unknown (>10-version) case. The PUT path (`object.rs`) binds the second element as `_` and is unchanged. The change is orthogonal to and composes with the #1312 commit fence on the same `rename_data` path (epoch rejection is a commit-gate failure surfaced through `Result::Err`, convergence is a post-commit signal); documented in docs/architecture/unified-object-generation.md. Tests: `classify_rename_convergence` white-box cases cover the full acceptance matrix (healthy 4/4 and 8/8, 3-same-1-divergent, failed/offline disk, no-common-quorum split, >10-version all-success and with-failure, mixed signed/unsigned) and fail on revert to the old "signature exists => heal" semantics. The decision function is tested directly rather than through the process-global heal channel, whose receiver is owned exclusively by the blackbox serial test (init_heal_channel is once per binary). Refs: https://github.com/rustfs/backlog/issues/1321 |
||
|
Zhengchao An
|
da0c2d3730 |
test(ecstore): per-disk call-counter registry for metadata fan-out (backlog#1325 block 1) (#4914)
test(ecstore): add per-disk call-counter registry for metadata fan-out First landable piece of the backlog#1325 test-infrastructure work: a test-only, per-disk call-counter registry that can observe `read_version` RPC counts recorded inside `tokio::spawn` tasks. This unblocks the RPC-count assertions in backlog #1309 / #1314 / #1315, which the thread-local `CapturingRecorder` cannot serve because it is blind to metrics emitted from spawned tasks. The new `#[cfg(test)]` module `disk_call_counters` (modeled on the existing `cleanup_fault_injection` seam) is a process-global registry keyed by object name; an RAII `observe(object)` scope collects per-disk counts and clears only its own on drop, so parallel tests using distinct object names stay isolated. A dual-`cfg` `SetDisks::record_read_version_call` seam records from inside both metadata-fanout `read_version` spawn sites for online disks only; the `#[cfg(not(test))]` variant is an empty `#[inline(always)]` fn, so production runtime behavior is unchanged. Two demo/regression tests prove the facility works across worker threads and is revert-detecting (neutralizing the recorder makes them fail). Refs: https://github.com/rustfs/backlog/issues/1325 |
||
|
Zhengchao An
|
ae15f5804d |
test(ilm): fix restore integration test object key to match transition filter (#4886)
* test(ilm): fix restore test object key to match transition filter restore_object_usecase_reports_ongoing_conflict_and_completion used the object key "restore/api-object.bin", but the shared set_bucket_lifecycle_transition_with_tier helper only transitions objects under the "test/" prefix. enqueue_transition_for_existing_objects therefore matched nothing and wait_for_transition timed out at 15s, failing the test deterministically. The test was added in #4860 but its ILM Integration (serial) lane is skipped on regular PRs, so it merged red and has failed on every main run since. Move the object under the test/ prefix like every passing sibling test in this file. * ci(ilm): exclude broken RestoreObject API test from serial lane restore_object_usecase_reports_ongoing_conflict_and_completion exposes a real regression, not a test bug: the RestoreObject copy-back (handle_restore_transitioned_object) now holds the object write lock added in #4877 across the entire tier read-back, so it never releases in time and the test's concurrent get_object_info times out with Lock(Timeout, 5s). The failure is deterministic and independent of the mock tier's injected latency. This is the same class of known-broken restore/transition failure already tracked under backlog#1148 (three sibling scanner tests are excluded here by name for the same reason), so exclude this one the same way until the restore copy-back path is fixed or the #4877 lock scope is revisited. The prior commit keeps its correct fix (the object key must live under the test/ transition prefix); that was masking this deeper issue by never letting the object transition in the first place. Restore copy-back deadlock/hang under the #4877 lock is escalated separately for a product-level decision (fix the copy-back vs. narrow/revert #4877). * test(ilm): fix scanner restore test object keys to match transition filter test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore and test_multipart_restore_preserves_parts_and_etag (both added in #4860) keyed their objects under restore/ instead of the test/ prefix that set_bucket_lifecycle_transition_with_tier filters on, so the objects never transitioned and wait_for_transition timed out at 15s. These surfaced only after the prior commit excluded the rustfs-side restore API test: nextest runs -j1 fail-fast, so that earlier failure stopped the run before these scanner tests executed. Unlike the excluded API test, both call restore_transitioned_object().await sequentially and only read afterwards, so they don't hit the concurrent-read-vs-#4877-write-lock timeout; the key prefix was their only problem. * ci(ilm): exclude the two remaining #4877-broken restore tests test_multipart_restore_preserves_parts_and_etag and test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore both call restore_transitioned_object().await, which since #4877 acquires the object write lock and deterministically times out (Lock Timeout, 5s) against an already-held lock, so restore never completes. They surfaced one at a time because nextest runs -j1 fail-fast. The earlier prefix fix was necessary but only advanced them from the transition wait to this restore-lock timeout. Exclude both by name alongside their already-excluded sibling test_transition_and_restore_flows (same root cause, tracked under backlog#1148) so the ILM Integration (serial) lane goes green. The #4877 lock scope still needs a product fix before any of these re-enable. * docs(ilm): describe the excluded restore tests' symptom as a lock timeout, not a deadlock The #4877 write lock is held across the tier read-back and outlives the 5s lock timeout; nothing proves a true deadlock. Wording flagged by Copilot review. * fix(ecstore): stop restore copy-back self-deadlocking on the #4877 write lock #4877 made handle_restore_transitioned_object hold the object write lock for the whole restore and forward no_lock=true so the set layer would not reacquire it. But the set-level copy-back rebuilds its own options (put_restore_opts -> ropts, and the complete_multipart_upload opts) that default no_lock=false, so the inner put_object / new_multipart_upload / complete_multipart_upload each re-acquire this object's write lock in their commit phase and block on the lock the restore already holds -> Lock(Timeout, 5s), and restore never completes. Confirmed via RUSTFS_OBJECT_LOCK_DIAG_ENABLE: restore_transitioned_object acquires the write lock, then holds it ~10.5s across two nested 5s acquire timeouts before failing. This is a real product deadlock: a RestoreObject on any transitioned object (multipart especially) hangs, not just the tests. Propagate no_lock into the copy-back options so the inner writes inherit the already-held lock. Use opts.no_lock (not a hardcoded true) so a caller that restores without the outer lock still locks correctly. put_object_part is left as-is: it locks the multipart upload-id resource, not the object key, so it does not conflict. Verified test_multipart_restore_preserves_parts_and_etag now passes (3.6s, was a 15s+ hang). * ci(ilm): re-enable multipart restore test; scope remaining exclusions The prior commit fixes the #4877 restore self-deadlock, so test_multipart_restore_preserves_parts_and_etag passes again - drop it from the serial-lane exclusion list and remove its 'currently excluded' note. The other restore/transition tests still fail, but each on a DIFFERENT, independent issue unrelated to the (now-fixed) lock, verified locally: - test_restore_chain_...: DeleteRestoredAction sets expire_restored but no delete path reads it, so cleanup deletes the whole object (unimplemented semantics), not the local restored copy only. - test_transition_and_restore_flows: transition xl.meta missing on one drive (EC metadata distribution), not restore. - restore_object_usecase_reports_ongoing_conflict_and_completion: asserts a concurrent mid-restore ongoing=true read that #4877's read-vs-restore serialization rules out (backlog#1148 ilm-8 criterion 1, an API-semantics decision). Comments and #[ignore] reasons updated to reflect each real cause. All remain tracked under backlog#1148. |
||
|
Zhengchao An
|
f78f146c35 | test(ecstore): extend crash-point injection to multipart complete and xl.meta update paths (#4853) | ||
|
cxymds
|
ea04c94204 | fix(ecstore): skip deferred readers without sources (#4836) | ||
|
houseme
|
750e5d15eb |
feat(checksums): add native S3 additional checksum support (#4805)
* feat(rio): wire XXHash3/64/128 and SHA-512 into ChecksumType (S2) Add the AWS 2026-04 additional checksum algorithms as base types in rustfs-rio's ChecksumType, covering every dispatch site (key, raw_byte_len, hasher, Display, from_string_with_obj_type, BASE_CHECKSUM_TYPES) so no path silently strips them. Derive BASE_TYPE_MASK from BASE_CHECKSUM_TYPES as the single source of truth, allocate the new base-type bits append-only above bit 9 to preserve the on-disk varint format, and add streaming hashers whose digest uses the S3 canonical big-endian encoding (seed 0). The new algorithms are COMPOSITE-only: an explicit FULL_OBJECT request is rejected and they are never routed through add_part()/can_merge(). A round-trip guardrail test asserts every base type survives all dispatch sites, failing loudly if a future algorithm is added but a match arm or the mask is forgotten. Refs rustfs/backlog#1254 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * test(rio): pin XXHash/SHA-512 digests to official vectors, big-endian (S3) Lock the byte order and seed of the new algorithms against the OFFICIAL upstream xxHash / SHA-512 empty-input test vectors (XXH3-64, XXH64, XXH3-128, SHA-512), in big-endian, so the stored and echoed checksum is byte-for-byte identical to what AWS SDKs (awscrt) compute — the interop correctness this feature hinges on. Add a non-empty regression lock (official "fox" vectors) that also asserts the encoded field is the standard-base64 of the raw digest. Refs rustfs/backlog#1255 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * test(rio): lock on-disk checksum round-trip and forward-compat degrade (S8) Cover the xl.meta varint (de)serialization for the new algorithms: to_bytes() -> read_checksums() must recover the value under the Display key for XXHASH3/64/128 and SHA512. Pin the rolling-upgrade contract that a node reading a future, unknown base-type bit degrades safely — skips the entry and returns without panicking or mis-decoding a length. Combined with the append-only bit allocation from S2, this protects mixed-version clusters. Refs rustfs/backlog#1260 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * feat(head): echo XXHash/SHA-512 additional checksums on HeadObject (S5) HeadObject with x-amz-checksum-mode: ENABLED now returns the XXHash3/64/128 and SHA-512 checksums that S3 stored, closing the head_object gap in #4800. s3s HeadObjectOutput has no typed field for these, so they are emitted as raw response headers via response.headers (the same mechanism RustFS already uses for tagging-count), keyed by ChecksumType::key(). The existing five typed algorithms are unchanged. Also carries the Cargo.lock update for the xxhash-rust dependency introduced in S2. Refs rustfs/backlog#1257 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(checksums): fail-closed on unknown checksum algorithm (S7) A. Harden unknown/unsupported checksum algorithms to fail closed instead of panicking. ChecksumMode::base() in the outbound S3 client (crates/ecstore/src/client/checksum.rs) previously did `panic!("enum err.")` for any mode without a concrete base algorithm (e.g. a bare ChecksumFullObject flag); it now falls back to ChecksumNone. Added unit tests proving base() never panics and hasher() returns Err for unsupported modes. rustfs-checksums FromStr already returns Err on unknown names; added a regression test asserting garbage/unknown names fail closed. B. Extend rustfs-checksums ChecksumAlgorithm with the AWS 2026-04 additional algorithms Sha512/Xxhash3/Xxhash64/Xxhash128. Updated FromStr, as_str, into_impl, name constants, the x-amz-checksum-* header constants and the HttpChecksum impls. Byte order/seed matches the server-side rustfs-rio spec: xxh3/xxh64 as u64 big-endian (8 bytes, seed 0), xxh128 as u128 big-endian (16 bytes), sha512 via sha2::Sha512. Added tests validating each digest against a direct library computation. MD5 stays intentionally rejected (PR #4513) and is left untouched. C. crates/ecstore/src/client/checksum.rs ChecksumMode is enumset repr="u8" with 7 variants already consuming 7 bits; adding the 4 new algorithms would overflow u8 and require a breaking repr change, so ChecksumMode is left unchanged. The new algorithms are available through the rustfs-checksums ChecksumAlgorithm path. Refs rustfs/backlog#1259 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * feat(get,put): echo XXHash/SHA-512 checksums on GetObject and PutObject (S5-GET, S4) Complete the additional-checksum round-trip so AWS SDKs can verify integrity on download and confirm it on upload: - GetObject with x-amz-checksum-mode: ENABLED now returns XXHash3/64/128 and SHA-512 checksums (the download-side path SDKs auto-verify). The values flow from build_get_object_checksums through GetObjectOutputContext into finalize_get_object_response and are emitted after wrap_response_with_cors. - PutObject echoes the server-computed additional checksum on its response, captured at the want_checksum set points before opts is moved. Both reuse a single centralized helper, inject_additional_checksum_headers, which HeadObject now also uses. This is the ONLY place that emits these headers, so when s3s gains typed fields for these algorithms the migration is one spot (fill the typed field, drop the insert) with no risk of duplicate headers. The five s3s-typed algorithms are unchanged. Trailing-checksum PUT echo (value lands after the body) is left for e2e coverage in S10. Refs rustfs/backlog#1257 rustfs/backlog#1256 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * feat(multipart): support XXHash/SHA-512 composite multipart checksums (S9) Make multipart uploads work end-to-end for the composite-only algorithms (XXHash3/64/128, SHA-512): - complete_part_checksum previously returned the outer None for any algorithm outside the five typed ones, which failed CompleteMultipartUpload with InvalidPart. It now accepts any valid base type with no double-check value (Some(None)) — mirroring the missing-value path of the typed algorithms — since s3s CompletePart has no field to carry a client-supplied per-part value and the part was already verified server-side at UploadPart. Genuinely unset/invalid types are still rejected. - The existing COMPOSITE assembly (Checksum::new_from_data over the concatenated per-part raw digests; full_object_requested() is false so add_part() is correctly bypassed) already works for these algorithms via the S2 wiring. A rio test locks the assembly and that add_part refuses them. - UploadPart and CompleteMultipartUpload echo the new-algorithm checksum on their responses via the shared inject_additional_checksum_headers helper (now pub(crate)), since s3s has no typed output field. Refs rustfs/backlog#1261 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * feat(rio): add MD5 as an additional checksum (x-amz-checksum-md5) (S6) Wire MD5 into ChecksumType as an additional (flexible) checksum, distinct from the legacy Content-MD5 / ETag path: header x-amz-checksum-md5, 16-byte digest, COMPOSITE-only, md-5 hasher. Pinned to the official empty-input MD5 vector. Thanks to the single-source-of-truth wiring from S2, every dispatch site (GetObject/HeadObject/PutObject echo, multipart complete_part_checksum and the COMPOSITE assembly) picks MD5 up automatically via base()/key()/the catch-all arm — no handler changes needed. Tests are extended to cover MD5 across them. Coordination with #4513: that PR made the OUTBOUND rustfs-checksums client reject "md5" so it could never silently fall back to CRC32. This change is on the server-side rio path and never falls back — it implements MD5 correctly rather than substituting another algorithm — so the #4513 intent is preserved, and the outbound client keeps rejecting md5 (S7). Refs rustfs/backlog#1258 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * perf(rio): drop per-request to_uppercase alloc in checksum parsing (S11) from_string_with_obj_type ran alg.to_uppercase() on every checksummed request, allocating a String just to compare against a fixed set of algorithm names. Replace it with eq_ignore_ascii_case, which is allocation-free and, for the ASCII algorithm names involved, exactly equivalent. A test locks that case-insensitivity, the CRC64NVME full-object assumption, composite-only FULL_OBJECT rejection, and unknown/empty handling are all unchanged. The other S11 notes are intentionally not acted on: the Phase-0 header scan is N/A (we chose full support over rejection, so there is no reject guard), and parallelizing the serialized hash passes is deferred pending a measured need. Refs rustfs/backlog#1263 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * refactor(checksums): collapse 5 duplicated response-checksum loops into one Review of the accumulated commits found the same "iterate decrypted checksums, match five typed algorithms, drop the rest" loop copy-pasted across five response paths (GetObject, HeadObject, GetObjectAttributes object-level and part-level, CompleteMultipartUpload). That was patch-on-patch duplication. Collapse it into a single source of truth: - rustfs-rio gains ChecksumType::is_s3s_typed() — the one place that defines the five-typed vs additional-algorithm split. - object_usecase gains ResponseChecksums + classify_response_checksums(), which performs the typed/extra split once. All five call sites now destructure its result; additional_checksum_echo_pairs() also uses is_s3s_typed() instead of a hand-rolled five-way comparison. Behaviour is unchanged (GetObjectAttributes still cannot surface the additional algorithms — an s3s XML-body limitation, now documented in one spot). One pass over the map; extra pairs pushed only when a new-algorithm checksum is present. Refs rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * test(checksums): unit tests for classifier/echo helpers + fix unused import Add direct unit tests for the refactored single-source-of-truth helpers: - rio ChecksumType::is_s3s_typed() — exhaustive typed-vs-additional split, and that flags (FULL_OBJECT/MULTIPART) on a base type don't change classification. - object_usecase classify_response_checksums() — typed fields vs `extra` headers, the checksum-type marker, and empty input. - additional_checksum_echo_pairs() — echo pair only for additional algorithms, none for the five typed ones, none for None. - inject_additional_checksum_headers() — writes all pairs; empty is a no-op. Also drop the now-unused AMZ_CHECKSUM_TYPE import in multipart_usecase.rs left by the classifier refactor (would fail the -D warnings gate). Refs rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * style(rio): fix typo flagged by CI (mis-decoding -> decoding a wrong length) The Typos CI check flagged "mis-decoding" (it reads "mis" as a word). Reword the S8 forward-compat comment; no code change. Refs rustfs/backlog#1260 Co-Authored-By: heihutu <heihutu@gmail.com> * test(e2e): integration test for XXHash/SHA-512/MD5 additional checksums (S10) Permanent verify-on-write integration test in the e2e suite for the AWS 2026-04 additional algorithms. aws_sdk_s3 has no typed builder for these, so the x-amz-checksum-<algo> header is injected via mutate_request (value from rustfs-rio, byte-for-byte identical to awscrt). Uses a client with automatic checksum calculation disabled (request_checksum_calculation=WhenRequired) so the injected header is the only checksum on the wire. For each of XXHash3/64/128, SHA-512 and MD5: a correct value is accepted and the object stored intact; a mismatched value is rejected with BadDigest and nothing is stored. Verified passing locally (1 passed) alongside a boto3+awscrt round-trip that additionally confirms the HEAD/GET header echo (14/14). Refs rustfs/backlog#1262 rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> * style(get): allow too_many_arguments on finalize_get_object_response The classifier refactor added an extra_checksum_headers parameter, pushing finalize_get_object_response to 8 args and tripping clippy::too_many_arguments under CI's `-D warnings`. Add the same #[allow] the sibling GET helpers already carry; no behavior change. Refs rustfs/backlog#1252 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
1553dc3f62 |
Address P2 follow-ups from the 2026-07-10..12 merged-PR review (backlog#1210-1220) (#4783)
* fix(obs): open cleaner compression source with O_NOFOLLOW The compressor opened the source log via File::open, which follows a symlink at the final path component. Between the scanner selecting a regular file and this open, an attacker with write access to the log directory could swap the entry for a symlink (TOCTOU) pointing at, say, /etc/shadow, whose contents would then be copied into an archive. Open the source with O_NOFOLLOW on Unix so such a swap fails with ELOOP; the temp/archive path already refused symlinks, this closes the source side. Refs rustfs/backlog#1210 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs): recompress instead of trusting leftover cleaner archives archive_header_ok only checked the first 2-4 magic bytes before treating an existing .gz/.zst as a completed prior result and letting the caller delete the source log. A file with valid magic but a truncated or forged body passes that check, so an attacker with write access to the log directory (or a crashed prior run) could plant such a stub and make the cleaner delete the real log without ever producing a usable archive — silent audit-data loss. Chosen fix: stop trusting cross-process leftovers entirely and always recompress the source in this pass, rather than fully decoding every leftover to validate it. Full-decode validation would add real CPU cost and decode-bug surface for a rare crash-recovery case; the existing atomic create_new+rename already overwrites whatever sits at the archive path (a planted symlink is replaced, never followed) with a freshly written, fsync'd archive, so a partial/forged leftover can never gate source deletion. This is the lowest-regression option. Refs rustfs/backlog#1211 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): cap memory-gate reservation at cache growth headroom The memory gate subtracts `admitted_since_refresh` from the snapshot's available bytes so a burst arriving faster than the 5 s refresh cannot over-allocate. That counter is GROSS: it only rolls over on the refresh and never rolls back when a fill is later evicted, cancelled, or loses the invalidation race. Under sustained high-throughput churn (net footprint flat and far below `max_capacity`) the raw counter balloons past the memory the cache actually holds, so `effective_available` collapses and the gate reports false memory pressure — skipping the hottest fills with SkippedMemoryPressure until the next 5 s refresh. This only lowers hit rate; it never returns wrong data and self-heals each refresh. Fix direction 1 (minimal regression): cap the reservation deduction at the cache's own growth headroom (`max_capacity - weighted_size()`) instead of letting the unbounded gross counter shrink the system-available budget. The cache can never hold more than `max_capacity`, so a burst adds at most that headroom of real memory before moka evicts to stay bounded (net-zero churn beyond that point) — capping the deduction there keeps the reservation honest without treating gross churn as growth. Chosen over net-accounting (direction 2, releasing bytes on every failure/cancel/eviction path) because that only plugs the leak on failed fills and would not address the core defect: churn of *successful* insert/evict fills over the 5 s window. It also touches only the gate plus one call site rather than every failure path in moka_backend. The cap only ever raises `effective_available`, so real memory pressure (a low snapshot at refresh) still suppresses fills; when the cache is at capacity the headroom is 0 and the deduction vanishes, correctly reflecting net-zero churn. `MokaBackend` now stores `max_capacity` and passes the live headroom into `allows_fill`. Adds targeted gate tests: gross churn far above headroom no longer falsely suppresses, yet the reservation still bounds a burst while the cache can genuinely grow. Refs rustfs/backlog#1212 Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): assert native O_DIRECT path runs in uring read test uring_preserves_o_direct_for_eligible_reads only compared bytes through LocalDisk::read_file_mmap_copy. On a filesystem that rejects O_DIRECT the read silently degrades to the buffered StdBackend fallback and the byte check still passes, so the test could go green without the native read_at_direct path ever executing -- a vacuous pass. Add a per-disk native_direct_reads counter on UringBackend, incremented only when pread_uring_direct completes, and rebuild the test to drive a real UringBackend's pread_bytes and assert the counter is non-zero (every eligible read went through the native tier). When io_uring or O_DIRECT is unavailable on the host filesystem (restricted CI runners, tmpfs), the test skips loudly via eprintln instead of asserting a tautology, while still checking byte-correctness on whatever tier served the read. The counter also gives a gray release a positive signal that the O_DIRECT tier is serving reads, not just a fallback count. Refs rustfs/backlog#1213 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): warn + count read-time EINVAL on native O_DIRECT reads classify_direct_read_error is only reached from the read side: the O_DIRECT open in pread_uring_direct already succeeded (an open-time refusal is handled earlier as DirectOpenError::ODirectRefused). So an EINVAL/EOPNOTSUPP arriving here is a read-time error on an fd the kernel accepted for O_DIRECT -- far more likely an alignment bug in the aligned read path than an unsupported filesystem. The old code latched the disk's native path off with only a once-per-disk debug trace, hiding a potential correctness regression behind a silent buffered-read downgrade. Diagnostics only: the fallback behaviour is unchanged (the native path is still latched off and the caller still reads via StdBackend). This adds a rustfs_io_uring_direct_read_einval_total counter and promotes the once-per-disk trace from debug to warn so an operator can see an alignment regression instead of an unexplained latency/CPU shift. Refs rustfs/backlog#1214 Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): document data-blocks-first default and its tail-latency cost DEFAULT_RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP is true and must stay true: deferred-parity is the deliberate, already-rolled-out full-object GET default from backlog#1159/#923. Flipping it back to false in code would silently revert that rollout for every deployment that has not set the env var, so this commit only documents -- no behaviour change. The added notes explain what data-blocks-first does (schedule data shards up front, engage parity lazily on a missing/corrupt data shard), the known trade-off (parity is engaged late, so a slow-but-not-dead data drive raises GET p99 because the faster parity shards are not raced against it until a data shard is declared missing), and the operational rollback switch (RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP=false), which is intentionally an env override rather than a code default change. No metric was added: the low-risk observability hook for "slow data drive engaged deferred parity" would live at the deferred-stripe engage point, which is out of this file's scope; this change stays documentation-only to avoid touching the hot GET path. Refs rustfs/backlog#1215 Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): document wide-directory walk stall hazard and tuning list_dir enumerates a whole directory in one os::read_dir call (count = -1), and the walk caller bounds that entire enumeration with the per-read stall budget (default 5s) as if it were a single read. For a wide, flat prefix -- one directory holding millions of immediate children -- a single readdir can exceed the budget on a healthy disk, trip DiskError::Timeout, and surface as a ListObjects 500 quorum failure though the drive is fine (a #2999 sub-class). This is documented, not rewritten: turning the one-shot readdir into a streaming/batched enumeration that refreshes the stall deadline between chunks is an architecture-level change with high regression surface (ordering, the count contract, quorum merge) and belongs in a separate follow-up. The supported mitigation today is operational, so the comments point wide-directory deployments at RUSTFS_DRIVE_WALKDIR_STALL_TIMEOUT_SECS and the high-latency drive-timeout profile, which widen the budget with no code change. Notes were added at list_dir, the scan_dir call site, and get_drive_walkdir_stall_timeout. No behaviour change. Refs rustfs/backlog#1216 Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): document consumer-peek vs producer-stall coupling In list_path_raw the consumer's peek_timeout is drawn from the same source and same value (walkdir_stall_timeout, default 5s) as the producer-side walk stall budget, but the two measure different things: the producer stall bounds a single drive read, while the consumer peek bounds the gap between two ADJACENT entries arriving from a reader. Because they share a value, the consumer cannot wait meaningfully longer for the next entry than the producer is allowed to spend producing one. Walking a region dense with non-listable internal items can make a HEALTHY drive miss the budget between visible entries; the consumer then declares it stalled and detaches it, dropping a good drive from the merge and capping the "large prefix succeeds" guarantee. Documented, not decoupled: giving the consumer peek an independent, strictly-larger budget would cut these false detaches but equally delays detaching a genuinely dead drive and shifts listing tail-latency semantics, so it wants soak data before changing the default. The comment records the invariant any such follow-up must keep -- consumer peek >= producer stall, never stricter -- so it can never fail a drive before the producer would. No behaviour change. Refs rustfs/backlog#1217 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(io-metrics): add time-based trigger for low-IOPS latency percentiles Percentiles were recomputed only every 128 IOs and seeded to 0, so a low-traffic deployment exported p95/p99 = 0/stale for a long time after startup. Add a 10s wall-clock trigger alongside the count throttle so the first recompute can fire before 128 samples accrue. Hot-path per-op mean update is unchanged. Refs rustfs/backlog#1218 Co-Authored-By: heihutu <heihutu@gmail.com> * test(e2e): cover codec-streaming parity under fault injection and NoSuchKey The codec-streaming compat A/B previously ran only against a healthy 4-disk EC set with successful full GETs: the DiskFaultHarness was constructed but never faulted, the error path was untested, and the range assertion silently compared legacy-vs-legacy (ranges always fall back to the duplex path), overstating what it proved. Add two genuinely-failable scenarios reusing the existing harness and fixtures: - Parity reconstruction A/B: take one data disk offline and re-run the full object matrix on both phases while the EC 2+2 set rebuilds each large object from the surviving shards. Assert codec == legacy byte-for-byte (sha256) and header-for-header, and assert the codec phase served the reconstructed objects with zero duplex-pipe fallback (the reader gate is drive-health-independent, so the codec fast path is really exercised through reconstruction). - NoSuchKey negative path: compare the HTTP status + S3 error code of a missing-key GET across the legacy and codec phases and require them to be identical (404/NoSuchKey), guarding against the codec env perturbing the error path. Also clarify the range-phase comment so it is not misread as codec-range correctness coverage: both sides are served by the same legacy range path, so the assertion only proves ranges keep working and keep falling back to legacy with the gates open. Verified: cargo check/--no-run pass and the test passes locally (1 passed; dup_codec=0 confirms the codec path ran). Refs rustfs/backlog#1219 Co-Authored-By: heihutu <heihutu@gmail.com> * ci(ecstore): exercise native O_DIRECT read path on an ext4 loopback The uring-integration leg ran on the runner's default TMPDIR, which may sit on tmpfs/overlayfs where open(O_DIRECT) fails and the native read_at_direct path silently latches off to the aligned StdBackend fallback. Mount a dedicated ext4 loopback and point TMPDIR at it so the real io_uring dep (bumped git->0.1.0->0.2.0->0.2.1) and the native O_DIRECT read path are actually covered rather than validated only by signature diffing. Refs rustfs/backlog#1220 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
63b4568f85 |
fix(ecstore): reclaim orphan data dirs on the healthy heal path (#4781)
PR #4356 wired `reclaim_orphan_data_dirs` only into `heal_object`'s post-heal tail, which runs after the `disks_to_heal_count == 0` early return. That early return is exactly the state of the objects the sweep targets: a valid `xl.meta` with all shards present plus a leaked pre-#3510 data dir needs no shard healing, so a healthy heal returned before reclaim and swept nothing. On a healthy deployment (single node, no degraded disks) the reclaim was therefore dead code — an admin heal walked the objects, "healed" them, and reclaimed no leaked space. Run the best-effort reclaim on the `disks_to_heal_count == 0` path as well, gated on `!opts.dry_run`. The shared match+log block is factored into `reclaim_orphan_data_dirs_best_effort` so both exits behave identically. A reclaim failure still never fails the heal. Adds an end-to-end regression: put a healthy non-inline object, plant an unreferenced UUID data dir under it on every disk that holds the object, then drive `heal_object`. A dry-run heal must leave the stray in place; a real heal must reclaim it while preserving the live data dirs, `xl.meta`, and object contents. The test fails against the pre-fix control flow. Refs #3231, #3191, #4356. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
13bdca6762 |
build(toolchain): switch Rust channel to stable (#4775)
* Change Rust toolchain channel to stable Signed-off-by: houseme <housemecn@gmail.com> * style: apply clippy --fix and cargo fix lint suggestions Run `cargo clippy --fix --all-targets --all-features` and `cargo fix --lib --all-targets` across the workspace, then resolve the remaining warnings by hand: - collapse needless borrows in `format!` args, prefer `?` over explicit early returns, and use `.values()` / `.flatten()` iterator adapters - rewrite the `Md5` scan loop via `manual_flatten` and re-indent the `select!` macro body (rustfmt skips macro interiors) - annotate the intentional dead-code `Md5` inherent methods (constructed only by the test factory) with `#[allow(dead_code)]` Behavior is unchanged. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Signed-off-by: houseme <housemecn@gmail.com> Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
6886dca7d1 |
feat(ecstore): make GET codec-streaming a single rollout switch (backlog#1183) (#4752)
backlog#1183, staged rollout step. Simplify enabling the zero-duplex GET
codec-streaming fast path to a single switch — RUSTFS_GET_CODEC_STREAMING_ROLLOUT
(default "off") — now that body/header parity is proven (parity e2e net + bench
A/B on backlog#1183).
- Add a clean production rollout token "on" (aliases "full"/"production"); the
legacy "internal"/"benchmark" tokens remain accepted.
- Flip DEFAULT_RUSTFS_GET_CODEC_STREAMING_ENABLE and the two ..._COMPAT_CONFIRMED
defaults to true. They are retained as emergency kill-switches (set any to
false to force the fast path off) but no longer gate enablement — the rollout
switch does.
No production behavior change: with no env set the rollout switch defaults to
"off", so GET stays on the legacy duplex path exactly as before. Flipping the
hard default to on is deferred to a follow-up after a production soak.
Note (intentional semantics change): with the rollout switch opted in
("on"/"internal"/"benchmark"), codec streaming now activates without also
setting the two ..._COMPAT_CONFIRMED vars — compatibility is confirmed, so those
confirmations are baked in.
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
houseme
|
264b2dd480 |
perf(metrics): drop needless per-emission work on hot metric paths (#4743)
* perf(metrics): drop needless per-emission work on hot metric paths Audit of the metrics hot paths surfaced four low-risk wins where emission did work it did not need to: - `record_file_cache_reclaim_success/error` (disk/local.rs) called `.to_string()` on `kind` (already `&'static str`) and on the `"ok"`/`"err"` literals, heap- allocating up to four `String`s per page-cache reclaim window — which runs per read-stream reclaim. The `metrics` macros accept `&'static str` label values directly, so pass them as-is. - `record_read_repair_dedup` (set_disk/core/io_primitives.rs) likewise `.to_string()`-ed an already-`&'static str` `reason`. - `SetDisks::get_object_reader` (set_disk/ops/object.rs) captured `Instant::now()` and emitted the `rustfs.lock.acquire.*` counter and histogram unconditionally on every GET, right beside an already-gated stage timer. Gate them behind `get_stage_metrics_enabled()` too, so an inactive observability config pays no per-GET clock read or recorder lookups. - The per-response-body-chunk counter in server/http.rs re-ran the `counter!` registry lookup on every chunk (a streamed GET emits many). Resolve the label-less handle once into a `LazyLock<metrics::Counter>`; the global recorder is installed at startup before any response streams, so the cached handle binds to the final recorder. No metric names or label values change. The only behavior change is that the `rustfs.lock.acquire.*` GET-path metrics now follow the GET stage-metrics flag, consistent with the neighbouring stage timings. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(metrics): gate page-cache reclaim metrics behind metrics_enabled() `record_file_cache_reclaim_success/error` run per read-stream reclaim window on large-object reads and emitted unconditionally. When general metrics are disabled the `counter!`/`histogram!` macros still construct three metric keys per call for nothing. Skip the emission behind `rustfs_io_metrics::metrics_enabled()`, matching how the io-metrics free functions self-gate. The serial reclaim-metrics test now enables the flag (save/restore) alongside the existing stage gate. Left ungated deliberately: `record_read_repair_dedup` (rare read-repair path, and its non-serial test would need a global-flag toggle), and the HTTP body-chunk counter (its cached handle already makes the disabled case a no-op increment). Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
e742a540a4 |
test(cache): guard the body-cache eligibility gate against deny-list regressions (#1146) (#4742)
`full_object_plaintext_len` decides whether a body-cache hook hit may serve bytes in place of the erasure read. It is a fail-closed allow-list: it excludes every read whose `ReadPlan::build` applies some other transform (ranged/part, raw/data-movement, restore, encrypted, remote) with an early `return None`, then returns a `Some(..)` length only for the whole-plaintext cases. A newly added `ReadPlan` branch that nobody teaches this gate about falls through to `None` and safely bypasses the cache. Flip it to a deny-list and the same new branch silently serves bytes in the wrong representation — the backlog#1108 / #1109 / #1146 class of bug. The existing unit and e2e tests only cover the branches that exist today. This adds `scripts/check_body_cache_whitelist.sh`, a structural guard wired into pre-commit / pre-pr / dev-check and CI, that asserts every exclusion predicate and a `return None` still precede the first `Some(..)`. Reordering a predicate, dropping one, moving the positive return ahead of the gate, or renaming/removing the function all fail; wording, formatting, and adding a new exclusion in the same gate do not. Mutation-tested against all four regression shapes. This machine-enforces the structural invariant that backlog#1146 was kept open to guard by hand. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
9d64d71bd7 | fix(ecstore): reduce GET reader setup shard fanout (#4735) | ||
|
houseme
|
6780140318 |
fix(object-data-cache): make the GET key write-unique and dedup the lookup (#4693)
fix(object-data-cache): make GET body cache key write-unique and dedup lookups Address four object-data-cache GET-path findings (backlog#1107 batch): ODC-06 (backlog#1111): the cache key was content-unique, not write-unique. Extend ObjectDataCacheKey with the resolved version's modification time (i128 unix nanoseconds, None -> 0), derived once in the shared planner so the ecstore hook and the usecase layer produce an identical key. An unversioned overwrite advances mod_time, so a stale node can no longer serve old bytes for up to the TTL under an MD5 collision; etag + size stay as belt-and-braces. ODC-16 (backlog#1121): every cacheable GET planned and looked up twice (once in the ecstore hook, once in the usecase layer), double-counting hits, hit_bytes and lookups. GetObjectReader now carries a GetObjectBodySource marker (Unprobed / HookMissed / HookServed); the hook stamps it, and build_get_object_body_with_cache serves a hook-served body directly and skips its lookup whenever the hook already probed. One hook-served GET now records exactly one lookup. ODC-19 (backlog#1124): ENABLE=true with no explicit mode defaulted to HitOnly, which never fills and keeps a permanent 0% hit rate. Default to FillBufferedOnly, log the resolved mode at startup, and warn when HitOnly is selected explicitly. ODC-24 (backlog#1129): max_entry_bytes above the in-memory GET fill limits was silently inert. Clamp the planner's size eligibility to min(max_entry_bytes, seek-support threshold, 64 MiB buffer cap) so ineligible sizes plan SkipTooLarge instead of being reported eligible, and warn at startup when the excess is inert. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
c2362bca14 |
perf(ecstore): slice in-memory shards instead of copying them twice (#4687)
* perf(ecstore): slice in-memory shards instead of copying them twice (backlog#1159)
The GET path reads a shard out of the page cache into a `Bytes`, then
`open_disk_reader` erased it behind `Box<dyn AsyncRead>` by wrapping it in
a `Cursor`. Downstream, `BitrotReader` could only get it back by copying:
once out of the `Cursor` into its scratch buffer, and once from there into
the caller's buffer. CPU profiling of a cached 1 MiB GET (device reads = 0)
attributed 8.23% of the whole server to `Cursor::poll_read` alone — a copy
of data that was already sitting in memory.
Keep the source concrete instead of erasing it. `ShardReader` is an enum of
`InMemory(Cursor<Bytes>)` and `Stream(Box<dyn AsyncRead ...>)`, and the new
`ShardSource::try_take_block(n)` lets an in-memory source hand over the
`[hash][data]` block as a slice. `read_appending` uses it to verify the hash
on the slice and `extend_from_slice` the shard straight into the caller's
buffer: one copy instead of two.
`try_take_block` defaults to `None`, so a streaming source keeps the old
path byte for byte, along with its short-read and EOF semantics. A source
that cannot serve `n` bytes declines rather than truncating, so a partial
block still becomes UnexpectedEof rather than a short shard. The hash is
still checked before anything is appended, so a corrupt shard never reaches
the caller's buffer on either path. The deferred parity reader opens its
source lazily and stays on the streaming path; parity is only read when a
data shard fails.
Tests gate equivalence and non-vacuity:
* `try_take_block` fires for `Cursor<Bytes>`, advances the position exactly
as a read of the same length would, declines when fewer than `n` bytes
remain, and returns `None` for a non-`Bytes` source — without this the
equivalence test below would silently compare one path against itself;
* both paths return identical bytes for the same shard;
* a corrupt shard fails on the fast path too, appending nothing.
Verified: clippy --tests -D warnings clean; `erasure::` 215 passed, 0 failed;
`set_disk::core::io_primitives` 49 and `io_support::` 22 pass.
Stacked on #4681 (`read_appending`), which this builds on.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(ecstore): implement ShardSource for Cursor<&[u8]> used by the erasure bench
`crates/ecstore/benches/erasure_benchmark.rs` builds
`BitrotReader<Cursor<&[u8]>>`, which the new `ShardSource` bound on
`ParallelReader`/`decode` does not accept. `cargo clippy --tests` does not
compile bench targets, so this only surfaced in CI's `--all-targets` run.
A borrowed slice carries no `Bytes` to hand out, so it takes the default
`try_take_block` and keeps the old streaming copy path — no behavior change.
Verified with the same target set CI uses:
`cargo clippy -p rustfs-ecstore --all-targets -- -D warnings` clean.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
Zhengchao An
|
80ddd8fa7e |
fix(ecstore): roll back delete on disks that staged then errored (#4676)
fix(ecstore): roll back delete on disks that staged then errored (backlog#1158) #4300 rolls back a failed delete only on disks that returned Ok, skipping any disk that staged its rollback backup, applied the delete, and then errored -- leaving that disk deleted while its peers are restored. On rollback, fan the undo out to every online disk instead; the disk-side restore_delete_rollback is already idempotent (Ok no-op when nothing was staged), so unstaged disks are unaffected. The err.is_some() skip now applies only to the success/cleanup path. Covers both single-object and batch delete. Refs backlog#1158. |
||
|
houseme
|
8039a4ceae |
test(cache): end-to-end regressions for the body-cache hook P0s (#4675)
fix(ecstore): make body-cache hook re-registrable + add e2e regressions ODC-21 (backlog#1126): the GET body-cache hook lived in a first-wins OnceLock. When AppContext is rebuilt (config reload, test re-init) a fresh ObjectDataCacheAdapter is constructed and re-registered, but the OnceLock kept ecstore's GET probe pointed at adapter #1 while every usecase-layer fill and invalidation targeted adapter #2 — silently degrading the feature to a 0% hit rate with no error, log, or metric, and stranding entries in the unreachable cache until their TTL. Replace the slot with RwLock<Option<Arc<dyn GetObjectBodyCacheHook>>> so re-registration atomically swaps to the newest adapter, and log at WARN when a swap replaces a *different* instance (Arc::ptr_eq). RwLock over ArcSwapOption because arc-swap's RefCnt is impl<T> (Sized, thin *mut T) and cannot hold an Arc<dyn Trait> without a sized newtype wrapper; the probe reads the slot once per full-object GET but only clones an Arc, negligible next to the metadata quorum fan-out already done before the probe. Add a test-only clear_get_object_body_cache_hook so tests register/unregister deterministically. With the hook now re-registrable, add true end-to-end regressions that drive get_object_reader (not the full_object_plaintext_len predicate) against a real erasure-coded, genuinely-compressed object via the blackbox make_local_set_disks harness, with a stand-in hook playing the app-layer cache (the injection point production uses; the adapter itself lives above ecstore). These close the gap the predicate-only tests left — a caller that opens a new shortcut serving the cached body directly, the original form of both P0s: - backlog#1108: a raw_data_movement_read must yield the STORED (compressed) bytes, never the cached plaintext. - backlog#1109: a compressed cache hit must publish the DECOMPRESSED length as object_info.size (the UploadPartCopy invariant), with the streamed length matching. - backlog#1146: a restore read (restore_request.days) must serve STORED bytes, not the cache. Mutation-verified each e2e test bites: dropping the raw_data_movement_read gate serves plaintext (fails #1108); removing the hit-site size republication publishes 2972 vs 660000 (fails #1109); dropping the restore gate serves plaintext (fails #1146). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
8c76efead2 |
fix(cache): stop the GET body-cache hook from bypassing ReadPlan (#4654)
* fix(cache): gate GET body-cache hook to preserve ReadPlan output The ecstore GET body-cache hook serves cached full-object plaintext directly, bypassing ReadPlan/ReadTransform. That is only sound when the normal read path returns that same plaintext byte-for-byte. Two probe conditions were missing, plus two usecase-layer planner gaps. ODC-01 (backlog#1108): raw/data-movement reads. ReadPlan::build returns the STORED representation for raw_data_movement_read (e.g. compressed bytes, length = oi.size), but the cache holds the post-decompression body. Decommission (raw_data_movement_read: true) would receive decompressed plaintext where raw compressed bytes are required, silently corrupting the destination pool. ODC-02 (backlog#1109): compressed objects. ReadTransform::Compressed rewrites object_info.size to the decompressed length; on a hook hit object_info is returned unchanged, so object_info.size is the compressed size while the stream carries the decompressed body. UploadPartCopy then uses src_info.size as the copy length and truncates the part. Fix: gate the hook probe with should_probe_body_cache_hook, refusing raw_data_movement_read, data_movement, and compressed objects, mirroring the conditions get_small_object_direct_memory_decision already applies. ODC-33 (backlog#1138): build_get_object_body_cache_plan lacked the is_remote() exclusion the ecstore hook enforces; add it so transitioned (remote-tier) objects are excluded uniformly. ODC-C1 (backlog#1142): zero-length bodies save no I/O (ecstore returns an empty body before the hook probe) yet the planner admitted them; change the guard to response_content_length <= 0 so they plan Skip, mirroring should_buffer_get_object_in_memory_with_threshold. Tests: body_cache_hook_gate_tests (4) cover plain-probe plus raw/data-movement/compressed skips; planner gains plan_skips_remote_transitioned_objects and plan_skips_zero_length_objects. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): allow compressed bodies via a fail-closed read allow-list The body-cache hook probe was gated by a deny-list that refused compressed objects outright, which cost the cache every compressed body — a growing share of stored data. Replace it with an allow-list that returns the exact plaintext length a hit may serve, or None. full_object_plaintext_len() answers a single question: would the normal ReadPlan produce this object's complete plaintext, and under which size? Compressed objects now qualify, and the hit site publishes the returned length as object_info.size, reproducing the contract ReadTransform:: Compressed establishes. A hit whose body length disagrees is refused and falls through to the erasure read. This also closes a gate the deny-list only covered by accident: a restore read forces ReadPlan down the Plain branch, so a compressed object yields STORED bytes under its compressed size. Refusing compressed objects hid that; admitting them exposes it, so restore reads are refused explicitly. Being fail-closed, a newly added ReadPlan branch bypasses the cache by default rather than silently serving the wrong representation — the structural defect behind both backlog#1108 and backlog#1109. Refs: backlog#1108, backlog#1109 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
GatewayJ
|
a269f8df05 | fix(ecstore): require write quorum for metadata early stop (#4300) | ||
|
houseme
|
399461c33e |
fix(ecstore): bound listing walks by drive stall, not total duration (#4647)
Foreground S3 listings wrapped the entire walk_dir stream in a single wall-clock timeout (RUSTFS_DRIVE_WALKDIR_TIMEOUT_SECS, default 5s). That budget measured how much data the walk had to produce rather than whether the drive was still answering, so a healthy but large prefix on slow media returned 500 InternalError / "Io error: timeout" to the client. The timer also kept running while the walk was blocked writing to a slow consumer, charging merge-side backpressure to the producer. WalkDirOptions::stall_timeout_ms already carried the right semantics but was only honored by the remote-disk RPC walk; LocalDisk::walk_dir ignored it entirely. A single-drive deployment therefore had no way to distinguish a hung drive from a big directory. Teach LocalDisk::walk_dir to bound each individual drive read with the stall budget, defaulting it from RUSTFS_DRIVE_WALKDIR_STALL_TIMEOUT_SECS when the caller does not pin one, and let the foreground listing path skip the wrapper-level total timeout. A walk that keeps making progress now runs to completion; a drive that stops answering still fails with DiskError::Timeout. Time spent blocked on the consumer stays outside the budget. Heal and rebalance walks already skipped the total timeout and previously ran unbounded on local drives. Give them an explicit, generous 60s stall budget so this change does not tighten them from "no bound" to the 5s default. Fixes #4644 Refs #2999, #3001 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
91a5c87132 | refactor(startup): thread explicit InstanceContext through the storage startup path (#4611) | ||
|
Zhengchao An
|
4f999bb6b8 | perf(ecstore): backfill rename_data old size and gate the PUT prelookup (#4598) | ||
|
Zhengchao An
|
ce6bc30b26 | test(ecstore): harden validation gate and EC coverage tests (#4590) | ||
|
Zhengchao An
|
90d769167b |
test(ecstore): pass codec fallback flag in tests
Pass the explicit allow_inplace_legacy_fallback flag in codec streaming reader tests so CI builds compile after the signature change. |
||
|
Henry Guo
|
c6d054245f |
fix(admin): refresh datausage live bucket usage (#4490)
* fix(admin): refresh datausage live bucket usage * fix(admin): reapply datausage memory overlay * fix(ecstore): avoid runtime lookup for empty shard costs --------- Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
houseme
|
8fc75e88c8 |
test(ecstore): multi-disk regression for read-before-write tagging under early-stop (backlog#881) (#4561)
test(ecstore): multi-disk regression for read-before-write tagging under early-stop Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
9636989fef |
perf(ecstore): in-place per-part legacy degradation for lazy multipart codec reader (backlog#879) (#4560)
perf(ecstore): in-place per-part legacy degradation for lazy multipart codec reader Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
ed81d2f6b8 |
test(ecstore): complete EC validation coverage gate
* test(ecstore): complete EC validation coverage gate * test(ecstore): stabilize validation suite after rebase * test(ecstore): fix rio-v2 clippy lint |
||
|
houseme
|
e3e5693e60 |
fix(ecstore): correct heal result drive-record reporting (#4555)
Two report-only defects on the heal result-reporting surface (they do not affect the data path or heal decisions): - default_heal_result (set_disk/ops/heal.rs): the offline-disk branch pushed an Offline record but fell through into the unconditional push, emitting a second (Corrupt) record for the same disk. This grew before/after.drives to disk_count + offline_count and misaligned every entry after the first offline slot. Add `continue` after the offline push, drive `disk_len` and the loop from a single `self.disks` snapshot, and assert `errs.len() == disk_len`. - Sets::heal_format (core/sets.rs): the before/after drive lists were pre-filled with N default placeholders and then N real entries were pushed, yielding a 2N list whose healed status updates (indexed 0..N) landed on the blank placeholder half. Assign the lists directly from formats_to_drives_info (mirroring the set-level heal_format) so the healed updates hit the real entries. Add regression tests covering offline/online record alignment and the NoHealRequired and heal paths of the pool-level heal_format. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
c77c5f047a |
fix(ecstore): defer multipart part.N.meta cleanup until after commit (#4548)
complete_multipart_upload deleted every part.N.meta via cleanup_multipart_path *before* the authoritative rename_data commit. If rename_data then failed write quorum, the upload directory was left with data files but no part metadata, so a retried CompleteMultipartUpload could never read the parts again and the upload became permanently uncompletable (client must abort and re-upload all parts). Move cleanup_multipart_path to run only after rename_data returns Ok, matching the existing "clean up only after commit" pattern already used for the old data-dir GC and the upload-dir delete_all. On a failed commit the staging part.N.meta now survive so the retry can complete. Add a hermetic regression test that forces rename_data to fail on every disk (destination bucket dir made read-only) and asserts the staging part.N.meta are preserved for retry. Fixes #946. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
f262fcfce0 |
perf(hotpath): add fine-grained PUT-path stage guards (HP-14) (#4541)
Close the ~10ms instrumentation residual on the PUT success path that the existing coarse writer_setup/encode/rename stage metrics do not attribute. Adds `hp_guard!` measurement scopes to the previously uninstrumented sub-stages called out in backlog#935 item 2: - SetDisks::acquire_read_lock / acquire_write_lock (namespace lock acquire) - S3::put_object_prelookup (pre-write get_object_info lookup) - MultiWriter::shutdown (bitrot writer flush/close) - SetDisks::commit_rename_data_dir (old data-dir reclaim) - S3Access::put_object (S3 authorization segment) Instrumentation only: `hp_guard!` expands to nothing without the `hotpath` feature, so this is a pure-observation change with zero behavior impact and zero cost in default builds. The pre-lookup site wraps only the lookup call in a scoped block so the guard measures that slice exactly while preserving the existing match and control flow. Verified: cargo check -p rustfs-ecstore (default and --features hotpath) and cargo check -p rustfs (default and --features hotpath) all pass. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
15808254d3 |
fix(ecstore): correct codec-streaming byte accounting and partNumber routing (#4535)
Two correctness defects on the opt-in codec-streaming GET path. ECA-02 (#943): ErasureDecodeReader only decremented `remaining` for the main fill buffer. Under the default DualInFlight policy each fill also produces a queued stripe that is delivered to the client via `prefetched_bufs.pop_front()` without touching `remaining`, so any object larger than one erasure block finished with `remaining > 0` and the GET terminated with LessData despite delivering all bytes. The inflated `remaining` was also fed back into the fill worker, which used it to trim the final stripe and to decide whether to read past EOF. Account for the queued-stripe bytes when they enter the prefetch queue; queued buffers come only from `Ok(true)` decodes so they are non-empty and bounded by `remaining - main_buf.len()`, ruling out underflow. ECA-04 (#945): the codec-streaming gate did not inspect `opts.part_number`. A partNumber GET carries `range == None`, so it was not classified as a Range request and reached the full-object codec-streaming reader, which drops the storage offset/length returned by GetObjectReader::new. A partNumber >= 2 request would then stream the whole object. Mirror the direct-memory part_number fallback and route any partNumber request back to the legacy duplex path, which applies the offset/length correctly. Regression tests: DualInFlight read_to_end on a multi-block object and on a non-block-aligned object; SingleInFlight vs DualInFlight byte-identical output; gate fallback on partNumber requests. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
3531abb34a |
feat(heal): disk-walk UNION enumeration to heal sub-quorum versions (backlog#920) (#4527)
B5 switched heal enumeration to list_object_versions, which only reflects the read-quorum metadata view: a version present on fewer than read-quorum disks was never enumerated, so it was never healed. Add a per-erasure-set disk-walk UNION enumerator (mirrors MinIO global-heal.go objQuorum=1 listPathRaw + mergeXLV2Versions) that surfaces every (object, version) present on ANY disk and feeds each to the existing per-version heal_object. - filemeta: MetaCacheEntries::resolve_union (dir_quorum=1/obj_quorum=1) yields the cross-disk version union at one tested seam. - ecstore: SetDisks::heal_walk_versions_page (list_path_raw fan-out, min_disks=1, dual object/version page bound, inclusive-forward de-overlap) + ECStore delegator + HealWalkVersion. - ecstore data-safety guard: before dangling-delete, try_regenerate_recoverable_meta physically probes part files via check_parts; when >= data_blocks data shards survive (meta lost but data recoverable) it regenerates xl.meta from a surviving FileInfo with the correct per-disk shard index instead of dangling-deleting. Genuine torn writes (< data_blocks) keep the current behavior — no resurrection. - heal: dw1: forward-marker cursor codec (reuses ResumeState.resume_cursor, idempotent restart on foreign tokens); list_versions_for_heal_page_disk_walk trait method (default falls back to the B5 read-quorum path); heal_bucket_with_resume selects the disk-walk enumerator when scan_mode==Deep || source==AutoHeal, else the unchanged B5 path; anti-loop guard aborts on (empty && truncated). Closes rustfs/backlog#920 |
||
|
Zhengchao An
|
2055044cb4 | fix(ecstore): paginate ListMultipartUploads across pools (#4522) | ||
|
houseme
|
7048a9a3ed |
test(ecstore): run shard_read_costs empty test under a Tokio runtime (#4496)
shard_read_costs_for_empty_disk_set_are_empty was a plain sync #[test], but shard_read_costs_for_disks consults process-global topology state (local_endpoint_hosts_for_shard_costs) whose fast-lock manager lazily spawns a background cleanup task on first access. When this test was the first in a process to touch that global — as under nextest's per-test isolation — the tokio::spawn panicked with a TryCurrentError because no runtime was present, making the test order-dependent flaky in CI (it passes only when some sibling tokio test initializes the manager first). Run it under a Tokio runtime like the sibling reservation tests (#[tokio::test]), so the lazy init's spawn always has a runtime. Test-only change; no production behavior change. Verified failing before the change and passing after under both `cargo test` and `cargo nextest run` in isolation. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
7fb95d4fc0 | fix(multipart): lock upload metadata reads and aborts (#4428) | ||
|
Zhengchao An
|
fefa70b31e |
fix(ecstore): stop ListMultipartUploads from returning one upload past max-uploads (#4447)
fix(ecstore): stop ListMultipartUploads from returning one upload past max-uploads (backlog#954) The result-collection loop in `SetDisks::list_multipart_uploads` pushed an upload into the page and only then checked `ret_uploads.len() > max_uploads`, so each page returned up to max_uploads + 1 uploads and pointed `next_upload_id_marker` at that surplus entry, violating the S3 ListMultipartUploads max-uploads contract. Move the cap check before the push (MinIO-style) so a page never exceeds max_uploads, and derive `is_truncated` from the post-marker cursor position (`upload_idx < uploads.len()`) rather than comparing the page length against the full listing length. The old length comparison mis-reported truncation on the final page whenever a marker had skipped earlier entries, which prevented marker-based pagination from terminating. Add a regression test that starts more in-progress uploads than a page holds and asserts a single page returns exactly max_uploads with a correct next-marker, that the exact-boundary case is not marked truncated, and that paginating one upload at a time enumerates every upload once with no loss, duplication, or non-termination. Incidental: add the missing `ctx` field to the `new_multipart_lock_test_store` cfg(test) helper so the ecstore test build compiles after the #4413/#4437 merge collision. Refs: https://github.com/rustfs/backlog/issues/954 |
||
|
Zhengchao An
|
a48bc89cdc |
fix(ecstore): lock batch object deletes (#4435)
* fix(ecstore): lock batch object deletes * fix(ecstore): honor no_lock in batch deletes |
||
|
Zhengchao An
|
df9cbc4ed1 |
fix(ecstore): validate erasure distribution values to avoid shuffle index panic (#4427)
fix(ecstore): validate erasure distribution values to avoid shuffle index panic (backlog#949) The element values of `erasure.distribution` read from `xl.meta` were never range-checked. `FileInfo::is_valid()` and `MetaObjectV1::valid()` only verified `distribution.len()` and the `erasure.index` bound, not that each distribution value is a valid 1-based slot in `[1, N]`. The metadata shuffle helpers then use these values directly as `distribution[k] - 1` indices, so a corrupt or adversarial `xl.meta` carrying a `0` (usize underflow) or a value greater than N (out-of-bounds) triggers a panic in the shuffle path, turning bad-disk metadata that erasure coding is meant to tolerate into a request/task crash. Fix, two layers: - Validate distribution values at metadata acceptance: `is_valid_distribution` now requires the distribution to be a permutation of `1..=N` (correct length, every value in range, no duplicates). `FileInfo::is_valid()` and `MetaObjectV1::valid()` use it, so `find_file_info_in_quorum` rejects corrupt metadata and it surfaces as a clean `ErasureReadQuorum` error instead of an index path. - Defensive indexing in the shuffle helpers (`shuffle_disks_and_parts_metadata`, `_by_index`, `_by_index_owned`, `shuffle_parts_metadata`, `shuffle_disks`, `shuffle_check_parts`): out-of-range distribution values are skipped via `checked_sub(1)` + bounds-checked slot access instead of a bare `idx - 1` index, matching the existing pattern in `collect_inline_data_shard_fileinfos_by_index`. Regression tests: `is_valid_distribution`/`is_valid`/`valid` reject distributions containing `0`, values greater than N, duplicates, and wrong length while accepting valid permutations; the shuffle helpers no longer panic on corrupt distributions and preserve output length. Refs: https://github.com/rustfs/backlog/issues/949 |
||
|
Zhengchao An
|
91dec123d9 |
refactor(ecstore): add per-instance InstanceContext, migrate erasure setup type (#4413)
* refactor(ecstore): add per-instance InstanceContext, migrate erasure setup type Phase 5 of the global-singleton consolidation (backlog#939): begin moving runtime identity state out of process globals so multiple ECStore instances can coexist in one process. Isolation is carried by the object graph (ECStore -> Sets -> SetDisks holding an Arc<InstanceContext>), not a task-local, which does not propagate across the many internal tokio::spawn boundaries in the data/background paths. This first slice migrates the erasure setup type -- previously three independent process-global bools -- into a single per-instance RwLock<SetupType> that derives is_erasure / is_dist_erasure / is_erasure_sd, removing a triple source of truth that could drift out of sync. - New runtime::instance module: InstanceContext + process bootstrap context. - The legacy free-function facade (is_erasure/update_erasure_type/...) keeps its signatures and forwards to the current instance's context, falling back to the bootstrap context before a store is published. - ECStore gains a pub(crate) ctx field and setup_is_* accessors; its constructors adopt the bootstrap context (never mint a fresh one) so startup writes and post-construction reads share one cell -- single-instance behavior is byte-for-byte unchanged. Tests: erasure predicate derivation vs the legacy behavior, object-graph carrier isolation across two ECStore instances, and bootstrap adoption. Refs: backlog#939 (Phase 5, Slice 1), backlog#653 (item 8) * refactor(ecstore): thread InstanceContext down the object graph (Phase 5 Slice 2) (#4415) * refactor(ecstore): source the namespace lock manager per-instance (#4417) refactor(ecstore): source the namespace lock manager per-instance (Phase 5 Slice 3) Phase 5 Slice 3 (backlog#939): give each instance its own lock namespace by sourcing SetDisks' lock manager from the instance context instead of the process singleton. This removes the false cross-instance mutual exclusion (and attendant ABBA risk) that a shared GlobalLockManager would cause once multiple instances coexist. - InstanceContext gains a `lock_manager: Arc<GlobalLockManager>`. `new()` mints a fresh manager (independent per-instance); `bootstrap_ctx()` aliases the process singleton via get_global_lock_manager(), so a single-instance deployment keeps exactly one shared namespace. - SetDisks::new sources `local_lock_manager` from `ctx.lock_manager()` (the ctx it already adopts), not `runtime_sources::global_lock_manager()`. Single instance: same Arc as before, so behavior is unchanged. - Remove the now-unused `runtime_sources::global_lock_manager()` wrapper. Tests: bootstrap lock manager aliases the process singleton; two fresh contexts own distinct managers; a SetDisks' lock manager is the one from its context and aliases the global singleton in a single-instance build. Verification: cargo test -p rustfs-ecstore (10 Phase 5 + set_disk locking regressions green), cargo clippy -p rustfs-ecstore --all-targets (clean), make pre-commit (pass). Refs: backlog#939 (Phase 5, Slice 3). Stacked on #4415 (Slice 2). |
||
|
Zhengchao An
|
cda7688909 |
fix(multipart): clean temp part data on failure (#4412)
fix(multipart): clean failed part temp data |
||
|
Zhengchao An
|
1e6207c08e |
fix(lock): fence write commit on lock loss (#4406)
fix(lock): fence write commit on lock loss (backlog#899 Phase 2) Phase 0+1 (#4388) made object write locks refreshable and marks the guard lost when the heartbeat can no longer refresh a quorum, but does not act on it. Under a partition a long write's lock can expire on an unreachable node and be reclaimed, letting a third party re-acquire it; the original writer keeps going and both commit -- a double write. Expose the loss signal through NamespaceLockGuard::is_lock_lost() and ObjectLockDiagGuard::is_lock_lost(), and fence the commit in put_object and complete_multipart_upload: immediately before rename_data (the atomic commit point), abort with a retryable NamespaceLockQuorumUnavailable (503) if the lock was lost. In multipart the check precedes cleanup_multipart_path so a lost lock leaves the upload intact and retryable. A write that already reached rename_data Ok is durable and never aborted. The loss criterion is unchanged (reacts to Phase 1's signal). Heal and the long-GET read side are deferred follow-ups. |
||
|
houseme
|
a413729b16 | perf(delete): gate and parallelize DeleteObjects per-object stat fanout (#4398) | ||
|
Zhengchao An
|
afc7f1d6f9 |
fix(ecstore): make post-commit old data dir cleanup best-effort (#4386)
* fix(ecstore): make post-commit old data dir cleanup best-effort (backlog#898) A write is authoritatively committed once rename_data returns Ok (the new version is durable on >= write_quorum disks and immediately readable). The subsequent reclamation of the now-dereferenced old object/<data_dir> is pure space reclamation, yet commit_rename_data_dir propagated a below-quorum GC failure via `?` into ErasureWriteQuorum -> 503, producing a false-negative ACK for an already-persisted write. This is a deliberate divergence from MinIO (erasure-object.go:1577), which couples the two; the divergence is justified by durability semantics, not parity. Changes: - commit_rename_data_dir now returns a structured OldDataDirCleanup receipt and never returns Err. Adds an old==committed-dir anti-misdelete guard and a committed_data_dir parameter. Classification is extracted into pure functions (classify_old_data_dir_cleanup / map_cleanup_join_result / is_cleanup_not_found) so it is unit-testable. Task panic/cancel is mapped to a non-ignored DiskError::other (never DiskNotFound), and not-found is normalized to reclaimed. - object.rs / multipart.rs consume the receipt instead of `?`. The result reverts to Ok, so the invalidate_get_object_metadata_cache self-heal and the capacity/compression accounting that a `?` early-return previously skipped now run on the cleanup-failure path too. - On residue, report_old_data_dir_cleanup emits leak metrics and enqueues an object heal over the existing heal channel (disk-health signal replacing the 503). heal_object -> reclaim_orphan_data_dirs already reclaims unreferenced local data dirs, closing the loop end to end. - Adds rustfs_old_data_dir_* counters (attempted/reclaimed/leaked/below_quorum) as the operator-visible backstop for leaked residue. - Adds a test-only (#[cfg(test)]) delete fault-injection seam; in production it inlines to a no-op None and has no behavioral effect. Tests: pure-function A/C group + join-error mapping + actions decision; A5/A5b real-disk guard/reclaim integration; end-to-end overwrite returning 200 while old-data-dir cleanup fails. #864 rollback guard test remains green. * fix(ecstore): resolve merge conflicts with origin/main in io_primitives.rs --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: houseme <housemecn@gmail.com> |
||
|
houseme
|
3f13d098b4 |
feat(observability): feature-gated hotpath instrumentation for the data path (#4394)
Merge the hotpath-rs wall-time instrumentation from the backlog#936 analysis worktree behind an opt-in 'hotpath' cargo feature, keeping the default build at zero overhead and zero dependency. - hotpath is an optional dependency everywhere (dep:hotpath feature syntax); the default dependency tree contains no hotpath crate at all - 40+ measurement points across S3 handlers, ECStore/SetDisks object and multipart ops, erasure encode/decode, bitrot, LocalDisk I/O, FileMeta codec, and HashReader - attribute sites use #[cfg_attr(feature = "hotpath", hotpath::measure)]; async_trait bodies use per-crate hp_guard! macros (ecstore + rustfs bin); rio gates measure_block! behind hp_measure_block! - feature chain: rustfs -> rustfs-ecstore -> rustfs-rio / rustfs-filemeta, each crate owning its own gate - hotpath-alloc is intentionally not wired up (hotpath 0.21.x TLS panic on cross-thread guard drop under tokio, see backlog#935); mimalloc stays the unconditional global allocator - docs/development/hotpath-profiling.md documents building, HOTPATH_* env vars, SIGTERM report flow, and how to reproduce the backlog#936 timing reports Refs: https://github.com/rustfs/backlog/issues/935 (HP-14, item 2), https://github.com/rustfs/backlog/issues/936 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
bd5d3c5d92 |
perf(ecstore): data-shards-only lockstep GET reads with stripe-aligned deferred parity engagement (opt-in) (#4392)
* feat(ecstore): add stripe-advance handles for deferred bitrot readers Give DeferredObjectReader a shared pending state and expose a DeferredReaderStripeHandle that advances the still-unopened source by whole bitrot blocks using the same bitrot_encoded_range geometry the reader was created with (identity mapping when hash_size == 0). This lets the GET decode path open a parity shard aligned to the stripe where a data shard failed instead of reading every parity shard on every stripe (backlog#923). An already-opened (or failed) reader rejects the advance so callers retire it rather than engage it out of alignment; bitrot verification after an advance checks the advanced stripe's block against that stripe's stored hash. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(ecstore): read only data shards on healthy lockstep GET behind opt-in gate PR #4289's lockstep fix made every reconstruction-verifying GET read all data+parity shards per stripe; the parity blocks are read, bitrot-hashed and then discarded, a deterministic 2x read-bytes/IOPS/hash-CPU amplification on healthy 2+2 objects (backlog#923). With the new opt-in gate RUSTFS_GET_LOCKSTEP_DATA_SHARDS_ONLY_ENABLE=true (default: false, behavior identical to main): - read_lockstep keeps only the data slots engaged while the object is healthy; parity slots stay unopened deferred readers. - When a data shard is missing or dies at stripe k, parity readers are engaged mid-object by advancing their deferred stripe handle to stripe k, preserving the lockstep alignment invariant from backlog#832. - Degraded stripes engage one parity beyond the decode quorum so reconstruction verification keeps an extra source to check against (erasure.rs only verifies when available > data shards); an engaged parity reader that errors is retired for the rest of the object like any other, and a parity reader that cannot be realigned is retired instead of being read out of position. - fill_deferred_bitrot_readers records stripe handles for deferred slots and, gate-on only, swaps eagerly opened parity readers for unopened deferred ones so they remain engageable mid-object; ready/error bookkeeping used by quorum decisions is untouched. - Both GET paths (legacy duplex via Erasure::decode_with_stripe_handles, codec streaming via ParallelReader::with_deferred_parity_handles) carry the handles from reader setup. Short-read -> UnexpectedEof -> whole-object retirement and the inconsistent-source rejection are unchanged in both gate modes; tests lock the healthy-path data-shards-only call counts, the default read-all-shards behavior, mid-object parity engagement for streaming and hash_size==0 formats, and mid-stream inconsistent-parity rejection. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
65953bfdb3 | fix(ecstore): reduce rename data version signatures (#4383) | ||
|
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> |