mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 16:28:15 +00:00
173e5ddc060aae9b1dfa6cd5940fba2d3b60e37d
4383 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Zhengchao An
|
173e5ddc06 |
build(make): require cargo-nextest for make test with explicit escape hatch (#4672)
build(make): require cargo-nextest for make test with explicit escape hatch (backlog#1153 infra-14) cargo-nextest changes test semantics — it runs each test in its own process (so serial_test's #[serial] mutex does not serialize across tests) and is the only runner that honours .config/nextest.toml [test-groups] (e.g. the ecstore-serial-flaky serialization guard). CI installs and runs nextest, but `make test` only warned when it was missing and then silently fell back to `cargo test`, which runs with different serialization behaviour than CI and can mask or invent flakes. Make cargo-nextest a hard dependency of `make test`: - Missing nextest now fails `make test` with a clear message and install instructions (`cargo install cargo-nextest --locked` or a prebuilt binary via https://nexte.st/docs/installation/). - Set RUSTFS_ALLOW_CARGO_TEST_FALLBACK=1 to opt into the plain `cargo test` fallback anyway; it prints a loud warning that serialization semantics differ from CI and .config/nextest.toml test-groups will NOT apply. - The check stays cheap (command -v). The warn-only test-deps prerequisite is dropped from check.mak in favour of recipe-level gating. Install docs live in the make-layer error message plus a header comment in tests.mak, with a single-line note in CONTRIBUTING.md (kept minimal to avoid conflicting with #4667 / infra-9, which rewrites the verification sections). No CI change: .github/workflows/ci.yml already runs cargo nextest and .github/actions/setup/action.yml already installs it. Refs rustfs/backlog#1153 (infra-14), master plan #1155. Signed-off-by: Zhengchao An <anzhengchao@gmail.com> |
||
|
Zhengchao An
|
4310b55238 |
ci: add schedule-failure-issue composite action and wire nightly alerts (#4671)
Scheduled workflow failures previously went unnoticed (15 consecutive red weekly s3-tests sweeps, silent perf nightly failures). Add a reusable composite action that opens a tracking issue titled "[scheduled-failure] <workflow name>" — or appends a comment to the existing open one (dedupe key = workflow name) — with the run URL, run attempt, and failed job names, labeled "infrastructure". Wire it into the scheduled paths of e2e-s3tests, mint, fuzz (nightly) and performance-ab via a final alert-on-failure job gated by "always() && github.event_name == 'schedule' && contains(needs.*.result, 'failure')", with issues:write scoped to that job only. e2e-s3tests and mint previously had no permissions key; they now get workflow-level contents:read, reducing every other job's token. Add a dispatch-only drill workflow that forces a job failure and runs the action through the exact consumer wiring, for end-to-end verification. The ci-7 e2e nightly does not exist yet; it is recorded as a pending consumer in the action README. Refs: rustfs/backlog#1149 (ci-8), rustfs/backlog#1155 |
||
|
Zhengchao An
|
fd18b1df49 |
ci(perf): fix nightly warp A/B startup failure, make it diagnosable, add small-object + 10MiB cells (#4669)
Both nightly runs of the warp A/B rig failed at server bring-up: `endpoint http://127.0.0.1:9000/health did not become healthy` — exactly 60s after start. The server binds its public listener only after startup converges (erasure-format load + IAM); its own budget (RUSTFS_STARTUP_READINESS_MAX_WAIT_SECS) defaults to 120s, so the rig's 60s health poll gave up before a slow cold start on the shared sm-standard-2 runner finished. The rustfs.log lived under /tmp (not the uploaded target/hotpath-ab/), so the root cause was invisible in the artifact. Root-cause + diagnosability (scripts/run_hotpath_warp_ab.sh): - Health poll is configurable via --health-timeout, default 180s (> the 120s server startup budget). Fails fast if the local server process exits before becoming healthy instead of polling out the full window. - Server logs + a startup-env file now write under OUT_DIR/server-logs/ so they ride along in the CI artifact. On a health failure the rig dumps the last 50 log lines to the job log. Workflow (.github/workflows/performance-ab.yml): - On every run, write gate.md (or, on a pre-gate failure, the failing phase's server-log tails) into $GITHUB_STEP_SUMMARY; short artifact retention. - Short measurement matrix (--duration/--rounds/--cooldown) so the expanded matrix fits; timeout-minutes 90 -> 120 as a Phase-0 stopgap until perf-3 caches the baseline binary (the ~65min double build dominates). - TODO(ci-8/perf-2) hook comment for the failure-alert composite action. Workload cells (absorbed perf-4): add put-4kib/get-4kib (the #4221 fsync regression size, ~-10% @4KiB, previously invisible) and get-10mib (historical large-GET EOF size) to both the PR-labeled and nightly matrices — 6 workloads x 2 phases x 2 drive-sync = 24 cells. A 1KiB cell is deferred to protect the budget until perf-3. Refs rustfs/backlog#1152 (perf-1, absorbed perf-4), rustfs/backlog#1155. |
||
|
Zhengchao An
|
7cfd08d4f5 |
ci(mint): restore multi-SDK pipeline on ubuntu-latest + pin mint digest (#4668)
The mint multi-SDK compatibility pipeline had exactly one recorded run (28730530597, 2026-07-05), which died at "Enable buildx": the self-hosted sm-standard-4 pod it landed on had no /var/run/docker.sock, so `docker buildx create` failed to connect to the Docker API before any test ran. Same heterogeneous-fleet root cause ci-1 fixed for the weekly s3-tests sweep. - Pin the job to GitHub-hosted ubuntu-latest, which reliably ships a running Docker daemon + buildx + python3. Header note records the diagnosis and the dind-sm-standard-2 tradeoff. - Pin MINT_IMAGE default to the linux/amd64 digest for minio/mint:edge resolved 2026-07-10 (was the rolling :edge tag); workflow_dispatch can still override. Header comment records the refresh command. - Quote shellcheck-flagged vars and drop an unused loop var so actionlint passes with zero findings. - Left report-only (ci-3 adds baseline gating later, per adjudication G4) and a TODO(ci-8) alerting hook on the scheduled job. Refs: rustfs/backlog#1149 (ci-2), 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> |
||
|
Zhengchao An
|
d26e06bf46 |
docs: make CONTRIBUTING and Makefile help match real verification gates (#4667)
CONTRIBUTING.md and the Makefile HEADER advertised nonexistent targets (make clippy, make check) and claimed 'make pre-commit' runs format + clippy + check + test, while .config/make/pre-commit.mak actually runs fmt-check + guard scripts + quick-check with no clippy and no tests. CONTRIBUTING.md also claimed the repository ships an automated pre-commit git hook, but no hook is versioned. - Replace phantom targets with the real ones (clippy-check, quick-check, compilation-check) - Document exactly what pre-commit and pre-pr run, gate by gate, with an explicit note that pre-commit runs no clippy and no tests - Point contributors to 'make pre-pr' as the full pre-PR gate - Rewrite the git-hooks section to say hooks are optional and not versioned; setup-hooks only marks an existing hook executable No make targets were added or changed; docs/help truth only. Refs: rustfs/backlog#1153 (infra-9), rustfs/backlog#1155 |
||
|
Zhengchao An
|
89ea931ee1 |
test(ci): strict nextest ci profile with quarantine + flake policy (#4666)
test(ci): add strict nextest ci profile with quarantine + flake policy Formalize the existing ecstore-serial-flaky mechanism into a strict CI gate (ci-10, absorbs infra-15; backlog#1149). - .config/nextest.toml: add [profile.ci] with global retries=0 (never mask a new race's first occurrence), fail-fast=false, and JUnit output at target/nextest/ci/junit.xml. Add a quarantine section where flaky tests get retries=2 under the ci profile only; each entry links one OPEN issue. First members are the two backlog#937 ecstore groups (concurrent_resend_same_part_commits_one_generation and store::bucket::tests::bucket_delete_*), which keep their existing ecstore-serial-flaky test-group serialization. Local default profile still never retries. - .github/workflows/ci.yml: run the main test step with --profile ci and upload the JUnit report (if: always(), 3-day retention, run-number in name). The migration-proof step stays on the default profile to avoid clobbering the ci JUnit artifact (its tests are not quarantined). - docs/testing/README.md: new skeleton (owned by backlog#1153 infra-11) holding the flake policy: discover -> open issue within 24h -> quarantine with issue link -> fix or delete within 30 days. AGENTS.md points to it. Refs: rustfs/backlog#1149, rustfs/backlog#937, rustfs/backlog#1155 |
||
|
Zhengchao An
|
f0b1202b65 |
ci(s3tests): fix weekly sweep runner infra (pin ubuntu-latest + setup-python) (#4665)
The weekly full ceph/s3-tests sweep (e2e-s3tests.yml, schedule path) has failed every recorded run. The 2026-07-05 run (rustfs/rustfs #28727691591) died at "Install Python tools" with `No module named pip`: the self-hosted `sm-standard-4` label is served by heterogeneous runners and the scheduled job landed on a minimal pod with neither `pip` (bare python3) nor `docker.sock`, so no test ever executed. Make the infrastructure assumptions explicit instead of trusting drifting self-hosted runner state: - Pin the job to GitHub-hosted `ubuntu-latest`, which reliably ships Docker, docker compose, python3 and pip. - Add an explicit actions/setup-python step before any pip usage so the workflow stays correct across runner-image drift. - Document the fix and rationale in the workflow header comment. Also quote the shell variables flagged by shellcheck (SC2086) in the Docker build/run steps and drop the unused loop variable (SC2034) so actionlint passes with zero findings on the changed file. The PR gate (ci.yml s3-implemented-tests) is unaffected: it avoids Docker via DEPLOY_MODE=binary and defers pip setup to run.sh's self-bootstrap. Refs: rustfs/backlog#1149 (ci-1), rustfs/backlog#1155 |
||
|
Zhengchao An
|
2d1323d2f0 |
ci(fuzz): narrow PR triggers and cover all fuzz targets (#4664)
Re-enable the Fuzz workflow (disabled_manually) with two changes that address the congestion that likely caused it to be disabled: - Narrow the PR trigger paths to fuzz/**, scripts/fuzz/** and fuzz.yml only. The broad crate paths (crates/ecstore|filemeta|utils) queued a ~45min fuzz-build on nearly every PR; coverage of those crates moves to the nightly schedule, which fuzzes against main. - Expand the smoke matrix, nightly matrix, run.sh default set and the fuzz-build staging/upload steps from 3 to all 5 buildable targets: archive_extract, bucket_validation, local_metadata, path_containment, policy_ingress. archive_extract and policy_ingress were previously in no matrix. The two *_storage_api.rs files are `mod` submodules of their parent targets (bucket_validation, path_containment), not standalone bins, so fuzz/Cargo.toml registers exactly 5 fuzz bins. Smoke jobs run one target per parallel matrix job (-max_total_time=60), so wall-clock stays per-target, well under the 15min budget for a fuzz-only PR. A TODO(ci-8) hook marks where scheduled-failure alerting will attach on the nightly job. Refs rustfs/backlog#1149 (ci-9, absorbed sec-13), rustfs/backlog#1155 |
||
|
Zhengchao An
|
36428bc490 |
fix(rustfs): allow drop_non_drop on dial9 guard for non-Drop feature sets (#4679)
Under feature sets where the dial9 session guard carries no Drop impl (e.g. --features sftp, after #4663), `drop(dial9_guard)` in the startup entrypoint trips clippy::drop_non_drop and fails the sftp Test and Lint job. The drop is an intentional flush point where the guard does implement Drop; annotate it so it compiles clean across all feature combinations. |
||
|
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 |
||
|
Henry Guo
|
f16878ef23 |
fix(table-catalog): pass PyIceberg live smoke (#4637)
* fix(table-catalog): pass PyIceberg live smoke * fix(table-catalog): satisfy live smoke clippy --------- Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
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) | ||
|
Zhengchao An
|
40cf94f243 | docs(tier-ilm): note local-first invariant for expire/GET race fix (#4651) | ||
|
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
|
8ffff47529 |
chore(deps): update workspace dependencies (#4643)
Run `cargo update` followed by `cargo upgrade --exclude ratelimit` on the latest main to pull the newest compatible versions. Manifest bumps (Cargo.toml): - md5 0.8.0 -> 0.8.1 - regex 1.12.4 -> 1.13.0 - sysinfo 0.39.5 -> 0.39.6 Lockfile-only bumps: der 0.8.1, dial9-tokio-telemetry 0.3.14, lru 0.18.1, regex-automata 0.4.15, symbolic-common/symbolic-demangle 13.9.0, and zlib-rs 0.6.6. Resolver re-selection moves crypto-common 0.1.7 -> 0.1.6 and generic-array 0.14.7 -> 0.14.9 (both semver-compatible). `ratelimit` is held at 0.10.1 as requested; `pollster` 1.0.0 is an incompatible major bump and is intentionally not taken. Verification: cargo check --workspace --all-targets, cargo clippy --all-features -D warnings, cargo nextest run --all --exclude e2e_test (8467 passed), cargo test --all --doc, and cargo deny check advisories/sources/bans/licenses all pass. 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> |
||
|
Zhengchao An
|
cefbe2ccc9 |
fix(test): allow InstanceContext bucket_metadata_sys reinitialization for tests (#4638)
The OnceLock introduced in
|
||
|
Zhengchao An
|
5e80980698 | test(ecstore): pin sorted scan_dir stream for dir-marker with children (backlog#1068) (#4626) | ||
|
唐小鸭
|
7ab1abee80 |
fix(admin): allow site replication peer edits (#4623)
* fix(site-replication): align IAM and bucket metadata replication * fix(admin): allow site replication peer edits |
||
|
Zhengchao An
|
f403ed1c2e |
feat(embedded): allow multiple embedded servers to coexist in one process
* feat(embedded): allow multiple embedded servers to coexist in one process
backlog#1052 S5: turn the embedded startup guard into a sequential lock
and make every write-once shared cell tolerate a second embedded start.
Two RustFSServers on different ports and volumes now start, run, and
shut down independently in the same process.
- EMBEDDED_SERVER_STARTED is released once each startup hands off; a
second startup that runs after the first is no longer rejected.
- The second embedded server constructs its own InstanceContext instead
of adopting the process bootstrap one, so region/endpoints/deployment
id land on that context (the first server keeps adopting bootstrap to
keep single-instance ambient facades unchanged).
- Startup-time tolerant paths:
- action_credentials publish treats AlreadyInitialized as success
(per-server ActionCredentialHandle already holds the real creds).
- GLOBAL_RUSTFS_PORT warns instead of panicking on a second set.
- Observability install returns Ok when the process subscriber is
already set — the second server reuses it.
- New acceptance test proves two servers start, respond on their own
ports, and one can be shut down without disturbing the other; the
survivor keeps serving S3 requests. IAM and root-credential lookup
still share a process domain (a second server whose creds differ from
the first will fail signature validation), tracked as a follow-up.
- Embedded doc rewritten: 'Limitations' → 'Multi-instance status'; the
AlreadyStarted error is now scoped to concurrent startups only.
The remaining work in #1052 is the auth path per-server dispatch and the
matching data-plane routing so two servers with different credentials
serve independent buckets end-to-end.
* feat(app): per-server auth and application context for multiple embedded servers (#4633)
backlog#1052 S6: each embedded server now authenticates against — and
its request path resolves — its OWN application context, so two servers
with different root credentials each accept their own access key and
reject the other's.
- AppContext is per-server: ensure_startup_after_iam constructs a fresh
context around this server's store + IAM + KMS and installs it into the
server's own ServerContextSlot, then publishes it as the process
default first-writer-wins (publish_global_app_context) for legacy
ambient readers. The old 'reuse the global if present' path is gone.
- FS::check (the S3 data-plane access gate) resolves auth against
self.server_ctx's context: check_key_valid gains a _with_context
variant that takes the root credentials and IAM system from an explicit
context (None = ambient, unchanged for all 140+ existing callers). The
region and the server context slot are published into the request
extensions for downstream handlers.
- Each embedded server seeds its own root credentials into its context
(ActionCredentialHandle.publish) at startup, so credential validation
no longer falls back to the first server's process-global identity.
- The bucket/object/multipart use-cases resolve their store from the
server's context (bucket_usecase_for/object_usecase_for/... take &FS).
New acceptance test: two servers with distinct credentials each
authenticate with their own key and reject the other's.
KNOWN FOLLOW-UP: full bucket-namespace isolation still requires threading
the instance context through the lower ecstore data plane (peer_sys /
disk registry / bucket-metadata reads still resolve via the process
GLOBAL_OBJECT_API), so the two servers do not yet present independent
bucket listings even though each holds its own store. That deeper pass —
a continuation of the #939 object-graph ctx threading — is the remaining
work on #1052.
Stacked on the S5 guard change.
|
||
|
houseme
|
6a81353785 |
feat(ecstore): converge startup topology under a wait-mode policy (#4631)
Multi-node startups (Kubernetes StatefulSets, bare-metal, VMs) can hit transient DNS/local-host resolution failures while peers and headless service records are still coming up. Previously endpoint resolution retried DNS for a fixed 90s window and then returned an error, which propagated out of run() and exited the process; in Kubernetes this drove repeated kubelet restarts even though the cluster eventually converged. Introduce a startup topology convergence policy with three modes: - orchestrated: wait effectively indefinitely for recoverable DNS/topology errors so the process stays Running (readiness stays false) instead of exiting; defer the DNS-IP cross-port validation because headless-service records can still be flapping. - bounded: wait a finite window (default 3m) then fail with an action-oriented error listing host/stage/elapsed and remediation hints. - fail-fast: fail on the first non-transient resolution error. Mode is auto-detected (Kubernetes -> orchestrated, distributed URL endpoints -> bounded, local path endpoints -> fail-fast) and can be overridden via RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE, RUSTFS_STARTUP_TOPOLOGY_WAIT_TIMEOUT and RUSTFS_STARTUP_TOPOLOGY_RETRY_MAX_DELAY. Also normalize divergent local/remote verdicts for endpoints that share a host:port, throttle retry warnings, and keep the DNS-independent local same-path checks in every mode. Configuration error handling is unchanged: malformed volumes, mixed styles/schemes, duplicate or root paths, and non-transient errors still fail fast. Read the topology env vars through rustfs_utils::get_env_opt_str for consistent alias handling, and simplify the log_once poisoned-lock branch to unwrap_or_else(PoisonError::into_inner). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
da755eb66b |
feat(ecstore): runtime-probed io_uring read backend, gray-off by default (backlog#1104) (#4632)
feat(ecstore): add runtime-probed io_uring read backend, gray-off by default (rustfs/backlog#1104) Wire the standalone rustfs-uring crate (https://github.com/rustfs/uring) into LocalIoBackend as an opt-in read backend. When RUSTFS_IO_URING_READ_ENABLE is set AND the per-disk io_uring probe succeeds, positioned reads go through the cancel-safe UringDriver; otherwise — default, or on a restricted host, or on any per-read driver error — reads fall back to StdBackend byte-for-byte, so the default build is unchanged. - Cargo.toml: rustfs-uring as a Linux-only git dependency (pinned rev). The guard scripts/check_no_tokio_io_uring.sh allows an explicit io-uring integration; only the tokio "io-uring" runtime feature is banned. - UringBackend mirrors StdBackend::pread_bytes's resolution/access/bounds preamble and only swaps the raw byte read; the other three trait methods delegate to the inner StdBackend. - Backend selection at LocalDisk construction via build_local_io_backend. - Differential test uring_backend_reads_match_std: with the flag set, every positioned read returns byte-identical data (driver-served when io_uring is available, StdBackend fallback otherwise). Verified: cargo check -p rustfs-ecstore on Linux; the differential test passes under real io_uring (seccomp=unconfined). The runtime errno degradation latch, per-disk probe cache, and productionization are tracked in rustfs/backlog#1101/#1102. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
houseme
|
d90e354b11 |
chore(experiments): remove io_uring spike, migrated to rustfs/uring (#4629)
The io_uring cancel-safety spike (imported in #4625) has been migrated to a dedicated repository, https://github.com/rustfs/uring (crate `rustfs-uring`), with the full per-issue commit history preserved. The standalone repo has independent CI that this workspace cannot run: it excludes the crate from the build graph, so its checks passed without ever building or testing it. rustfs/uring CI is green on fmt + clippy, a native leg that runs the suite with real io_uring and fails on any skip, and a docker leg exercising both the graceful-degradation and real-io_uring paths. Removing the in-tree copy keeps rustfs/rustfs clean; the crate will be wired in later as a git dependency behind runtime probing (rustfs/backlog#1051). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
6eb238ce75 |
refactor(app): give each context's credential handle its own copy (#4627)
backlog#1052 S3, fifth slice (also part of what the plan called S4). ActionCredentialHandle was a unit struct forwarding get to the process singleton (rustfs_credentials' GLOBAL_ACTIVE_CRED), so every context served the same credentials regardless of which server it belonged to. The handle now keeps its own copy and gains publish(): - publish() lands on the owning context and tries to publish the process global; readers prefer the owned copy and fall back to the global while the initial startup publish still lands there — ambient callers (iam token signing, request-signature validation) keep working. - Single instance: both cells are written together, so reads are unchanged; two contexts that both published stay isolated even though the global remembers only the first (covered by a new test). - The publish return signals "took effect": true if this handle newly holds credentials OR the global was newly initialized, matching the pre-existing init_global_action_credentials fail-fast contract, now scoped to the handle. The interface trait gains a default-body-friendly signature; the test double implements it as a no-op. The startup publish path (S4) still calls init_global_action_credentials directly; wiring it through the handle is a follow-up. |
||
|
houseme
|
bf81a9bab0 |
fix(experiments): import io_uring cancel-safety spike and apply backlog#1051 audit remediation (#4625)
* chore(experiments): import io_uring cancel-safety spike as audit baseline (backlog#894) Import the Spike 0 io_uring cancel-safety prototype from the closed PR #4381 branch (houseme/p2-spike0-uring-cancel-safety) as the baseline for the backlog#1051 audit remediation. This crate is a standalone workspace and is deliberately kept out of the main Cargo.lock/build graph (NOT production code). Subsequent commits apply the fixes tracked in backlog#1051 sub-issues, one commit per issue. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): drain probe SQE to its CQE before releasing buffer (rustfs/backlog#1053) The probe path had no pending-table backstop: after pushing the read SQE, any early return (`submit_and_wait` error, missing CQE) dropped the probe buffer and file while the read could still be in flight in io-wq, and the caller dropped/unmapped the ring on the error path. If the kernel then wrote the 512-byte result into that freed heap block, it was a use-after-free — the exact bug class this spike exists to prevent, living in its own probe path. Fix: once the SQE is pushed, drain to its CQE via `drain_probe_cqe`, retrying the WAIT on EINTR without re-pushing (the kernel consumed the SQE atomically before the wait). A bounded attempt count prevents a probe against a hung device from blocking forever; on any drain failure the buffer (and file) are `mem::forget`-ed ("leak over UAF") so the kernel can never write into freed memory. Unmapping the ring on its own is safe; only the user buffer must survive. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): split probe/runtime/transient errno classes, guard offset (rustfs/backlog#1059) `is_expected_restriction` folded EINVAL into the "environment restricted" class, but at runtime EINVAL is triple-meaning — offset > i64::MAX (signed loff_t), O_DIRECT misalignment, and setup entries over the cap. Implementing the rustfs/backlog#1048 permanent-degradation latch literally against this class would fault a healthy disk off io_uring on one alignment retry or offset-arithmetic bug. Document that the class is probe-time ONLY and that P2 must split errnos into probe-restriction / runtime-parameter-error / transient (EINTR/EAGAIN). Add a concrete guard: `submit` rejects offset > i64::MAX with an InvalidInput error instead of letting it reach the kernel as a runtime EINVAL. The probe EINTR half of this issue is already handled by the drain loop from rustfs/backlog#1053 (retry the wait, never re-push). Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): abort on driver-thread panic instead of freeing in-flight buffers (rustfs/backlog#1054) The ownership model's "CQE is the only reclamation point" invariant held only while the driver thread never unwound. On a panic inside drive(), Rust drop order freed the `pending` table (every in-flight buffer) before the ring, while the kernel could still be writing into those buffers → mass UAF. `catch_unwind` cannot fix this: the destructors run during the unwind, before the catch boundary. Move ring + pending + backlog into a `DriverState` whose `Drop` checks `thread::panicking()` and calls `process::abort()` BEFORE any field destructor runs — leaving the ring mapped and the buffers allocated (leak over UAF). The capacity-overflow panic that made this reachable (caller-controlled `len`) is closed at the source in the len-guard commit (rustfs/backlog#1057). Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): reject reads above MAX_RW_COUNT to stop u32 truncation (rustfs/backlog#1057) The SQE length field is `len as u32`, so len == 4 GiB became a 0-byte read the kernel answered with res=0 → an Ok(empty) the caller decodes as a false EOF (and len > 4 GiB read only the low 32 bits). Silent truncation (CWE-197), forbidden by the repo's rust-code-quality rules. `submit` now rejects len > MAX_RW_COUNT (2 GiB - 4 KiB) with InvalidInput; the `len as u32` cast in the driver is consequently lossless. This also closes the caller-controlled capacity-overflow panic feeding rustfs/backlog#1054. P2 must chunk reads larger than the cap. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): resubmit short reads to satisfy the whole-range contract (rustfs/backlog#1058) CQE res >= 0 was truncated and delivered as final with no resubmit loop, and Pending did not even store the requested length. io_uring can legally short-read a regular file (io-wq signal interruption, NOWAIT partial page cache, O_DIRECT tail blocks), while LocalIoBackend::pread_bytes is a whole- range contract — a short shard fed to EC bitrot verification surfaces as intermittent, hard-to-attribute integrity/quorum errors. Track offset/nread in Pending and drive a resubmit loop: a short non-EOF read re-queues the remainder into buf[nread..], keeping the entry (and its buffer, and in_flight) until the FINAL CQE of the logical read; res == 0 is treated as a real EOF. The resubmitted SQE reuses the op's user_data, so a late ASYNC_CANCEL from a dropped future still cancels the logical read cleanly. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): bound the shutdown drain and record cancel outcomes (rustfs/backlog#1055) Shutdown made "drain to in_flight == 0" a hard precondition for unmapping the ring, but ASYNC_CANCEL is best-effort: it cannot interrupt a regular-file read already executing in io-wq, so on a D-state/NFS-hung disk the CQE may never arrive and drain-to-zero never terminates — the driver loops forever and shutdown()/Drop join blocks the caller (and any tokio worker) permanently. This is an internal contradiction (safe unmap needs drain; a bad disk makes drain unbounded) in the very environment io_uring exists to handle. Add a bounded-drain escape hatch: after DRAIN_TIMEOUT with ops still in flight, leak the whole DriverState (ring stays mapped, buffers stay allocated — leak over UAF) and exit so shutdown() returns. Soften shutdown()'s hard assert to a warning for that degraded path; clean-drain tests still assert in_flight == 0 themselves. Also record the ASYNC_CANCEL three-state result (succeeded/not-found/already-executing) so the hung-disk signal is observable instead of discarded. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): assert NODROP, monitor CQ overflow, handle EBUSY (rustfs/backlog#1056) In-flight had no upper bound and could exceed CQ capacity (entries=64 → CQ=128) with zero overflow handling: no NODROP check, no overflow read, no EBUSY handling. A lost CQE means its pending entry is never reclaimed, drain never completes and shutdown hangs — and the spike only avoided this by accidental reliance on the io-uring crate's auto-flush + NODROP kernel + poll cadence, all of which P2's eventfd/AsyncFd reaping removes. Assert the NODROP feature at probe (degrade via ENOSYS otherwise), monitor the kernel CQ-overflow counter each turn and surface a non-zero value as fatal, and handle submit() EBUSY as CQ-overflow backpressure (keep the backlog, reap this turn) instead of swallowing it. The hard in-flight bound (permits ≤ CQ capacity) lands with the backpressure work in rustfs/backlog#1060. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): add backpressure with permits released at the CQE (rustfs/backlog#1060) Submission was unbounded (unbounded mpsc + uncapped pending/backlog), so a concurrent large-object read storm had no memory ceiling. The subtler trap: the planned SQ-depth semaphore, implemented the natural RAII way (permit held by the ReadHandle/future), would release permits at future drop while orphan buffers stay resident in the pending table awaiting slow-disk CQEs — decoupling the permit count from resident memory and reopening the DoS surface exactly in the EC quorum-drop hot path. Add a `Backpressure` semaphore sized to the SQ depth (entries < CQ capacity, so CQ overflow is structurally unreachable). `submit` acquires before handing the op to the driver; the driver releases the permit at the CQE (pending-table removal), NOT at future drop, tying the in-flight/memory bound to actual kernel residency. Permits are balanced on the shutting-down reject and send-failure paths, and the driver wakes all waiters on exit. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(experiments/uring): open the probe file via O_TMPFILE instead of a predictable path (rustfs/backlog#1061) The probe wrote a predictable temp path (uring-spike-probe-{pid}-{seq}) with std::fs::write (O_CREAT|O_TRUNC, no O_EXCL/O_NOFOLLOW): a local attacker could pre-plant a symlink there and have the process — often root — truncate and overwrite an arbitrary target (CWE-59/377), with a TOCTOU window between write and open (CWE-367). This probe is the direct blueprint for P2's per-disk startup probe, so copied verbatim it becomes a production vulnerability. Open via O_TMPFILE (anonymous inode, no name → nothing to plant a symlink at, no TOCTOU, no leftover), falling back to O_CREAT|O_EXCL|O_NOFOLLOW + 0600 + per-process nonce + immediate unlink on filesystems without O_TMPFILE. P2's per-disk probe should create inside the tested data-disk directory the same way, which also validates that disk's filesystem + io_uring combination. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(experiments/uring): correct invariant 2 mechanism, add invariants 6/7/8 (rustfs/backlog#1063) Invariant 2 (the spike's flagship finding) mis-described the fd-reuse hazard: it claimed the danger window is submission→CQE and that the kernel would "write into someone else's file". Both are wrong — a submitted op holds a struct file reference and is immune to fd close/reuse; the real window is SQE construction (as_raw_fd) → io_uring_enter (backlog residency), and for a READ the consequence is reading the WRONG file, not writing. A P2 optimization reasoning from the false premise (drop Arc<File> after submit / registered-file table) would step straight into it. Correct the mechanism and add the invariants this audit hardened: driver-thread unwind safety (6), backpressure permit released at the CQE (7), reused-buffer content hygiene (8, detailed in rustfs/backlog#1062), plus the errno three-class contract, bounded-drain escape hatch, and short-read resubmit responsibility. Mark the now-remediated items in the leftover list. Co-Authored-By: heihutu <heihutu@gmail.com> * docs(experiments/uring): pin the reused-buffer content-hygiene invariant for P3 (rustfs/backlog#1062) The spike leaks nothing today (fresh zeroed buffer per op + truncate to res), but rustfs/backlog#1048's P3 constraint mandates a driver-owned aligned slab whose buffers are reused across requests as dirty memory. Both the SPIKE invariants and the #1048 constraint address only buffer LIFETIME (UAF), not content hygiene: once buffers are reused, any path that forgets to bound the caller-visible bytes to cqe.res (O_DIRECT full-block read sliced upstream, error path returning the whole buffer) discloses a previous tenant's object data (CWE-226) in an S3 store. Pin invariant 8: reused-buffer bytes visible to the caller must be strictly ⊆ [0, cqe.res). Documented in SPIKE.md and marked at the delivery point in the driver so P3 preserves it; needs a dirty-buffer + short-read regression test. Co-Authored-By: heihutu <heihutu@gmail.com> * test(experiments/uring): pin fd ownership and orphan-integrity directly (rustfs/backlog#1064) The memory-safety assertions were all counter proxies, and invariant 2 (fd owned by the pending table) had zero coverage — deleting Pending.file compiled and left every test green because each test kept its own Arc<File> alive. Add two direct observations: pending_table_owns_fd_after_caller_drop drops the caller's Arc while the op is in flight and asserts F_GETFD still succeeds (only the pending table's clone keeps the fd open; removing that field would close it → EBADF). orphan_in_flight_does_not_corrupt_delivered_reads keeps an orphaned buffer in flight while 64 delivered reads must return byte-exact, asserting the orphan buffer is not reclaimed early and its kernel writes corrupt nothing. A driver-level poison/canary leg is noted as a P2 acceptance-matrix item (ASAN cannot see a kernel write into a freed buffer). Co-Authored-By: heihutu <heihutu@gmail.com> * test(experiments/uring): cover CQ-overflow safety and read boundaries (rustfs/backlog#1065) The suite never approached CQ capacity and never touched EOF/len boundaries. Add no_cq_overflow_under_load (300 ops through a CQ of 128 with backpressure capping in-flight at 64, asserting cq_overflow stays 0 and all deliver), boundary_reads (len==0, read at EOF, a cross-EOF short read delivered to a live receiver exercising the positioned resubmit path, and the rejected huge-len/huge-offset guards), and pipe_half_close_reads_eof (a closed write end surfaces res==0 EOF). Co-Authored-By: heihutu <heihutu@gmail.com> * test(experiments/uring): cover Drop-without-shutdown and de-flake cancel_stress (rustfs/backlog#1066) All tests ended via explicit shutdown(), so the UringDriver Drop impl's live-thread branch (send Shutdown before join) was never exercised; add drop_without_shutdown_drains_and_cancels which drops the driver with ops in flight and asserts the held futures resolve to ECANCELED (a join-first regression or unbounded hang makes it hang). Also de-flake cancel_stress: the exact assert delivered == OPS/2 raced the driver — an even-i read can complete between read_at returning and drop(handle), delivering to the still-live receiver and flipping the split. Relax to the deterministic conservation identity plus delivered >= OPS/2. Co-Authored-By: heihutu <heihutu@gmail.com> * test(experiments/uring): make run-docker.sh assert each leg's real path (rustfs/backlog#1067) Both legs ran the identical cargo test and checked only the exit code, and a skip is indistinguishable from a real pass at that level: leg 1 depended on the host Docker's default seccomp "usually" blocking io_uring, and leg 2 printed "both legs passed" even if every test skipped (vacuous pass — zero real io_uring coverage). Add an explicit seccomp profile (seccomp-block-uring.json) that returns EPERM for io_uring_setup/enter/register so leg 1 deterministically hits the graceful-degradation path regardless of host defaults, and assert leg 1 actually degraded (SKIP lines present) while leg 2 did NOT skip a single test (io_uring really ran). Either violation now fails the harness. Co-Authored-By: heihutu <heihutu@gmail.com> * style(experiments/uring): apply repo rustfmt (max_width=130) to the audit changes Normalize the formatting of the remediation code to the repo rustfmt.toml. Pure formatting; no behavior change. clippy --all-targets -D warnings is clean. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
Zhengchao An
|
008b872414 |
refactor(iam): hand the built IAM system to the AppContext explicitly (#4624)
backlog#1052 S3, fourth slice. The AppContext's IamHandle already owned an Arc<IamSys>, but the value it held was read back from the process singleton after init — so a future second server's context would silently bind to the first server's IAM domain (credentials, policies, users all belong to a specific store's .rustfs.sys). - rustfs_iam gains build_iam_sys(store): construct an IAM system bound to a store without touching the singleton. init_iam_sys wraps it (build, publish first-wins, return the handle — previously Result<()>). - Startup threads the built handle: the inline bootstrap path passes the Arc it just created into ensure_startup_after_iam, which constructs the AppContext from the passed value instead of re-resolving the global. The deferred-recovery path resolves the freshly published global directly (context-first resolution cannot be used there: the AppContext does not exist yet — creating it is the finalizer's job; caught by the embedded deferred-IAM e2e). - IamHandle::is_ready() reports the held system's readiness instead of consulting the process singleton; the dead global re-resolver is removed. token_signing_key stays on the credentials global until S4. 157 iam tests + embedded e2e (basic + deferred recovery) green. |
||
|
Zhengchao An
|
5fbd49a800 |
refactor(ecstore): move the bucket metadata system into InstanceContext (#4622)
backlog#1052 S3, third slice — the hard blocker for a second embedded server's service stage. GLOBAL_BUCKET_METADATA_SYS was a process-global OnceLock whose init ran .set().unwrap(): a second instance's service startup panicked the whole process. The system it guards is inherently per-store (it holds the store handle and that store's bucket→metadata cache), so it now lives on the store's own InstanceContext: - init_bucket_metadata_sys writes the cell of the store it was handed (api.ctx) — no signature change — and keeps the double-init fail-fast, now scoped to the instance (assert on the per-context set). The dist-erasure check for the refresh loop reads the same context. - get_global_bucket_metadata_sys / the crate-private getter behind the ~20 get_*_config helpers resolve through the current_ctx() facade — the published store's context, or bootstrap before that — so every existing reader keeps its exact single-instance behavior. - New acceptance test: two real stores in one process each initialize their own metadata system (the old global cell panicked right there), plus a shared builder extracted from the store-graph test. 201 bucket/metadata regression tests green. |
||
|
Zhengchao An
|
cae6189744 | test(e2e): disable embedded console on raw rustfs spawn sites (#4617) | ||
|
Henry Guo
|
52529403a6 |
test(table-catalog): record live conformance evidence (#4606)
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com> |
||
|
Zhengchao An
|
476352106e |
refactor(app): per-context server-config handle and Arc-owned notification system (#4621)
* refactor(app): give each context's server-config handle its own copy backlog#1052 S3, first slice: establish the per-context state pattern on the smallest subsystem. ServerConfigHandle was a unit struct forwarding both get and set to the process-global GLOBAL_SERVER_CONFIG, so every AppContext shared one config regardless of which context a caller held. The handle now keeps its own copy: a set lands on the owning context and on the process global (ambient readers — the config loader path, the scanner — keep working), and a get prefers the owned copy, falling back to the global while the initial load still publishes there. Single instance: the owned copy and the global are written together, so reads are unchanged. Two contexts that both published stay isolated even though the shared global only remembers the last write (covered by a new test). The global write in set() is the transitional bridge for ambient readers; it goes away when those readers migrate and multi-instance flips on (backlog#1052 S5). * refactor(notify): hand out the notification system as an owned Arc backlog#1052 S3, second slice. GLOBAL_NOTIFICATION_SYS stored the NotificationSys by value and every accessor returned Option<&'static NotificationSys> — a process-lifetime borrow that a per-server AppContext can never hold. The global now stores an Arc and the accessor chain (ecstore runtime sources, ECStore::notification_system, the iam forwarders, the rustfs shims, NotificationSystemInterface and the AppContext resolvers) hands out owned Arcs instead. Call sites are unchanged apart from two borrow adjustments in the rebalance admin handler (pass &Arc where a reference is expected; borrow with as_ref() so the handle survives to the second use). Behavior is identical — same single global instance, first init still wins — but the type now permits a future per-server context to own its notification system (backlog#1052 S3 follow-up). |
||
|
Zhengchao An
|
90c0deff58 |
refactor(admin): resolve the object store through the injected server context slot (#4620)
backlog#1052 S2, second slice: admin request dispatch. Admin operations are registered as &'static dyn Operation, so unlike FS they cannot carry per-server state. Instead, the (per-server) S3Router now holds the server's ServerContextSlot and injects it into every request's extensions at both dispatch points (check_access and call) — the same extensions channel already used for ReqInfo/RemoteAddr. - make_admin_route binds the slot; the HTTP builder passes the same slot the S3 service (FS) uses, so both paths dispatch to the same server. - admin/runtime_sources gains object_store_from_req / an extensions-based variant for handlers that have already moved request fields out; both fall back to the ambient process context when no slot was injected (direct handler tests, non-router paths) — single-instance unchanged. - 27 handler resolution sites across 12 files now resolve per-request; helper functions without request access (11 sites: site_replication state helpers, quota/table_catalog internals, object_zip_download workers) stay on the ambient path until app subsystems become per-server (backlog#1052 S3). - One site (site-replication resync) resolves before the request body is consumed, keeping the original error precedence. New test: the router injects its slot into request extensions by pointer identity. Embedded e2e (basic S3 + deferred-IAM recovery) still green. |