mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
f63af3df63309f043882f355fdbc27ba81f6123c
1900 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Zhengchao An
|
f63af3df63 |
chore: retire completed-migration scaffolding, wire orphaned boundary check (#4719)
The ecstore/global-state migrations are done (backlog#815, #939, #1052 all closed). Review of every migration-era test/gate measure found three things actually retirable or broken — everything else is a live anti-regression guard and stays. Remove: - scripts/check_metrics_migration_refs.sh — guards a migration that finished: rustfs_metrics:: has zero hits, the metrics crate no longer exists, and the script was never wired into CI or make (only reference was one line in config-model-boundary-adr.md, also removed). - crates/obs init_metrics_collectors — the "backward-compatible alias kept during migration" the removed script was guarding. Zero callers; pure delegate to init_metrics_runtime. Archive (docs/superpowers/plans/, continuing the 2026-07 convention, with the standard archived banner): - startup-timeline.md, scheduler-baseline.md, profiling-numa-capability-inventory.md, kms-development-defaults-inventory.md — one-shot snapshots whose only consumer is the already-archived migration-progress ledger (their same-dir links there start resolving again after the move); zero script pins; fed the closed backlog#660/#665 architecture-review ledger. Fixed the one outbound link (startup-timeline -> readiness-matrix) that the move would have broken — check_doc_paths.sh deliberately does not scan plans/, so nothing else would have caught it. Wire (found orphaned by the same review): - scripts/check_extension_schema_boundaries.sh guards a live contract crate but was never invoked anywhere. Add lint-fmt.mak target, include in pre-commit/pre-pr/dev-check, add ci.yml Quick Checks step (job already installs ripgrep), sync the CONTRIBUTING.md enumerated list, and harden the script against a silently-passing rg probe when src/ is missing. Keep (verified live, documented so the next cleanup pass does not repeat this analysis): - scripts/check_architecture_migration_rules.sh — added a header stating it is a permanent boundary guard, not retirable migration scaffolding; 'migration' in the name is historical. - check_migration_gate_count.sh + floor, delete-marker e2e proof, all pinned docs, compat-cleanup-register sync, remaining inventories (referenced by live docs). Verification: all 7 guard scripts pass, actionlint clean, cargo check --workspace (excl e2e) clean, cargo fmt --check clean. Adversarially reviewed by two independent skeptic passes; their 7 findings (alias left behind, broken outbound link, missing banners, wrong backlog attribution, CONTRIBUTING drift, rg exit-2 hole, missing header rationale) are all folded in. |
||
|
houseme
|
9bf102f965 |
fix(cache): reserve admitted bytes in the memory gate to bound burst overshoot (#4718)
The fill gate compared each request against a snapshot refreshed at most every 5 s, with no accounting for what it had already let through. A burst arriving while the snapshot still read high therefore all passed the same check-then-act test and over-allocated far past the real headroom before the next refresh — a gap the burst stress test could expose but not close. Track admitted bytes since the last refresh in the shared snapshot cell and subtract them from available memory in `allows_fill`, reserving the request's size on each admission. Cumulative admission is now bounded to the real budget even though every fill reads the same stale snapshot; the refresh resets the counter because the fresh reading already reflects those allocations. The `min_free_memory_percent == 0` opt-out still short-circuits first, and the already-low-memory path is unchanged. New test `moka_backend_gate_reservation_bounds_burst_under_stale_snapshot`: 20 concurrent 40 KiB fills against a 500 KiB stale-high snapshot (300 KiB budget) admit only a bounded handful, not the whole storm. Passed 10/10 runs; mutation-verified — dropping the reservation admits all 20 and fails the test. Refs: backlog#1107 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
05fae6f939 |
test(ecstore): add TierConfigMgr state-machine unit coverage (#4713)
* test(ecstore): unit-test TierConfigMgr add/edit/remove/verify state machine (backlog#1148 ilm-4) Covers the tier config state machine and persistence paths that previously had only 4 codec tests and none for tier_config.rs: - add: non-uppercase name, duplicate name, unsupported type, missing backend payload, and a regression anchor documenting that AWS-reserved names (STANDARD) are not currently rejected. - edit: unknown tier, missing-credentials rejection for RustFS and MinIO. - remove: idempotent unknown-tier no-op, in-use rejection, empty-backend success, force skips the in_use probe, and probe-error surfacing. - verify: unknown tier, healthy backend, unhealthy backend. - pure query helpers (empty/is_tier_valid/tier_type/get/list_tiers). - persistence: JSON marshal/unmarshal roundtrip, external tier-config.bin roundtrip for Azure and GCS payload mapping, truncated/unknown-format/ unknown-version rejection, legacy v1 version-word acceptance, and encode failure on missing payload. Tests are hermetic: error paths return before backend construction, and a MockWarmBackend injected into driver_cache exercises remove/verify without any real remote. Refs backlog#1155. Co-authored-by: overtrue <anzhengchao@gmail.com> * fix(tier): reject reserved names STANDARD/RRS in TierConfigMgr::add (backlog#1148 ilm-4) (#4721) |
||
|
Zhengchao An
|
846aa95c32 |
test(security): GHSA-named regression tests for 3p3x and r5qv (backlog#1151 sec-6) (#4707)
test(security): add GHSA-named regression tests for 3p3x and r5qv (backlog#1151 sec-6) Anchor the two fixed advisories to discoverable, named regression tests so `rg -i "ghsa|3p3x|r5qv"` finds a guard for each, and future fixes are forced to update pinned behavior (red -> green). GHSA-3p3x-734c-h5vx (constant-time WebDAV/FTPS secret comparison, rustfs#4403): - ftps_core.rs: new `assert_ftps_ghsa_3p3x_wrong_credentials_rejected` drives the `ct_eq` reject branch in FtpsAuthenticator::authenticate; asserts wrong password and unknown user are both rejected (530) and indistinguishable. - webdav_core.rs: the auth-failure block now sends a valid access key with a wrong secret (exercising the `ct_eq` branch, not just the unknown-access-key path) plus an unknown user, asserting both 401 and indistinguishable. - Module doc comments map advisory -> tests -> fix PR on both files. GHSA-r5qv-rc46-hv8q (internode RPC fail-closed, rustfs#4402): - http_auth.rs: renamed the default-fallback rejection test to `ghsa_r5qv_resolve_shared_secret_rejects_default_fallback` (and broadened it to cover default env secret + blank secrets), and added `ghsa_r5qv_verify_rpc_signature_fails_closed_on_missing_or_invalid_auth` pinning the exact advisory scenario (missing/forged/cross-URL signature is rejected; a correctly signed request still passes). File-level doc maps the advisory. Docs: new docs/testing/security-regressions.md with the advisory -> test mapping table and where each layer runs; linked from docs/testing/README.md. sec-14 will formalize the written policy in AGENTS.md. The unit-level ghsa_r5qv_* tests run in the default CI pass. The WebDAV/FTPS e2e live in the protocols suite (fixed ports, --test-threads=1); they cannot join the e2e-smoke profile and are wired into CI by sec-5. Refs: rustfs/backlog#1151 (sec-6), rustfs/backlog#1155 |
||
|
Zhengchao An
|
4b83efaf36 |
ci(ilm): add gated s3-tests lane for lifecycle expiration cases (#4715)
* ci(ilm): move first batch of 5 s3-tests lifecycle expiration cases into a gated behavior lane (backlog#1148 ilm-10)
The 20 lifecycle cases in excluded_tests.txt were labeled "vendor-specific"
but the real blocker was the absence of a Ceph lc_debug_interval equivalent.
RUSTFS_ILM_DEBUG_DAY_SECS (ilm-5) now provides that, so Days>=1 expiration
behavior is testable in seconds.
These cases assert that objects/versions/uploads are actually removed by the
background scanner and the stale-multipart cleanup loop. They cannot join the
default single-server s3-implemented-tests gate: it disables the scanner, and a
global RUSTFS_ILM_DEBUG_DAY_SECS also shrinks the x-amz-expiration header that
the already-passing test_lifecycle_expiration_header_* cases assert on. So this
adds an isolated lane instead of putting them in implemented_tests.txt.
First batch (5), each verified against the pinned upstream source and the
RustFS evaluator/scanner/stale-multipart execution paths:
- test_lifecycle_expiration (prefix Days=1/Days=5, scanner delete)
- test_lifecyclev2_expiration (same via ListObjectsV2)
- test_lifecycle_noncur_expiration (NoncurrentVersionExpiration)
- test_lifecycle_deletemarker_expiration (ExpiredObjectDeleteMarker cascade)
- test_lifecycle_multipart_expiration (AbortIncompleteMultipartUpload)
Dropped from the batch, kept excluded with reason:
- test_lifecycle_expiration_days0: RustFS accepts Expiration{Days:0} (returns
200; only Days<0 is rejected in crates/lifecycle/src/core.rs validate()),
while AWS/this test expect InvalidArgument. Real validation gap.
Changes:
- scripts/s3-tests/lifecycle_behavior_tests.txt: new exact-node-id run set.
- scripts/s3-tests/run.sh: honor an IMPLEMENTED_TESTS_FILE override so a lane
can point at an alternate whitelist.
- .github/workflows/ci.yml: new s3-lifecycle-behavior-tests PR-gate job that
starts rustfs with RUSTFS_ILM_DEBUG_DAY_SECS=10, scanner enabled (cycle 2s,
no start delay) and a 2s stale-multipart cleanup interval, running the new
list serially.
- scripts/s3-tests/report_compat.py: recognize the behavior list so the weekly
scope=all sweep (plain server) does not misclassify expected failures.
- scripts/s3-tests/excluded_tests.txt: remove the 5 enabled cases; re-annotate
the remaining 15 lifecycle exclusions with real reasons + batch-2 plan.
- docs/architecture/s3-compatibility-matrix.md: sync counts (implemented 451,
excluded 274, behavior 5) and document the lane.
Refs backlog#1148 (ilm-10), master plan backlog#1155.
* fix(lifecycle): reject zero-day expiration/noncurrent/abort rules (backlog#1148 ilm-10) (#4722)
|
||
|
Zhengchao An
|
abe6c41227 |
test(e2e): negative header-SigV4 rejection suite (backlog#1151 sec-1) (#4708)
test(e2e): add negative header-SigV4 rejection suite (backlog#1151 sec-1) RustFS delegates SigV4 verification to the s3s dependency, so nothing in this repo pins OUR end-to-end wiring of it. Add negative_sigv4_test.rs sending REJECTED header-SigV4 requests against a live RustFSTestEnvironment and asserting the HTTP status plus the S3 error code XML: - valid_header_sigv4_request_succeeds (positive control so the negatives cannot pass for the wrong reason) - tampered_signature_returns_signature_does_not_match -> 403 SignatureDoesNotMatch - wrong_secret_key_returns_signature_does_not_match -> 403 SignatureDoesNotMatch - tampered_payload_is_rejected (body != signed x-amz-content-sha256) -> not 200 - skewed_date_returns_request_time_too_skewed (>15min) -> 403 RequestTimeTooSkewed - malformed_authorization_header_returns_clean_4xx (present-not-missing) -> 4xx, never 5xx Signatures are hand-built via rustfs_signer::request_signature_v4 primitives (get_signing_key/get_signature/get_scope) so the test controls the timestamp, secret, signed payload hash, and final signature bytes. Missing-credential negatives are intentionally not duplicated (covered by multipart_auth_test / anonymous_access_test). Refs backlog#1151 (sec-1), master plan backlog#1155. |
||
|
Zhengchao An
|
ac646cfbe4 |
test(e2e): large-object degraded-read EOF truncation regression net (dist-13) (#4709)
Adds an S3-API-level e2e regression net proving that a large-object GET on a
degraded EC set never returns a silently truncated body under a full
Content-Length -- the historical "unexpected EOF" bug fixed on main by
rustfs#4594 (short-body GetObject stream -> UnexpectedEof), rustfs#4560
(in-place per-part legacy degradation for the lazy multipart reader), and
rustfs#4585 (DARE package-boundary truncation detection). Those fixes each
ship a *unit* regression; this covers the layer they do not -- the full HTTP
GET path streaming a real body reconstructed from real on-disk EC shards.
New file crates/e2e_test/src/degraded_read_eof_regression_test.rs, single-node
4-disk (EC 2+2) DiskFaultHarness, three scenarios:
(a) one disk offline: a 6 MiB single object (>=2 EC stripes) and a 3x5 MiB
multipart object GET back byte-identical with the correct Content-Length.
(b) mid-stream bitrot within quorum: 2 of 4 shards corrupted mid-file on a
large multipart object still reconstructs the full, hash-matching body.
(c) beyond read quorum (the heart of the net): 3 of 4 shards corrupted
mid-file -- the read MUST fail cleanly (non-2xx or a mid-stream body
error), NEVER close with a truncated body under the full Content-Length.
The shared get_checked() helper panics on the forbidden outcome (a clean 2xx
whose collected body is shorter than the advertised Content-Length), so the
truncation bug can never be silently tolerated.
CI placement: these spawn a 4-disk server per test and are resource-heavy, so
they stay OUT of the fast PR e2e-smoke filter. A new e2e-reliability nextest
test-group (max-threads=1) serializes them (and the existing
reliability_disk_fault_test) across nextest's process boundary; ci-7's nightly
full-e2e run picks them up. Visible via `cargo nextest list -p e2e_test`.
Refs rustfs/backlog#1150 (dist-13), rustfs/backlog#1155, rustfs#4594,
rustfs#4560, rustfs#4585, rustfs#2955.
|
||
|
houseme
|
d5d6f1160d |
test(object-data-cache): add concurrency stress tests and GET benchmarks (#4711)
* test(object-data-cache): add concurrency stress tests for the fill machinery The race coverage so far pins specific interleavings with an injected barrier. These add sustained, real-parallelism contention to catch what pinned tests cannot — a leaked singleflight leader, a stranded semaphore permit, an index/cache divergence that only surfaces under volume: - moka_backend_concurrency_storm_leaves_no_leaked_state: 48 tasks x 400 mixed fill/lookup/invalidate ops on a 6-key pool over 4 worker threads, then asserts inflight_fills == 0 (no leaked in-flight fill), a clean post-storm fill/hit/invalidate sequence still works (state not corrupted), and clear() drains the cache. - moka_backend_gate_bounds_admission_under_concurrent_burst: a low-memory snapshot must reject an entire 32-fill burst — admission is bounded, not a check-then-act race that lets the burst through. (This does not cover the separate 5s-stale-snapshot overshoot, which needs byte-based reservation.) Both are mutation-verified: neutering invalidate_object fails the storm's "invalidation must win" assertion, and making allows_fill always-true fails the burst's full-rejection assertion. The storm passed 20/20 runs. `rt-multi-thread` is added to dev-deps so the tasks run on real worker threads. Refs: backlog#1107 Co-Authored-By: heihutu <heihutu@gmail.com> * bench(object-data-cache): measure the GET-path cost the audit argued statically Adds a criterion benchmark driving the public ObjectDataCache facade, so the "hot path is clean / high throughput" claim rests on numbers, not analysis: - plan_get ~104-112 ns (per-GET planning + metric label-set hash) - lookup_hit ~143-147 ns (a resident-body hit: moka get + Bytes clone + metric) - fill_distinct ~2.2-2.4 us (cold fill: plan + singleflight + index + moka + inner spawn) A hit at ~143 ns against a multi-millisecond erasure read is the ~1000x saving the cache exists for; the per-GET metric cost is real but ~100 ns, not a bottleneck; the fill cost (incl. the cancellation-safety spawn kept on purpose) is negligible on a background task. Run with `cargo bench -p rustfs-object-data-cache`. Refs: backlog#1107 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
ce53feee87 |
test(lifecycle): regression test for expire/GET race (#3491) (#4706)
Adds a serial-lane ILM integration regression test that pins the local-first ordering contract established by rustfs#3491, where `expire_transitioned_object` deletes local metadata BEFORE any remote tier cleanup so a concurrent GET can never observe live local metadata pointing at an already-removed remote tier version (user-visible `NoSuchVersion`). `serial_tests::test_expire_transitioned_object_never_races_concurrent_get` in `crates/scanner/tests/lifecycle_integration_test.rs`: - Transitions an object to a mock warm tier, then runs a tight concurrent GET loop while calling `expire_transitioned_object`; every GET must return a full, correct body or a clean object/version-not-found -- never a tier-fetch failure. - Deterministic ordering assertion (revert-proof): immediately after expiry the remote tier object is still present and the mock recorded zero remote `remove` calls, proving remote cleanup is deferred to free-version recovery. Reverting to remote-first ordering turns this assertion red. Supporting changes: - Re-export `expire_transitioned_object` from `api::bucket::lifecycle`. - Record remote-tier mutating ops in the scanner test `MockWarmBackend`. - Update tier-ilm-debugging.md to reference the new regression test. Runs in the CI ILM Integration (serial) lane (ci.yml test-ilm-integration-serial), picked up by the existing binary(lifecycle_integration_test) filter. Refs: rustfs/backlog#1148 (ilm-2), rustfs/backlog#1155 |
||
|
Zhengchao An
|
5c7c757a30 |
ci(repl): wire replication e2e suite into nextest profiles (backlog#1147 repl-1) (#4712)
Activate the 36 dormant replication e2e tests in crates/e2e_test/src/replication_extension_test.rs (zero ran anywhere before). Split via the ci-4 nextest profile mechanism, no hand-rolled cargo-test lane: - PR smoke (profile.e2e-smoke, existing e2e-tests job): the 20 fast bucket-replication tests (target-registration / replication-check / list / remove / delete admin paths) that validate config synchronously and never wait for async convergence. Each spawns its own single-node rustfs server(s) on random ports with isolated temp dirs, so parallel-safe by construction (serial_test's #[serial] is a no-op under nextest's process-per-test model; no test-group needed). - Nightly (profile.e2e-repl-nightly + .github/workflows/e2e-replication-nightly.yml): the remaining 16 = 6 slow data-plane tests + 9 _real_dual_node + 1 _real_single_node. Defined as 'replication module MINUS the PR allowlist' so new replication tests default to nightly and are never silently unrun. The nightly workflow builds the binary once, installs awscurl so the STS dual-node test runs (skips gracefully with a visible log line otherwise), and routes scheduled failures through .github/actions/schedule-failure-issue (ci-8). Explicit division of labor with ci-5 e2e-full: these run only here. Counts (cargo nextest list): e2e-smoke 83 (63 + 20), e2e-repl-nightly 16. Docs updated: e2e-suite-inventory.md, e2e_test/README.md. Refs backlog#1147 repl-1, backlog#1155. |
||
|
Zhengchao An
|
c41a813a31 |
docs(e2e): contributor guide for the e2e_test crate (backlog#1153 infra-10) (#4705)
docs(e2e): write contributor guide for e2e_test crate (backlog#1153 infra-10) Turn the e2e_test README skeleton into a dense, link-heavy contributor guide: crate overview + grouped module map, how to run (whole crate / one module / e2e-smoke profile / ILM serial lane / protocols suite), #[ignore] semantics as a pointer to live sources, how to add a test (RustFSTestEnvironment + RustFSTestClusterEnvironment, common.rs/chaos.rs helper inventory, isolation rules, #[serial]-vs-nextest reality), a CI subset map table, and a troubleshooting section (stale binary / RUSTFS_TEST_PORT / orphan processes). Keeps the ci-4 "CI smoke subset" section intact and restructures around it. Adds a reciprocal link from crates/e2e_test/AGENTS.md. Refs rustfs/backlog#1153 (infra-10), #1155. |
||
|
Henry Guo
|
3f25426534 |
fix(ecstore): reject incomplete listing usage refreshes (#4698)
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
houseme
|
7437f99c45 |
fix(cache): key the object body cache on data_dir for write-uniqueness (#4703)
* refactor(object-data-cache): derive Default for ObjectDataCacheGetRequest The GET request literal is hand-listed field-by-field across ~13 test sites in two crates. Adding `mod_time_unix_nanos` in backlog#1111 had to touch every one and still missed a literal, producing a compile error caught only in a later CI lane. Derive `Default` and spread the engine-crate literals so the next field addition is absorbed rather than fanned out. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): key the object body cache on data_dir for write-uniqueness The cache key's correctness rested on being write-unique, but its only write-scoped component was `mod_time` — a wall-clock timestamp that is not monotonic and can be absent. Two writes that collide on MD5 (same etag+size) and land on an equal mod_time (clock skew, same-tick, or absent) derived the same key, so a node that never saw the overwrite could serve the previous body for up to the TTL, and the same collision turned the fill-after-invalidation race into a serving bug. This is the store's strong-read-after-write guarantee leaning on a probabilistic argument. Add `data_dir` — the xl.meta directory UUID ecstore regenerates on every body write — as the primary write-unique anchor: - surface `data_dir: Option<Uuid>` on ObjectInfo, copied from FileInfo in the single GET-path constructor `from_file_info`; - carry it into ObjectDataCacheKey as `data_dir_u128` (held as u128 to keep the engine crate free of a uuid dependency), derived in the one planner site both the ecstore hook and the usecase layer share, so both produce an identical key by construction; - keep `mod_time` as a second anchor and `etag+size` as belt-and-braces; an absent data_dir falls back to the prior behavior — strict improvement, no regression. Two writes distinct only by data_dir now derive different keys even under an MD5 collision with identical mod_time — the case mod_time alone cannot cover. Blast radius is compiler-guarded: ObjectInfo derives Default and every real construction site uses `..Default::default()`, so only from_file_info and one full-literal test needed the field. The three P0 body_cache_hook_e2e regressions, engine (80), and app (36) suites pass unchanged; a mutation that severs the planner wiring fails planner_key_changes_with_data_dir. Refs: backlog#1111, backlog#1118 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(cache): thread data_dir field through the merged mutation-hook test Merging main (which landed the object-mutation-hook work, backlog#1131) brought in a GetRequest test literal that predates the data_dir field. Spread it via `..Default::default()` — the derive(Default) added here means this is the last such hand-listed literal to need touching. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
85fd824581 |
feat(object-data-cache): close write-side invalidation gaps and add an admin surface (#4694)
* feat(object-data-cache): close write/delete-side invalidation gaps The object data cache exposed only a single per-(bucket,object) invalidation primitive and no write-side ecstore hook, so several delete paths left dead bodies resident until TTL (hygiene/capacity, not stale-serving: lookups follow a fresh metadata quorum and cannot serve a gone object). This adds the missing primitives and wires them in. ODC-26 (backlog#1131): add an `ObjectMutationHook` trait beside the GET body hook, registered next to it at startup, and call it from the ecstore-internal delete paths (`apply_expiry_on_non_transitioned_objects`, `expire_transitioned_object` including the restored-copy branch, and `delete_object_versions`). The app impl is one `invalidate_object` call under a new `AfterLifecycleExpiry` reason. ODC-27 (backlog#1132): force prefix delete now invalidates the whole prefix, not just the prefix string. `store.delete_object(delete_prefix)` returns no deleted-name list, so this uses a new prefix primitive rather than the batch path. ODC-28 (backlog#1133): DeleteBucket now flushes the bucket via a new bucket-scope primitive (covers force and non-force, which share the delete_bucket call). ODC-C2 (backlog#1143): add `ObjectDataCache::clear()` and two admin handlers (GET stats, POST flush) routed through admin runtime_sources. The starshard identity index gains a single `remove_matching` full-scan API backing prefix/bucket/clear; it is documented as admin/delete-path only and never runs on the GET or fill hot path. New invalidation reasons and metric labels added; outcome (removed/noop) labelling kept correct for every new primitive. Also fixes a pre-existing broken intra-doc link in memory.rs. Co-Authored-By: heihutu <heihutu@gmail.com> * refactor(ecstore): extract the shared HookSlot behind both cache hooks This PR introduced object_mutation_hook.rs by mirroring body_cache_hook.rs, which left two process-global registration slots whose register/get/clear bodies were line-for-line identical except the trait type and the WARN string: a RwLock<Option<Arc<dyn _>>>, an Arc::ptr_eq "different instance" warning, the poison-recovery closure, and the same read-lock-and-clone read. Two copies of the same swap-vs-warn logic can drift apart under maintenance. Hoist it into a generic HookSlot<T: ?Sized> that owns the logic once. Each hook module keeps its `static HOOK: HookSlot<dyn XxxHook>` and its thin, unchanged public wrappers (register_/get_/clear_), so the crate's public surface and every call site are untouched — this is an internal consolidation, not a contract change. The load-bearing #1126 guarantee (newest registration wins, so a rebuilt AppContext is never stranded on a first-wins slot) previously had no direct test — the hook tests only covered register-then-notify. HookSlot now has its own unit tests including re_registration_swaps_to_the_latest_instance; mutation-testing confirms a first-wins regression fails exactly that test. No behavior change: the two hooks' existing tests, the P0 body_cache_hook_e2e regressions, and the app-layer mutation-hook tests all pass unchanged. Refs: backlog#1126, backlog#1131 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(admin): register the object-data-cache routes in the policy inventory This PR added GET /object-data-cache/stats and POST /object-data-cache/flush but did not list them in the two registries that must account for every admin route: the route-policy inventory (route_policy.rs) and the route matrix (route_registration_test.rs). Their coverage tests — route_policy_inventory_covers_registered_routes and test_admin_route_matrix_matches_registered_routes — failed on CI because a registered route had no policy/matrix entry. These two tests are not part of `make pre-commit` (which runs fmt + arch + quick-check, not the full suite), so the gap passed local pre-commit and only surfaced in the CI Test-and-Lint lane. stats is a read (ServerInfoAdminAction, Sensitive); flush mutates (ConfigUpdateAdminAction, High) — matching the actions the handlers already enforce. The MinIO-alias matrix test is unaffected: these are native rustfs endpoints with no MinIO equivalent. Refs: backlog#1143 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
f9874b591a |
[DO NOT MERGE until published] chore(ecstore): switch rustfs-uring to crates.io 0.1.0 (#4701)
* chore(ecstore): switch rustfs-uring from the git rev to crates.io 0.1.0
Prepared ahead of the rustfs-uring 0.1.0 crates.io release (rustfs/uring):
flip ecstore's dependency from the pinned git revision to the published
`0.1.0`, and drop the now-unneeded git-source allowance in deny.toml.
DO NOT MERGE until rustfs-uring 0.1.0 is on crates.io. Until then cargo
cannot resolve `rustfs-uring = "0.1.0"`, so this branch will not build and
CI will be red — that is expected, not a defect in the change.
After the crate is published:
1. `cargo update -p rustfs-uring --precise 0.1.0` to regenerate Cargo.lock
(deliberately not touched here — the registry entry, with its checksum,
cannot be written before the crate exists);
2. push the Cargo.lock;
3. CI goes green; merge.
No code change. The guard `scripts/check_no_tokio_io_uring.sh` is unaffected
(it bans only tokio's io-uring runtime feature, not an explicit rustfs-uring
dependency). `audit.yml`'s `allow-dependencies-licenses` for rustfs-uring is
left in place — it is a first-party Apache-2.0 crate either way.
Co-Authored-By: heihutu <heihutu@gmail.com>
* chore: refresh rustfs-uring lockfile
---------
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
houseme
|
48bdf87e53 |
refactor(obs): drop the dial9 S3 config left stranded by the upload revert (#4700)
#4663 added a full S3 upload path (config fields + env parsing + wiring), then reverted the wiring when its dependency turned out to carry RUSTSEC advisories (D9-14). The revert stopped at enabled.rs and left the config layer half torn down: `Dial9Config` still carried `s3_bucket`/`s3_prefix` with no consumer, plus a dead `s3_upload_requested()`, and `Dial9SessionGuard::config()` was likewise never called. All three are zero-consumer dead code. Since S3 upload cannot be built at all, the config type should not model it. Drop the two fields and both accessors. `from_env` still reads the S3 env vars and warns about them (an operator who set them deserves to know why they do nothing — that warning is deliberate and stays), but drops the values instead of storing configuration nothing reads. While here, collapse `from_env`'s two `record_config` calls into one by extracting `from_env_enabled`, so the enabled/disabled paths share a single publish point. No behavior change: the S3 warning fires identically, and every other field is untouched. Net -13 lines, and `Dial9Config` drops from 8 fields to 6. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
95819cb986 |
docs(object-data-cache): state the correctness boundary and timing channel (#4695)
docs(object-data-cache): state the correctness boundary and the timing channel The crate doc still described itself as "a minimal skeleton for the initial rollout phase", which stopped being true several releases ago, and neither the crate nor the operator-facing env constant said anything about what the cache does or does not guarantee. Record two things a reader has to know. The correctness boundary: a hit is sound because the key matched metadata the caller just resolved from a read quorum, not because invalidation ran. Process-local invalidation is hygiene that frees capacity; the key is what keeps a stale body from being served. Anyone tempted to weaken the key should meet this sentence first. The timing side channel: a hit skips the erasure read, bitrot verify and decode, so it is reliably faster than a miss. A principal authorized to read an object can time a single GET and learn whether someone read that object within the entry's lifetime. This crosses no authorization boundary — the probe needs read access to that exact object, checked before the cache is consulted — but it does disclose a co-tenant's recent access pattern. Say so where operators look: on RUSTFS_OBJECT_DATA_CACHE_ENABLE. Timing noise would cost precisely the latency the cache exists to save, so the mitigation is to leave the cache off for buckets where access-pattern confidentiality matters. Refs: backlog#1139 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
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> |
||
|
Zhengchao An
|
d0ca14d8df |
test(ecstore): cover post-commit hard-crash in rename_data crash harness (#4689)
The rename_data crash-consistency harness (backlog#935/#878) only armed pre-commit crash points (AfterDataRename, AfterBackupBeforeMetaCommit), which assert the *old* version survives. The other half of the "partial commit -> old or new, never mixed" invariant — a hard power loss *after* the xl.meta commit rename must leave the *new* version — had no crash-point coverage. The existing after-metadata-commit failpoint only exercises the graceful in-process rollback (restores old), a different path from a hard crash (no rollback runs, commit stays on disk). Add a RenameDataCrashPoint::AfterMetaCommit injection right after the commit rename (cfg(test), compiles to a const-false no-op in production, like the existing points) and overwrite/fresh x strict/relaxed tests asserting the object reads back as the new version. The fresh case pins that the point is genuinely post-commit: a pre-commit misplacement would leave no readable object and fail the assertion. cargo test -p rustfs-ecstore crash_consistency: 9 passed. clippy -D warnings clean; fmt and arch guards clean. |
||
|
houseme
|
d715cb5c34 |
refactor(ecstore): single-source the bitrot read/verify path (backlog#1159) (#4697)
P-A (`read_appending`) and P-C (the in-memory fast path) each copied the hashed-read logic, so `BitrotReader` ended up with the hash verification, the short-read error, and the scratch-buffer fill written three times across `read` and `read_appending`'s two branches. That is patch-on-patch: a change to the bitrot contract would have to be made in three places and kept in sync by hand. Collapse the duplication onto three single-source pieces: - `split_and_verify` — a free function that splits `[hash][data]`, verifies (unless skip_verify), and returns the data slice plus the hash time. Free rather than a method so it can run while `self` is borrowed for the block. - `read_scratch_block` — the single-pass fill of `self.buf` with the short-read-to-UnexpectedEof contract. - `short_shard_read` / `begin_read` — the shared error and preamble. `read` and `read_appending` now differ only in what they must: how the block is acquired (caller's slice vs `try_take_block` vs scratch fill) and where the verified shard lands (`copy_from_slice` vs `extend_from_slice`). This also fixes a latent inconsistency the duplication hid: the old `read` did `copy_from_slice` *before* verifying, so on a hash mismatch it left the corrupt bytes in the caller's buffer before returning the error, while `read_appending` verified first. Both now verify before writing, so a shard that fails the hash never reaches the caller's buffer on either method — the stronger of the two behaviors. Net -19 lines; behavior otherwise unchanged. Verified: `erasure::` 215 passed, 0 failed (including the fast-path equivalence and corrupt-shard tests, and the existing `test_bitrot_read_hash_mismatch`); clippy --all-targets -D warnings clean. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
60ad15a7f9 |
fix(obs): remove the dial9 task-dump switch that could never work; correct measured claims (#4688)
* fix(obs): remove the dial9 task-dump switch that could never do anything Measured on a bench host (Linux x86_64) against the code merged in #4663: with `RUSTFS_RUNTIME_DIAL9_TASK_DUMP_ENABLED=true`, a `dial9-taskdump` build, and `--cfg tokio_taskdump`, dial9 recorded **zero** TaskDump events. dial9 captures a task dump only for futures it wrapped itself — those spawned through `dial9_tokio_telemetry::spawn`, which is where `TaskDumped<F>` gets applied. `tokio::spawn` gets no wrapper, and RustFS spawns with `tokio::spawn` throughout. Same workload, same binary, only the spawner changed: tokio::spawn -> 0 dumps dial9::spawn -> 14709 dumps, all with callchains Upstream documents this (README line 151) and tracks the doc gap at dial9-rs/dial9#477. I did not read it before wiring `with_task_dumps` in #4663, and so shipped exactly the kind of lying configuration knob that PR set out to delete. Remove it: the two environment variables, the config fields, the `with_task_dumps` call, and the `dial9-taskdump` feature — whose only effect was to constrain the build to Linux while recording nothing. Re-adding it only makes sense together with migrating the paths under investigation to dial9's spawner. Tracked as D9-16 in rustfs/backlog#1157. Also drop the `--cfg tokio_taskdump` requirement from the Makefile. Measured: dumps are captured with and without it (14709 vs 14674, within noise), and upstream never asked for it. That requirement was mine, invented and untested. Cargo.lock loses tokio's `backtrace` dependency, which `tokio/taskdump` pulled in. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(obs): replace guessed dial9 retention numbers with measured ones Three corrections, all to claims I wrote in #4663 without measuring them. "Under a high poll rate that budget can wrap in minutes" was a guess. Measured on a single-node 4-drive cluster under warp mixed (66 MiB/s, 110 obj/s, 32 concurrent): 13023 events/s, 0.16 MiB/s, so the default 1 GiB budget wraps after roughly 108 minutes. Even at ten times the throughput that is ~11 minutes. State the measured rate and how to scale it instead. dial9 was described as the tool for drive stalls. It is not. RustFS does disk I/O on the blocking pool and through io_uring, never on an async worker, so a slow drive never lengthens a poll. Injecting 200 ms of latency on one of four drives cut throughput by 64% and left the poll distribution unchanged (polls >= 5 ms: 49 -> 56; p999: 2.67 ms -> 2.75 ms). Enabling dial9's CPU and sched profilers does not help: sched events are per-worker only, and the CPU profiler samples on-CPU while a stalled drive is an off-CPU wait. Say so plainly, and point at the `rustfs_io_*` metrics instead. What dial9 *is* good for, on the same traces: single polls of 418-625 ms with no fault injected at all — real worker stalls nothing else in the obs stack surfaces. Lead with that. Also link the two upstream issues filed for the gaps we documented: dial9-rs/dial9#658 (writer death unobservable) and #659 (worker-s3 CVEs). Measurements: rustfs/backlog#1157 (D9-11, D9-13, D9-18). Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
6f05a740b3 |
refactor(ecstore): extract the shared io_uring read preamble (backlog#1145) (#4696)
`pread_uring` and `pread_uring_direct` accumulated across seven rounds (#4632/#4635/#4645/#4649/#4653/#4658/#4662) and each carried its own copy of the same open preamble: resolve the bucket path, run the volume access check, resolve the object path, and check the path length. The only real difference is what happens after — one opens buffered and errors as `DiskError`, the other opens `O_DIRECT` and errors as `DirectOpenError` so it can latch an O_DIRECT refusal. Extract that shared sequence into `resolve_uring_object_path`, a blocking helper called inside both `spawn_blocking` closures. Each caller keeps exactly what differs — its open flags, its error type, its metadata/bounds/align handling — and no longer restates the resolution. No behavior change: the same checks run in the same order and produce the same errors (the O_DIRECT path still maps them through `DirectOpenError::Disk`). Net -12 lines. Verified on a real Linux host (16-core, real io_uring): clippy -p rustfs-ecstore --all-targets -D warnings clean; disk::local tests 144 passed, 0 failed — including the io_uring, O_DIRECT, fd-cache, and page-cache-reclaim cases that exercise both read paths. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
ca1463a6c5 |
ci: run e2e smoke subset via nextest e2e-smoke profile (#4674)
ci: run e2e smoke subset via nextest e2e-smoke profile (backlog#1149 ci-4) The e2e-tests job previously ran a single test (delete_marker_migration _semantics) out of ~400 in the e2e_test crate. Define a nextest profile.e2e-smoke whose default-filter selects 17 fast single-node dependency-free modules (63 tests) and run it in the job, reusing the downloaded debug binary. The profile is the single wiring mechanism for e2e tests in CI; admission criteria live in crates/e2e_test/README.md, and docs/testing/e2e-suite-inventory.md records authoritative per-module counts from cargo nextest list. |
||
|
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
|
13f8768e7f |
feat(ecstore): make replication timing intervals env-overridable for tests (#4680)
Introduce RUSTFS_REPL_HEALTH_CHECK_INTERVAL_MS, RUSTFS_REPL_MRF_FLUSH_INTERVAL_MS and RUSTFS_REPL_RESYNC_POLL_MAX_MS so tests and operators can shorten the replication background loops. Defaults are unchanged; invalid values fall back with a warn and values below 10ms are clamped to avoid busy-spin. Add replication_fast_env() e2e helper (backlog#1147 repl-4). |
||
|
Zhengchao An
|
40875026df |
feat(ecstore): add RUSTFS_ILM_DEBUG_DAY_SECS lifecycle time-acceleration for tests (#4677)
feat(ecstore): add RUSTFS_ILM_DEBUG_DAY_SECS lifecycle time-acceleration for tests (backlog#1148 ilm-5) Model a Ceph rgw_lc_debug_interval-style switch: when RUSTFS_ILM_DEBUG_DAY_SECS is set to a positive integer N, lifecycle Days-based rule evaluation treats one 'day' as N seconds instead of 86400, so Days>=1 expiration/transition/noncurrent rules become exercisable in seconds. Unset => byte-identical to production. All Days->deadline conversions funnel through expected_expiry_time(); the new ilm_day_secs() helper (OnceLock-cached in production, env-read under cfg(test)) rescales both the day offset and the default rounding boundary. An explicit RUSTFS_ILM_PROCESS_TIME still wins for the rounding boundary. Absolute Date-based rules are deliberately NOT rescaled. Activation emits a WARN; test/debug only. Refs rustfs/backlog#1148 (ilm-5), rustfs/backlog#1155. |
||
|
Zhengchao An
|
515e14cd42 |
test(ilm): activate ignored ILM integration tests via serial CI lane (#4682)
Implements ilm-1 from the ILM test-strategy plan: the 33 #[ignore]d ILM integration tests (14 in crates/scanner/tests/lifecycle_integration_test.rs, 19 in rustfs/src/app/lifecycle_transition_api_test.rs) never ran anywhere. This adds a dedicated serialized CI job and repairs the app-layer suite, activating 29 of them. - ci.yml: new test-ilm-integration-serial job running the two ILM test surfaces with 'cargo nextest run -j1 --run-ignored ignored-only'. The tests share process-global singletons and fixed ports (9002/9003); serial_test's #[serial] does not serialize across nextest's process-per-test boundary, so -j1 is the serialization mechanism. - app suite repair: the 19 app tests rotted after the InstanceContext migration (usecases resolve the store via AppContext; the test env never installed one, so every store-touching call failed with InternalError 'Not init'). Add a test-only install_test_app_context helper behind the sanctioned context/runtime_sources boundaries and switch the tests from without_context() to from_global(). All 19 now pass. - delete test_lifecycle_transition_basic (+3 orphaned helpers): required an external MinIO at localhost:9000 and slept 1200s; superseded by the mock-tier tests in the same file. - fix a timing flake: poll free-version metadata removal instead of asserting a single read right after remote-object absence. - 3 scanner tests fail on main for product reasons (multipart restore UnexpectedEof; noncurrent transition/expiry after immediate compensation transition on versioned buckets); they keep #[ignore] with a backlog reference and are excluded from the lane by name. Lane: 29 tests, ~43s test time, green twice locally. Refs rustfs/backlog#1148 (ilm-1), rustfs/backlog#1155. |
||
|
houseme
|
5f1a475c56 |
perf(ecstore): stop zeroing pooled shard buffers on the GET path (backlog#1159) (#4681)
`ShardBufferPool::take` handed out a `resize(len, 0)`-ed buffer, and the
reader then overwrote every byte of it. CPU profiling of a cached 1 MiB
GET (device reads = 0, so all cost is CPU) attributed 4.81% of the whole
server to that memset — a buffer pool exists to reuse an allocation, and
memsetting it gives the saving straight back.
The zeroing was load-bearing only because `BitrotReader::read` takes
`&mut [u8]`, which must be initialized. But the reader never reads what
the caller put there, and never returns a partially filled buffer: both
the hashed and the no-hash path either fill the whole shard or fail with
UnexpectedEof, and a hash mismatch is an error rather than a short read.
So the initialization bought nothing observable.
Add `BitrotReader::read_appending(&mut Vec<u8>, want)`, which appends into
the buffer's spare capacity instead of demanding initialized bytes:
* hashed path — unchanged single copy, `extend_from_slice(data)` in place
of `copy_from_slice` into a pre-zeroed buffer, and only after the hash
verifies, so corrupt bytes never reach the caller's buffer;
* no-hash path — `read_buf` writes straight into the spare capacity and
advances the length only over bytes the reader actually wrote, so an
uninitialized tail can never be exposed.
`ShardBufferPool::take` now yields an empty buffer with capacity, and
`read_shard` no longer needs to `truncate`. `read` keeps its old signature
for the remaining callers.
Four tests gate the contract rather than the call:
* `read_appending` is byte-for-byte identical to `read` on both paths;
* a truncated shard is UnexpectedEof, never a partially filled buffer;
* bytes that fail the bitrot hash never reach the caller's buffer;
* `want > shard_size` is rejected;
plus the pool test now asserts the allocation is reused (same pointer) and
never zeroed.
Verified: `erasure::` 213 passed, 0 failed; on a real Linux host
`erasure::` 209 and `disk::local::` 143 pass serially, and the failures
seen in a parallel full-suite run reproduce identically on unmodified main
(they are ENOSPC from a full root filesystem plus pre-existing flakes).
Not claimed: an end-to-end throughput number. The A/B on the bench host was
too noisy to attribute (one rep pair was not fully cached, and its root
filesystem filled mid-run); what is measured is that the removed memset was
4.81% of GET CPU in the pre-change profile.
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
|
b604074217 |
perf(object-data-cache): take fill off the GET path and fix the metrics (#4678)
* fix(object-data-cache): make cache metrics one-increment-per-GET
Rework the object data cache observability so each counter carries one
clear meaning, and drop dead per-entry state.
ODC-17 (backlog#1122): split requests_total, which was incremented by
both plan_get and lookup_body, into plan_total{decision,reason,size_class}
(emitted only by plan_get) and lookup_total{result,size_class} (emitted
only by lookup_body). Each is now incremented exactly once per GET per
layer; help text states this.
ODC-18 (backlog#1123): add a JoinedInflightFill result (label
joined_inflight) so a singleflight waiter is no longer counted as an
insert. record_fill_result now always counts the outcome but records
fill bytes only when non-zero and the duration histogram only when
present, so joined_inflight / skipped_by_mode / skipped_size_mismatch
no longer inflate fill_bytes_total or add non-fill duration samples.
The waiter->JoinedInflightFill mapping lives in MokaBackend (owned by a
concurrent branch); cache.rs handles the variant already.
ODC-29 (backlog#1134): stop refreshing the cache-state gauge on every
lookup, and debounce it on fill/invalidate to at most once per second
via an AtomicU64 millis timestamp, since moka's entry_count is a
settling approximation.
ODC-36 (backlog#1141): give invalidations_total an outcome label
(removed|noop) and skip the gauge refresh on the no-op path. Extend
ObjectDataCacheInvalidationResult with Removed{keys}/NoOp (Success kept
as a transitional variant until MokaBackend reports the removal count);
NoopBackend now reports NoOp.
ODC-30 (backlog#1141): drop the dead content_length/etag/inserted_at
fields (and getters) from ObjectDataCacheEntry, which are redundant with
the moka key identity; the constructor keeps its arity so the
out-of-scope MokaBackend caller still compiles. Gate is_null_version
behind cfg(test).
Tests use a thread-local metrics recorder to avoid the global-singleton
flakiness. Fill-enabled test configs set min_free_memory_percent=0 so
the cgroup gate does not refuse fills in CI pods.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(object-data-cache): move fill off GET path, bound & harden the fill
Implements five object-data-cache audit findings.
ODC-15 (backlog#1120): the GET response no longer blocks on cache fill.
The buffered/materialized body is already in hand, so the fill now runs
in a detached task and the response is built immediately. Singleflight is
made non-blocking: `try_acquire` returns Leader or Busy, and a non-leader
skips its own fill (SkippedSingleflightBusy) instead of waiting on another
request's leader. The inner fill spawn is KEPT: once the index key is
registered, the recheck/undo must complete even if the enclosing fill
future is aborted, so removing it would weaken the cancellation-safety
guarantee (ODC-13 test) and the invalidation-race undo.
ODC-11 (backlog#1116): enforce the fill-concurrency knobs. MokaBackend
now holds a Semaphore sized min(per_cpu * parallelism, max), acquired
after winning leadership and before the memory gate. On saturation it
rejects (try_acquire_owned) with SkippedFillConcurrency rather than
queueing, so the fill path never reintroduces GET-latency coupling.
ODC-14 (backlog#1119): the memory gate no longer does a blocking sysinfo
refresh under a mutex on the async fill path. A dedicated periodic
refresher (tokio interval + spawn_blocking) updates an atomic snapshot
every 5s; allows_fill is now lock-free. The min_free_memory_percent == 0
short-circuit still runs before any snapshot read. No runtime at
construction (sync unit tests) simply keeps the seed snapshot.
ODC-32 (backlog#1137): singleflight no longer emits metrics while holding
the map mutex. try_acquire/remove_entry capture the map length under the
guard, drop it, then emit the gauge/counter.
ODC-07 (backlog#1112): the materialize read is bounded via
take(capacity + 1) so an over-long stream cannot grow the buffer past the
in-memory GET threshold, and any length mismatch is now a hard error
matching the direct-memory GET path, instead of warn-and-serve.
cache.rs is owned by a concurrent branch; this commit only appends the
SkippedFillConcurrency and SkippedSingleflightBusy variants + label arms.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(object-data-cache): report JoinedInflightFill and invalidation outcome
Reconciles the moka_backend half of two metrics-semantics findings after
merging fix/odc-metrics-semantics (which landed the cache.rs half).
ODC-18 (backlog#1123): the singleflight non-leader path now returns
JoinedInflightFill instead of counting as an insert, so N concurrent GETs
of one cold key record one insert, not N. This composes with the ODC-15
redesign: the non-leader does NOT wait for the leader (it already owns the
body and never re-serves from the fill result), so no blocking wait is
reintroduced. Drops the redundant local SkippedSingleflightBusy variant in
favor of the canonical JoinedInflightFill (mapped to no bytes / no duration
by the facade). SkippedFillConcurrency (ODC-11) is retained.
ODC-36 (backlog#1141): MokaBackend::invalidate_object now returns
Removed { keys } / NoOp instead of the transitional Success, so the facade
labels invalidations_total{outcome=removed|noop} and skips the cache-state
gauge refresh on the no-op path.
Tests: duplicate-fill test asserts JoinedInflightFill (bites when reverted
to Inserted); new no-op invalidation test; matching-identity test asserts
Removed { keys: 1 }.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(object-data-cache): drop unused mut on materialize stream binding
The ODC-07 change moves `final_stream` into `AsyncReadExt::take`, so the
`build_get_object_body_with_cache` parameter is no longer mutated in place.
Co-Authored-By: heihutu <heihutu@gmail.com>
* refactor(object-data-cache): close the cross-branch coordination seams
Merging the metrics and hot-path branches left four transitional shims that
only existed because each side could not edit the other's files. With both
sides present they are dead weight, and two of them actively misreport.
- ObjectDataCacheEntry::new took content_length and etag only to keep the
caller in moka_backend.rs compiling, then discarded both. Drop them; the
entry is a Bytes wrapper and the caller now passes only the body.
- SkippedSingleflightClosed lost its only producer when the waiter model was
replaced by Leader/Busy election. Remove the variant.
- The fill-accounting match ended in a wildcard that charged fill_bytes and a
duration to every non-Inserted outcome, so a fill rejected by the memory
gate or the concurrency semaphore — which never touched the backend and
wrote nothing — still inflated fill_bytes_total. Enumerate every variant
explicitly: only outcomes that wrote the body report bytes and duration.
Exhaustiveness also forces a future variant to state its accounting rather
than inherit a wrong default.
- InvalidationResult::Success mapped a no-op to outcome=removed, the exact lie
backlog#1141 set out to fix, and its doc comment claimed MokaBackend still
returned it after that backend had been widened. It has no producer left.
Remove it, and tighten the app-layer test from "any successful variant" to
the real contract: the pre-mutation call reports Removed{keys:1}, the
post-delete call NoOp.
Refs: backlog#1123, backlog#1141, backlog#1135
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
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
|
00536da80c |
refactor(obs): make dial9 telemetry opt-in and actually record events (#4663)
* refactor(obs): make dial9 telemetry opt-in and actually record events
The dial9 Tokio-runtime profiler was disabled by default, yet every build
paid for it, and enabling it produced trace files with no events in them.
Recorded empty traces
---------------------
`build_traced_runtime` called `TracedRuntime::builder()...build(..)`, but dial9
only starts recording in `build_and_start*`. `build` still returns a live guard
whose `is_enabled()` reports true, and still creates and seals segment files —
they just contain a header and no events. It also skipped `with_trace_path`, so
the background worker driving the segment pipeline was never spawned.
Measured on the new smoke example: 310 bytes of bare segment header, against
5640 bytes for the same workload once recording actually starts.
Switch to `with_trace_path(..).build_and_start(..)`.
Cost was unconditional
----------------------
`--cfg tokio_unstable` was a global `[build] rustflags` entry and `rustfs-obs`
depended on `dial9-tokio-telemetry` unconditionally, so all builds depended on
Tokio's non-semver API. Worse, an environment `RUSTFLAGS` replaces (never
appends to) the config-file value, so any caller exporting their own RUSTFLAGS
silently dropped the flag — the long comment in build.yml was a scar from that.
dial9 is now an opt-in feature (`dial9`, plus `dial9-s3` and `dial9-taskdump`),
the global rustflag is gone, and `crates/obs/build.rs` fails the compile if the
feature is on without the flag. Telemetry builds go through `make build-profiling`.
Metrics that could not lie
--------------------------
`rustfs_dial9_{events_total,bytes_written_total,rotations_total,cpu_overhead_percent}`
were hard-coded to zero — a Counter pinned at 0 reads as "nothing happened".
Removed. `rustfs_dial9_enabled` was sourced from the environment, so it read 1
even when the traced runtime failed and the process fell back to a standard
runtime; it is replaced by `rustfs_dial9_supported` (compile-time),
`rustfs_dial9_configured` (intent) and `rustfs_dial9_active_sessions` (reality).
No `writer_healthy` gauge is exported: dial9's `RotatingWriter` can enter its
`Finished` state and stop writing, but exposes no way to observe that, so the
gauge could only ever be hard-coded to 1. Documented as a known gap instead.
Final events were lost
----------------------
The `TelemetryGuard` lived in a `static OnceLock`, which is never dropped, so
buffered events were never flushed at exit. `build_tokio_runtime` now returns
the guard and `run_process` drops it before any exit path.
Also
----
- `disk_usage_bytes` was a `read_dir` + per-file `stat` on the metrics
collection path. It is now sampled by a background task into an atomic.
- `SAMPLING_RATE`/`S3_BUCKET`/`S3_PREFIX` were parsed, warned about, and
discarded. S3 upload is now wired to dial9's `with_s3_uploader` behind
`dial9-s3`; `SAMPLING_RATE` has no upstream equivalent and is removed.
- Wire `with_task_dumps` (async backtraces of stalled tasks), configurable via
`RUSTFS_RUNTIME_DIAL9_TASK_DUMP_{ENABLED,IDLE_THRESHOLD_MS}`.
- Split `telemetry/dial9.rs` into `config`/`state`/`enabled`/`disabled`; the
stub keeps the public API identical so callers need no `#[cfg]`.
- Drop four print-only examples and the manual test bin that exercised the
removed `init_session` scaffolding.
Verified: cargo check/clippy/test across default, `dial9`, and `dial9-s3`;
build.rs correctly rejects `dial9` without `--cfg tokio_unstable`;
`make pre-commit` passes.
Co-Authored-By: heihutu <heihutu@gmail.com>
* docs(obs): document dial9 as an on-demand profiler
scripts/run.sh advertised a `SAMPLING_RATE` knob that was never passed to dial9,
and claimed "CPU overhead < 5% (with sampling rate 1.0)" and "lower values reduce
CPU overhead" on the strength of it. The knob is gone; the guidance built on it
had to go too.
Replace it with what is actually true: dial9 needs a `make build-profiling`
binary, its disk budget evicts oldest-first (so a high poll rate can overwrite
the incident you are chasing), and it cannot be toggled without a restart.
Add docs/operations/dial9-runtime-profiling.md covering the build variants, an
investigation walkthrough, the configuration table, how to read the three
supported/configured/active_sessions gauges against each other, and the upstream
gap that makes writer death only indirectly observable.
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(obs): add a dial9 smoke example that proves events are recorded
The bug this guards against is invisible to every existing signal: with
`build` instead of `build_and_start`, dial9 creates the trace file, seals
segments, and reports `TelemetryGuard::is_enabled() == true` — it simply
records no events. Only the segment's byte count tells the two apart.
Measured on this workload: 5640 bytes when recording, 310 bytes (a bare
segment header) when not. The example asserts >= 2048 bytes, and was verified
to fail with the `build` call restored.
Also correct the comment on the `is_enabled` check in `finish_traced_runtime`.
It claimed to catch "recording silently off"; it does not. It only rejects the
inert guard a lenient config yields after a build failure. Recording is
guaranteed by `build_and_start`, not by that check.
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(rustfs): accept Unsupported runtime telemetry capability
A binary built without the `dial9` feature now reports the runtime-telemetry
capability as `Unsupported` rather than `Disabled`. The distinction matters to
operators: `Disabled` implies the capability can be switched on by setting an
environment variable, which is not true here — telemetry needs a rebuild.
Widen the assertion and pin the new semantics: when `dial9::is_supported()` is
false, the state must be exactly `Unsupported`.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(obs): drop the dial9-s3 feature, its TLS stack is vulnerable
CI's Dependency Review and `cargo deny` both reject the branch: dial9's
`worker-s3` feature depends on aws-sdk-s3-transfer-manager 0.1.3, which pins
aws-smithy-http-client onto hyper-rustls 0.24 and rustls-webpki 0.101.7. That
webpki carries RUSTSEC-2026-0098, -0099 and -0104.
0.1.3 is the latest release of the transfer manager, and 1.2.0 the latest of the
smithy client, so there is nothing to upgrade to. Cargo's feature unification can
add features but cannot drop a transitive dependency, so it cannot be worked
around from here either — the rest of the workspace already resolves to the safe
rustls-webpki 0.103 / hyper-rustls 0.27.
Remove the `dial9-s3` feature and the `with_s3_uploader` wiring. The two S3
environment variables stay parsed and warned about, now naming the real reason
rather than a missing build feature. Trace segments are collected from the output
directory instead. Tracked as D9-14 in rustfs/backlog#1157.
With this, Cargo.lock is byte-identical to main: the PR no longer touches the
dependency graph at all.
Also correct the `dial9-taskdump` documentation. It claimed the feature "compiles
to a no-op elsewhere"; in fact `tokio/taskdump` raises a `compile_error!` on any
target other than linux/{aarch64,x86,x86_64}. Verified by trying to build it on
macOS, which is how the claim was found to be wrong.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
houseme
|
f83f9ada13 |
fix(ecstore): reclaim page cache after io_uring reads (#4662)
fix(ecstore): io_uring reads must reclaim the page cache like StdBackend (backlog#1145) `StdBackend::pread_bytes` calls `fadvise(DONTNEED)` over the range it just read whenever `should_reclaim_file_cache_after_read(length)` holds — `RUSTFS_OBJECT_FILE_CACHE_RECLAIM_READ_ENABLE` (on by default) above a 4 MiB threshold. Large object reads are usually cold, and leaving them resident evicts everything else, so this is a deliberate policy rather than a side effect of how StdBackend happens to read. `pread_uring` and `pread_uring_direct` never did it. Enabling io_uring on a disk therefore turned the policy off silently: shard reads at or above the threshold stayed resident and grew the page cache without bound. This is the same class of mistake as the O_DIRECT loss fixed earlier — copying the read itself while dropping the policy attached to it. It also badly distorts any benchmark. An end-to-end warp GET A/B on a 16-core host (4 MiB objects, conc 64) showed io_uring at 8782 MiB/s against StdBackend's 116 MiB/s. That is not a 76x speedup: with io_uring the run issued *zero* device reads and grew the page cache, while StdBackend read 2950 MiB from the device and shrank it. The two legs were not running the same policy. (Disabling the mmap read path changed nothing — 1.01x — so the mmap-vs-pread difference was not the cause either.) Both io_uring read paths now reclaim the range they read, with the same gate, the same range, and the same error handling as StdBackend. O_DIRECT should leave nothing resident anyway, but a filesystem that quietly buffered the read still has to honour the policy, so `pread_uring_direct` reclaims too. The test measures the policy, not the call: it reads an 8 MiB file through each backend and asks `mincore(2)` how much stayed resident. Crucially it also asserts the reclaim-disabled case leaves the range resident — without that gate, a backend that never reclaimed would pass silently. Verified: with the fix removed, the test fails with "uring: reclaim is on ... 2048/2048 pages remain"; with the fix it passes. Verified on a real Linux host (16-core, real io_uring): clippy --tests -D warnings clean; disk::local tests 143 passed, 0 failed. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Henry Guo
|
99eef032c6 |
fix(ecstore): restore Windows async read trait import (#4652)
* fix(ecstore): restore Windows async read trait import * fix(ecstore): gate async read import to non-Unix |
||
|
houseme
|
904554b417 |
fix(object-data-cache): make memory container-aware and bound cache config (#4655)
* fix(object-data-cache): bound cache config and make memory container-aware Harden the object data cache configuration path so a single bad input degrades to the disabled adapter instead of OOM-killing pods, panicking at boot, or silently mis-sizing the cache (backlog#1110/1113/1114/1115/ 1127/1140/1130). - ODC-05: resolve capacity and the fill gate from the effective (container-aware) memory. Prefer sysinfo cgroup_limits() and use min(host, cgroup) as the total plus cgroup-derived availability, falling back to host values when no cgroup limit constrains. Log the resolved capacity and its basis once at startup. - ODC-08: cap ttl/time_to_idle at 30 days in validate() so an unbounded Duration can no longer trip moka's ~1000-year builder assertion. - ODC-09: drop the max_entry_bytes floor from the derived-capacity clamp so MAX_ENTRY_BYTES can no longer inflate total capacity above the safety clamp; reject an entry larger than the resolved capacity instead. - ODC-10: require an explicit max_bytes to clear max_entry_bytes plus the weigher overhead so a fillable-but-unretainable cache is rejected. - ODC-22: reject max_entry_bytes at/above the u32 weigher boundary. - ODC-35: warn (not reject) when time_to_idle exceeds ttl and document the min(ttl, time_to_idle) expiry interaction. - ODC-25: give numeric env overrides two-valued semantics via a new rustfs_utils::get_env_parse_outcome; a malformed value now disables the whole cache with one aggregated warning instead of silently keeping defaults. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): require identity_keys_max of at least 2 The identity index admits a new key by evicting the oldest one, so a budget of 1 evicts the previous key on every fill and can never hold two live keys of one object at once — a versioned bucket alternating two versions then hits a permanent 0% hit rate. Reject the degenerate value at config validation time, where the adapter turns it into a disabled cache with a warning, since the bounded-eviction policy cannot rescue it at runtime. Handed off from the identity-index batch (backlog#1128), which changed the overflow policy but could not touch validate(). Refs: backlog#1115, backlog#1128 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): let a zero free-memory floor opt out of the gate Making the memory gate container-aware turned it live in CI: the runners are Kubernetes pods, so the gate now reads the pod's cgroup free memory, finds it below the 20% floor, and refuses the fill. Tests that assert a fill succeeds then failed on CI while passing on a developer host, which has no cgroup and falls back to host memory. The gate is behaving correctly — the tests were the ones depending on a live memory reading. Treat min_free_memory_percent = 0 as a deliberate opt-out rather than an invalid value: allows_fill returns early before it touches any snapshot, so admission becomes independent of where the suite runs. Operators gain the same escape hatch. Every test that requires a fill to succeed now sets the floor to 0. The tests that exercise the gate keep it enabled via memory_gated_config, so the ODC-05 coverage they provide is preserved rather than short-circuited. Refs: backlog#1110 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): opt the usecase fill tests out of the memory gate The previous commit exempted the fill-dependent tests it could find, but missed the six adapters built inside object_usecase.rs. Fixing the first batch moved nextest's fail-fast point forward and CI surfaced them: build_get_object_body_with_cache_materializes_once_and_hits_later and ..._uses_cached_body_without_reader_preread both assert a fill lands, and both ran with the default 20% free-memory floor. Set the floor to 0 on all six, matching the sibling test modules. Verified by pinning the gate's snapshot to 0% available — harsher than any CI pod — and confirming these tests still pass, which shows the exemption path never reads memory at all. An exhaustive sweep over every fill-enabled ObjectDataCacheConfig in the tree now shows no remaining site: the only unexempted ones are fill_enabled()'s matches! arm and two adapter tests that assert the adapter is disabled and therefore never fill. Refs: backlog#1110 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
15b8b13698 |
feat(ecstore): cache part-file descriptors for io_uring reads (#4658)
* feat(ecstore): run one io_uring ring per shard on each disk (backlog#1145)
A buffered read that hits the page cache completes inline inside
`io_uring_enter`, so the thread driving a ring performs that read's
memcpy. One ring per disk therefore capped cache-hit reads at a single
core's memory bandwidth: measured on a 16-core host, one driver thread sat
pinned at 100% CPU while throughput stayed flat at ~5 GB/s regardless of
read size, against 50 GB/s for the blocking-pool baseline.
rustfs/uring#6 taught the driver to hold N independent rings, each with its
own thread, pending table, backpressure semaphore, and eventfd. Wire it up:
`UringBackend::try_new` now calls `probe_and_start_sharded`, and
`RUSTFS_IO_URING_SHARDS` selects the count per disk.
The default is a quarter of the available parallelism clamped to `1..=4`,
because the cost is `disks × shards` driver threads (each normally blocked
in `poll(2)`). Any override is clamped to `1..=16`, so a mistyped value can
neither disable the driver (0) nor spawn threads without bound; an
unparseable value falls back to the default.
Effect (warm page cache, 16-core, rustfs/uring's concurrent_pread_bench):
1 MiB, conc 8: 1 shard 4911 MB/s -> 8 shards 47361 MB/s (9.6x);
the blocking-pool baseline is 50662 MB/s
64 KiB, conc 32: StdBackend 153678 IOPS, p999 3030 us
8 shards 345402 IOPS, p999 897 us
64 KiB, conc 128: StdBackend 135155 IOPS, p999 10716 us
8 shards 389047 IOPS, p999 4092 us
Sharding removes the throughput deficit *and* keeps io_uring's tail-latency
advantage, rather than trading one for the other.
Unchanged: io_uring read stays gray-off by default
(`RUSTFS_IO_URING_READ_ENABLE`), reads are byte-for-byte identical to
StdBackend, the per-disk degradation latches and probe cache (backlog#1101)
and the O_DIRECT tiered fallback (backlog#1102) all still apply. Rings stay
per-disk, so a stalled disk cannot starve another disk's rings
(backlog#1055). Bumps the rustfs-uring pin to the merged #6 commit.
Verified on a real Linux host (16-core, real io_uring): cargo clippy
--tests -D warnings clean; disk::local tests 132 passed, 0 failed —
including the existing io_uring and O_DIRECT cases now running on the
sharded driver, plus a new test covering the shard-count default, override,
and clamping.
Co-Authored-By: heihutu <heihutu@gmail.com>
* feat(ecstore): cache part-file descriptors for io_uring reads (backlog#1145)
`pread_uring` opened the file on the blocking pool for every read, so each
read paid a `spawn_blocking` round trip — the very thread hop io_uring
exists to avoid. Sharding the driver (backlog#1145) removed the previous
ceiling and left this as the binding cost. Measured on a 16-core host with
a 4-shard driver, warm page cache:
64 KiB, conc 8: 143942 -> 263054 IOPS (+83%), p999 240 -> 65 us
64 KiB, conc 32: 150128 -> 204876 IOPS (+36%), p999 2508 -> 871 us
64 KiB, conc 128: 129172 -> 361287 IOPS (+180%), p999 15329 -> 3046 us
1 MiB, conc 32: 33875 -> 42301 IOPS (+25%)
At 64 KiB / conc 128 the open is what masked io_uring entirely: with it,
io_uring beat StdBackend by 3.5%; without it, by 189%.
Add a bounded per-disk descriptor cache used by the io_uring read path.
A hit takes no `open` and no `spawn_blocking`, so the read never leaves the
runtime worker.
Why caching a part-file descriptor is safe:
* only `<object>/<data_dir>/part.N` reaches this backend's `pread_bytes`;
`xl.meta` — the one path replaced in place — is read through `read_all`
/ `read_metadata` and never gets here;
* part files are never rewritten in place. A replacement is always
write-new-tmp then `rename`, which swaps the inode, so a cached
descriptor can never observe a torn shard.
Why invalidation is nevertheless REQUIRED: heal reuses the existing
version's `data_dir` and renames a rebuilt shard onto the SAME part path.
A cached descriptor would keep serving the pre-heal (corrupt) inode,
defeating the heal and eroding read quorum. `delete` likewise unlinks a
part that a cached descriptor would keep readable. So `rename_data`,
`rename_file`, and `delete` all call the new
`LocalIoBackend::invalidate_cached_fds` after they mutate, and a 5s TTL
bounds the blast radius should a future mutation path forget to.
Two preamble checks the miss path runs are not silently lost on a hit:
* bounds — the driver only short-reads at EOF (it resubmits otherwise),
so `bytes.len() != length` is exactly the old `meta.len() < end_offset`
check, and now yields the same `FileCorrupt`;
* volume access — skipped while an entry is live. An unreachable disk
keeps serving already-open descriptors for at most the TTL, after which
the re-open re-runs the check. Disk health is tracked independently of
this per-read probe.
Scope: buffered io_uring reads only. The O_DIRECT path keeps opening per
read (its reads are >= 4 MiB, so the open is a small fraction), and
StdBackend is untouched — it must take the blocking hop for the pread
regardless, so caching would buy it only 2-6% while carrying the same
invalidation risk. `RUSTFS_IO_URING_FD_CACHE=false` restores open-per-read.
Verified on a real Linux host (16-core, real io_uring): clippy --tests
-D warnings clean; disk::local tests 135 passed, 0 failed. The new
heal-staleness test first asserts a read still returns the PRE-heal bytes —
proving the cache is live and the hazard real — then that invalidation makes
the healed shard visible. A second test drives `rename_file` and `delete`
through `LocalDisk` to prove those paths actually invalidate, and a unit
test pins prefix invalidation to component boundaries (`a/b` must not drop
`a/bc`).
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
Zhengchao An
|
aaffd3ede8 | fix(ecstore): align Deep verifier shard geometry (#4656) | ||
|
houseme
|
d29e637890 |
fix(object-data-cache): close identity-index races in fill/invalidate (#4657)
fix(object-data-cache): close identity-index races in fill/invalidate (backlog#1117/#1118/#1125/#1128/#1136) The object body cache's fill-vs-invalidation contract (index.insert before cache.insert, then re-check index membership and undo the fill on mismatch) had several races where the identity index and the cache could drift, either poisoning the race metric, breaking invalidation silently, or letting a principal evict a hot body on demand. ODC-12 (backlog#1117) — generation-aware index removal. The eviction listener removed (identity, key) by key equality with no way to tell an evicted old entry from a freshly refilled one under the same key. Verified against moka 0.12.15: an insert over a TTL/TTI-expired but not yet purged entry runs do_insert_with_hash inline during insert().await, and do_post_update_steps awaits notify_upsert INLINE with cause Expired (was_evicted() == true), so the listener deleted the index key the refill just registered and the post-insert recheck then self-destructed the fresh entry (SkippedInvalidationRace). A deferred Size notification for an old generation had the same effect on a cleanly refilled key. Chose Option A (generation token) over Option B (re-insert after cache.contains_key): B removes the key first and only reconciles afterward, which reopens a narrow window where a concurrent invalidate_object misses the key while it is transiently absent from the index. A never removes a key whose generation does not match, so the decision is atomic under the shard lock. The token is the cache entry's Arc pointer captured at fill time (readable from the value moka hands the listener), so no change to the entry type is needed. ODC-13 (backlog#1118) — cancellation-safe insert/recheck/undo. The recheck+undo sat after two awaits inside the S3 GET future; a client disconnect could cancel the fill at the recheck await after cache.insert made the entry visible, stranding a stale body with no index entry. The register/insert/recheck/undo sequence now runs in a spawned task whose JoinHandle the leader awaits: cancelling the await detaches the task, which still finishes the recheck and undo. The dropped leader unblocks waiters as before (SkippedSingleflightClosed), so no waiter is stranded. ODC-20 (backlog#1125) — do not prune in-flight sibling keys. prune_missing could remove another in-flight fill's index key (a different key of the same identity that had registered but not yet published its cache entry), making that fill's recheck fail and delete its own valid entry. Added a read-only ObjectDataCacheSingleflight::has_inflight and consult it in the prune predicate alongside cache.contains_key. ODC-23 (backlog#1128) — bounded eviction instead of clear-and-reject. Exceeding identity_keys_max drained the whole key set and rejected the incoming key, so a principal able to GET identity_keys_max+1 distinct versions could evict an object's hot body on demand, and identity_keys_max=1 on a versioned bucket meant a permanent 0% hit rate. The key set now evicts only the oldest key(s) (the Vec preserves insertion order) and inserts the new key. The insert result carries evicted_keys so the backend removes exactly those from the cache and still caches the newest body. The Overflow variant and the now-unused SkippedIdentityOverflow construction are gone. NOTE: the audit also asks for identity_keys_max >= 2 validation; that lives in config.rs (out of scope here) and is deferred to the config batch. ODC-31 (backlog#1136) — deterministic race coverage. Added a #[cfg(test)] fill barrier between the index insert and the cache insert so the fill-vs-invalidation recheck can be driven deterministically, plus a companion test asserting the identity-budget eviction drops the evicted keys' cache entries. New tests: - index: key_set_bounded_eviction_evicts_oldest_and_inserts_new, key_set_removes_only_matching_generation_token, key_set_duplicate_refreshes_generation_token - starshard_index: identity_index_bounded_eviction_evicts_oldest, identity_index_stale_eviction_preserves_refreshed_key, identity_index_matching_eviction_removes_key - moka_backend: moka_backend_refill_after_expiry_without_maintenance_inserts, moka_backend_concurrent_fills_same_identity_both_insert, moka_backend_identity_budget_evicts_oldest_and_caches_newest, moka_backend_fill_loses_race_to_invalidation, moka_backend_cancelled_fill_still_undoes_lost_race cargo fmt --all, cargo check/clippy/test -p rustfs-object-data-cache all pass (44 tests). Verified the refill-after-expiry and concurrent-fill tests fail without their respective fixes. 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
|
fe682e16e9 |
feat(ecstore): run one io_uring ring per shard on each disk (backlog#1145) (#4653)
A buffered read that hits the page cache completes inline inside
`io_uring_enter`, so the thread driving a ring performs that read's
memcpy. One ring per disk therefore capped cache-hit reads at a single
core's memory bandwidth: measured on a 16-core host, one driver thread sat
pinned at 100% CPU while throughput stayed flat at ~5 GB/s regardless of
read size, against 50 GB/s for the blocking-pool baseline.
rustfs/uring#6 taught the driver to hold N independent rings, each with its
own thread, pending table, backpressure semaphore, and eventfd. Wire it up:
`UringBackend::try_new` now calls `probe_and_start_sharded`, and
`RUSTFS_IO_URING_SHARDS` selects the count per disk.
The default is a quarter of the available parallelism clamped to `1..=4`,
because the cost is `disks × shards` driver threads (each normally blocked
in `poll(2)`). Any override is clamped to `1..=16`, so a mistyped value can
neither disable the driver (0) nor spawn threads without bound; an
unparseable value falls back to the default.
Effect (warm page cache, 16-core, rustfs/uring's concurrent_pread_bench):
1 MiB, conc 8: 1 shard 4911 MB/s -> 8 shards 47361 MB/s (9.6x);
the blocking-pool baseline is 50662 MB/s
64 KiB, conc 32: StdBackend 153678 IOPS, p999 3030 us
8 shards 345402 IOPS, p999 897 us
64 KiB, conc 128: StdBackend 135155 IOPS, p999 10716 us
8 shards 389047 IOPS, p999 4092 us
Sharding removes the throughput deficit *and* keeps io_uring's tail-latency
advantage, rather than trading one for the other.
Unchanged: io_uring read stays gray-off by default
(`RUSTFS_IO_URING_READ_ENABLE`), reads are byte-for-byte identical to
StdBackend, the per-disk degradation latches and probe cache (backlog#1101)
and the O_DIRECT tiered fallback (backlog#1102) all still apply. Rings stay
per-disk, so a stalled disk cannot starve another disk's rings
(backlog#1055). Bumps the rustfs-uring pin to the merged #6 commit.
Verified on a real Linux host (16-core, real io_uring): cargo clippy
--tests -D warnings clean; disk::local tests 132 passed, 0 failed —
including the existing io_uring and O_DIRECT cases now running on the
sharded driver, plus a new test covering the shard-count default, override,
and clamping.
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
houseme
|
baf7e899b9 |
fix(ecstore): bound every walk filesystem call by the stall budget (#4650)
The listing path no longer has a wrapper-level total timeout, so any filesystem call the walk makes without a stall bound would have no liveness bound at all. The first pass covered list_dir and read_metadata but left four reads unbounded: the volume access() probe and fs::metadata() in walk_dir, the xl.meta read in its dir-object branch, and is_empty_dir() in scan_dir. Split the helper so reads that do not yield a Result go through the same budget, and route the four remaining calls through it. A hung drive now fails those reads with DiskError::Timeout instead of parking the walk until the merge consumer's own stall timeout aborts it. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
8b233cdd94 |
fix(ecstore): keep O_DIRECT for eligible reads via native io_uring (backlog#1102) (#4649)
fix(ecstore): keep O_DIRECT for eligible reads via native io_uring (rustfs/backlog#1102)
Wire ecstore's io_uring read backend to rustfs-uring's native aligned
O_DIRECT read (`read_at_direct`, merged in rustfs/uring#3), so an
O_DIRECT-eligible read keeps BOTH io_uring's async submission AND
O_DIRECT's page-cache bypass instead of trading one for the other.
`UringBackend::pread_bytes` now selects the read shape by a tiered,
per-disk-latched ladder — never a blanket downgrade:
1. io_uring latched off for this disk (backlog#1101) -> StdBackend.
2. O_DIRECT-eligible + native path usable -> pread_uring_direct: open
the file O_DIRECT, probe the device alignment once (cached), and
read via read_at_direct. Best path: async + no cache pollution.
3. O_DIRECT-eligible but the fs refused O_DIRECT earlier (latched)
-> StdBackend aligned path (which itself degrades to buffered).
4. non-O_DIRECT read -> buffered pread_uring.
Latching keeps a failure from being re-attempted per-read: an fs that
refuses O_DIRECT (EINVAL/EOPNOTSUPP on open) latches the native direct
path off for that disk; a restriction-class errno from the read latches
io_uring off wholesale (backlog#1101). Any other error falls back for
that one read without masking a genuine data problem as a permanent
downgrade. The alignment comes from the existing probe_direct_io_align
(statx STATX_DIOALIGN, default 4096) and is cached in a per-disk
OnceLock so the probe runs at most once.
This replaces the interim guard that routed every O_DIRECT-eligible read
to StdBackend (which disabled io_uring on the hottest shard reads); the
native path is now the default and StdBackend is only a fallback tier.
Bumps the rustfs-uring pin to the merged #3 commit.
Verified on a real Linux host (16-core, real io_uring + O_DIRECT):
cargo check --tests, clippy -D warnings, and disk::local tests all pass
(128 passed, 0 failed), including uring_preserves_o_direct_for_eligible_reads
across unaligned ranges.
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
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> |
||
|
houseme
|
0ad4a26056 |
fix(ecstore): keep O_DIRECT for eligible reads when io_uring is enabled (backlog#1102) (#4645)
fix(ecstore): keep O_DIRECT for eligible reads when io_uring is enabled (rustfs/backlog#1102) UringBackend::pread_uring opens the file buffered, so with io_uring enabled an O_DIRECT-eligible read (RUSTFS_OBJECT_DIRECT_IO_READ_ENABLE + length >= the threshold) silently lost O_DIRECT and polluted the page cache — a behavior change the moment io_uring was turned on for a disk that uses O_DIRECT. The io_uring backend is gray-off by default, so no deployment is affected, but the gap had to close before it can be enabled anywhere O_DIRECT is in use. Route O_DIRECT-eligible reads back through StdBackend's aligned path (which itself falls back to buffered when the filesystem rejects O_DIRECT). A native aligned O_DIRECT read in the driver remains part of rustfs/backlog#1102. Adds uring_preserves_o_direct_for_eligible_reads: with BOTH flags on and the threshold at 1, unaligned ranges still return exactly the requested bytes. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
dc3099bf0f |
feat(ecstore): run bucket operations on the store's own instance context (#4642)
backlog#1052 S7 — the final piece: full bucket-namespace isolation between embedded servers in one process. Server B's requests resolved B's own ECStore (per-server dispatch landed earlier), but the store's bucket operations still went through ambient process facades, so both servers effectively operated on the FIRST server's disks and metadata: - LocalPeerS3Client::local_disks_for_pools() called all_local_disk() (the ambient disk registry = the first published store's context), so list/make/delete/heal bucket scanned and wrote the wrong volumes. - BucketMetadata::save() persisted through the ambient object handle, so a second server's bucket metadata landed in the first server's .rustfs.sys; set/remove/get/created_at all used the ambient metadata system. Now the whole chain is bound to the owning store's InstanceContext: - S3PeerSys/LocalPeerS3Client gain *_with_instance_ctx constructors and operate on that context's registered disks; ECStore::new (and the test store builder) pass the store's context. The legacy constructors keep the bootstrap default. - BucketMetadata::save_with_store persists through an explicit store; BucketMetadataSys::persist_and_set uses the system's own api handle. - metadata_sys gains instance-scoped variants (get_in / created_at_in / set_bucket_metadata_in / remove_bucket_metadata_in) that resolve the context's metadata system and fall back to the ambient default before the instance cell is initialized (early startup, unchanged behavior). - The store's bucket handlers (make/get_info/list/delete + the table-bucket delete guard and emptiness check) use the per-context variants and this instance's disks. Acceptance (e2e): two embedded servers with different credentials are now isolated end to end — each authenticates only its own key, neither sees the other's buckets or objects, and both data planes stay intact. The embedded module doc drops the shared-IAM caveat. 579 ecstore bucket/metadata/peer regressions plus the embedded basic and deferred-IAM e2e stay green. |
||
|
houseme
|
e6e4aef45b |
test(ecstore): cover the io_uring per-disk probe cache hit (backlog#1101) (#4641)
test(ecstore): cover the io_uring per-disk probe cache hit (rustfs/backlog#1101) Follow-up to #4635. The per-disk negative probe cache was implemented but had no dedicated test. Add uring_probe_cache_skips_known_unsupported_disk: a root recorded in URING_UNSUPPORTED_DISKS makes try_new return None without a fresh probe. Completes the #1101 acceptance ("cache hit does not re-probe"). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
d1daa97ac5 |
fix(ecstore): keep local path startup fail-fast under Kubernetes (#4640)
The startup topology convergence auto-detection (added in #4631) checked Kubernetes before endpoint style, so a local path deployment (single or multi-drive, non-distributed) running inside Kubernetes was classified as orchestrated with an effectively unbounded wait window. Path-style endpoints have no hostnames to resolve, so the wait never actually triggers and runtime behavior is unchanged, but the startup log then advertised mode=orchestrated / wait_timeout=MAX for a purely local deployment, which is misleading and contradicts the documented policy (local path endpoints -> fail-fast). Order the auto-detection by endpoint style first: only distributed URL endpoints, which resolve hostnames, pick orchestrated (Kubernetes) or bounded (otherwise); local path endpoints stay fail-fast regardless of the platform. Update the doc comment to match and add a policy case locking Kubernetes + local path to fail-fast. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Henry Guo
|
24d0c09347 |
fix(admin): keep datausage live refresh stable (#4630)
fix(ecstore): keep fuller data usage overlay Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
houseme
|
7dc03603da |
feat(ecstore): add io_uring degradation latch and per-disk probe cache (#4635)
feat(ecstore): io_uring runtime degradation latch + per-disk probe cache (rustfs/backlog#1101) Follow-up to #1104. Add the runtime half of the io_uring degradation contract: - Runtime latch: UringBackend gains an `active` flag (mirrors DirectIoReadState::supported). A read that returns a restriction-class errno (EPERM/EACCES/ENOSYS/EOPNOTSUPP) — io_uring became unusable on this disk — trips the latch, and every further read goes straight to StdBackend with no more per-read io_uring attempts. Data errors (EIO), missing files, and parameter errors do NOT latch, since StdBackend would hit them too. - Per-disk probe cache: a process-wide negative cache of disks whose probe failed, so try_new skips re-probing (creating a ring + driver thread) on reconnect. Tests: the errno classifier (restriction-only) and a latched-off read that still returns correct bytes via StdBackend; the #1104 differential test still passes under real io_uring. Co-authored-by: heihutu <heihutu@gmail.com> |