fix(ecstore): harden io_uring integration#4726
Merged
Merged
Conversation
Contributor
|
CLA requirements are satisfied for this pull request. |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
overtrue
changed the title
…ion guard (rustfs/backlog#1176) pread_uring's miss path opened a descriptor on the blocking pool and only then inserted it into the moka cache. moka's invalidations cover only entries present at call time, so a heal/delete commit that invalidated between the open and the insert could not stop the just-opened stale inode from being cached afterwards — serving the pre-heal/pre-delete inode for up to the TTL and defeating the heal. Add an invalidation generation to FdCache, bumped by invalidate_exact and invalidate_under before they touch moka. The read path snapshots the generation before opening and inserts via insert_if_fresh, which refuses the insert if the generation moved during the open and, with a post-insert re-check, removes the entry if an invalidation raced the insert itself. Reads that never miss are unaffected. Co-Authored-By: heihutu <heihutu@gmail.com>
…ths (rustfs/backlog#1175) The fd-cache invalidation contract was only wired into DiskAPI::delete, rename_file and rename_data, but object deletion almost never goes through LocalDisk::delete — DeleteObject(s) reach delete_version, delete_versions -> delete_versions_internal, and delete_paths, all of which remove a version's data dir (move_to_trash / rename_all staging) with no invalidation. A cached io_uring descriptor kept the deleted part.N inode readable for up to the TTL, so a GET in that window could still return deleted data. Invalidate every cached fd under the removed data dir at each site: in delete_version and delete_versions_internal the data_dir uuid and object path are in hand (invalidate_cached_fds_under(volume, "{path}/{uuid}")); delete_paths invalidates under each removed path. A later rollback that restores a data dir just causes the next read to re-open it. Co-Authored-By: heihutu <heihutu@gmail.com>
…log#1177) Three residual paths could keep serving a stale descriptor: - delete_volume removed the whole bucket tree (remove_dir_all/remove_dir) with no invalidation, and the cache-hit read path skips the volume-access check, so a cached fd kept a removed object readable. Add invalidate_cached_fds_for_volume (a per-volume moka predicate) and call it after the bucket is removed. - A retired LocalDisk instance (renew_disk on reconnect builds a fresh one) kept its populated cache alive while still referenced by in-flight ops, so invalidations through the new instance never reached it. close() now clears the backend's cache via clear_cached_fds. - rename_data's post-commit rollback (a commit-metadata fsync failure under strict durability) restored the old data dir without dropping fds cached during the committed window; the streaming branch now invalidates the dst part fds on those rollback paths. The inline branch's rollback runs inside spawn_blocking and is left to the TTL backstop. Co-Authored-By: heihutu <heihutu@gmail.com>
…ustfs/backlog#1171) The runtime degradation classification reused the probe-time restriction errnos, which the driver's C7 contract explicitly warns against, so a single per-file error could latch a whole disk off io_uring: - is_io_uring_unsupported no longer includes EACCES: at read time on an already-open fd it is per-file (an LSM hooks security_file_permission on every read) and StdBackend hits the same denial, so falling back masks nothing and a full-disk latch would be wrong. ENOSYS and EPERM (seccomp/LSM applied after startup) remain. EOPNOTSUPP is now classified per-path by the caller. - pread_uring_direct's read-error arm now mirrors StdBackend: an O_DIRECT-shape error (EINVAL/EOPNOTSUPP) latches only direct_uring.supported so eligible reads take StdBackend's aligned path, instead of over-latching the whole io_uring backend or never latching a read-side EINVAL at all. - try_new only negative-caches genuine restriction-class probe failures in URING_UNSUPPORTED_DISKS; an unexpected (possibly transient) probe failure now falls back without latching, so the next reconnect re-probes. Co-Authored-By: heihutu <heihutu@gmail.com>
…s/backlog#1172) A probe-gated gray release was flying blind: the permanent per-disk `active` latch flipped with no log and no metric, so the only message operators ever saw was the startup "io_uring read backend enabled" line — which stayed true on dashboards even after the very first read latched the disk back to StdBackend forever. Add latch_active_off, which flips the latch with `swap` and logs the true->false transition exactly once at warn with a dedicated event constant, disk root, and errno. Both the buffered and O_DIRECT read paths use it. A fallback/latch metric counter and periodic export of the driver StatsSnapshot (cq_overflow, cancel_already) remain as follow-ups that need rustfs_io_metrics plumbing. Co-Authored-By: heihutu <heihutu@gmail.com>
…ustfs/backlog#1181) The dependency-review allow said rustfs-uring is "pulled as a git dependency", but ecstore now pins it from crates.io. Update the rationale and scope the allow to the exact pinned version (pkg:cargo/rustfs-uring@0.1.0) so a future version bump forces a conscious re-review of the license/provenance claim instead of being waved through on an outdated justification. Co-Authored-By: heihutu <heihutu@gmail.com>
houseme
force-pushed
the
fix/uring-audit-integration-1160
branch
from
31967ef to
40680ca
Compare
…ustfs/backlog#1170) UringBackend held Arc<UringDriver> and had no Drop, so when the last LocalDisk reference dropped in async context (disk reconnect via renew_disk, or shutdown), UringDriver's own Drop ran on that thread — sending Shutdown and joining each shard thread, which can block up to the bounded-drain timeout (5s) on a hung / D-state disk, stalling a tokio worker. Wrap the driver in ManuallyDrop (deref is transparent, so read call sites are unchanged) and add a Drop that takes the Arc and, when a runtime is present, drops it on a blocking thread so the potentially-blocking join never runs on a runtime worker. Off-runtime it drops inline. Co-Authored-By: heihutu <heihutu@gmail.com>
…fs/backlog#1173) Two byte-for-byte parity breaks against StdBackend on the io_uring read paths: - A zero-length read on an fd-cache hit returned Ok(empty) without any bounds check, while StdBackend and the uring miss path return FileCorrupt for an offset past EOF. Fstat the cached descriptor on the length==0 path and match. - reclaim_read_range fadvise(DONTNEED)'d the raw unaligned [offset, offset+len) range, but fadvise only drops fully-covered pages, so the head partial page stayed resident — whereas StdBackend's mmap path reclaims the page-aligned superset. Bitrot shards' 32-byte block headers keep offsets off page boundaries, so this diverged on the common case. Page-align the reclaim window to match the mmap path exactly. (The third parity item from the audit — a failed reclaim fadvise failing the read — is already parity: StdBackend's mmap path propagates the same fadvise error with `?`, so no change is needed.) Co-Authored-By: heihutu <heihutu@gmail.com>
…g reads (rustfs/backlog#1174) The driver's backpressure permits count operations, not bytes, and it zero-fills a full-size buffer per op, so a single unbounded read could pin ~length bytes per permit (128 permits x shards x up to ~2 GiB). ecstore passes a whole part's shard range as one pread_bytes with no upstream chunking. On the buffered path, split reads larger than URING_MAX_OP_LEN (128 MiB) into sequential chunks, awaited one at a time, so worst-case in-flight memory is bounded by permits x URING_MAX_OP_LEN per shard. The threshold is high enough that ordinary shard reads keep the single-op, zero-copy fast path unchanged. The O_DIRECT path (opt-in, alignment-constrained) is left for a follow-up. Co-Authored-By: heihutu <heihutu@gmail.com>
…ustfs/backlog#1178) The fd cache holds up to FD_CACHE_CAPACITY (512) descriptors per disk, but try_new cannot know the disk count and nothing checked the process fd budget. On a bare-metal / non-systemd run with the common 1024 soft RLIMIT_NOFILE, two disks would already exhaust fds with EMFILE surfacing on reads and probes. Check the soft limit at try_new: enable the cache only with ample headroom (>= 16384), otherwise log a warning once and fall back to open-per-read. The packaged systemd unit sets 1,048,576, so tuned deployments are unaffected. Co-Authored-By: heihutu <heihutu@gmail.com>
…ustfs/backlog#1179) The ecstore io_uring tests degrade to a silent pass when io_uring is unavailable (bare `return`s or plain eprintlns), so a CI leg on a restricted runner never exercises the real UringBackend/FdCache/latch paths yet still goes green — an integration regression could merge unseen. Add uring_test_skip: it emits a grep-able `SKIP <name>` line and, when RUSTFS_URING_TESTS_MUST_RUN is set (a CI leg that guarantees io_uring, e.g. a seccomp=unconfined container), panics instead of skipping. Route the silent-skip sites through it. Wiring a dedicated CI leg that sets that env on a capable runner is tracked in the issue; this provides the enforcement mechanism. Co-Authored-By: heihutu <heihutu@gmail.com>
…og#1180) Add an end-to-end test that seeds the descriptor cache with a read, removes the part via disk.delete_paths (one of the primary object-delete entry points that does not go through LocalDisk::delete), and asserts the next read no longer returns the removed inode — pinning the invalidation added in #1175. The sharded cancel-routing half of #1180 is covered in the rustfs-uring PR. Co-Authored-By: heihutu <heihutu@gmail.com>
houseme
force-pushed
the
fix/uring-audit-integration-1160
branch
from
40680ca to
04c0b66
Compare
…trics, CI leg (backlog#1160) (#4729) * fix(ecstore): chunk large O_DIRECT reads too, bounding in-flight memory (rustfs/backlog#1174) The buffered read path already splits reads above URING_MAX_OP_LEN into sequential chunks; do the same for the O_DIRECT path, which was left for a follow-up. read_at_direct aligns each chunk's sub-range internally, and chunk sizes are a multiple of URING_MAX_OP_LEN so a boundary re-read is at most one block. Extract classify_direct_read_error so the single-op and chunked paths share one copy of the EINVAL/EOPNOTSUPP-vs-subsystem latch classification rather than duplicating it. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): invalidate cached fds on the inline rename_data rollback (rustfs/backlog#1177) The streaming rename_data branch invalidates cached part fds on its post-commit rollback paths, but the inline branch runs its commit and rollback inside a single spawn_blocking closure where the async invalidate cannot be called, so it was left to the TTL backstop. Capture the closure's result instead of `??`-propagating it: on error (a commit-metadata fsync failure under strict durability rolls the committed rename back), invalidate the dst part paths at the async level before returning. Inline objects keep their data in xl.meta rather than separate part inodes, so this is largely defensive, but it removes the caveat and keeps the two branches consistent. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(ecstore): export io_uring latch/fallback and driver stats metrics (rustfs/backlog#1172) Complete the gray-release observability. Beyond the warn log added earlier, emit metrics so a dashboard can answer "how much traffic is on io_uring vs falling back, and is any disk degrading": - rustfs_io_uring_latch_off_total — a disk latching io_uring off at runtime. - rustfs_io_uring_read_fallback_total — each io_uring -> StdBackend read fallback (latched-off short-circuit, O_DIRECT error, buffered error). - a low-frequency per-disk exporter of the driver StatsSnapshot as gauges (in_flight, cq_overflow, cancel_already), spawned in try_new. It holds only a Weak reference so it never keeps the driver alive, and drops any temporary strong reference on the blocking pool so a last-reference UringDriver::Drop join never runs on an async worker (rustfs/backlog#1170). submit_errors is deliberately not exported yet: it is a field added in the unreleased rustfs-uring 0.2.0, and ecstore still pins 0.1.0. It lands once the dependency is bumped (rustfs/backlog#1181). Co-Authored-By: heihutu <heihutu@gmail.com> * ci: add a real-io_uring integration leg on ubuntu-latest (rustfs/backlog#1179) The existing self-hosted sm-standard runners cannot guarantee io_uring is available (a container seccomp filter can block io_uring_setup), so the ecstore uring tests degrade to a silent skip and never exercise the real UringBackend/FdCache/latch paths in CI. Add a job on GitHub-hosted ubuntu-latest, which runs a recent kernel with no container seccomp filter, running the uring-named ecstore tests with RUSTFS_IO_URING_READ_ENABLE=true and RUSTFS_URING_TESTS_MUST_RUN=1 — the non-vacuity gate makes the leg fail rather than skip if io_uring is unavailable, so an integration regression can no longer merge green behind a vacuous pass. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com>
houseme
added a commit
that referenced
this pull request
Jul 11, 2026
…#4739) Close the three test-completeness items in rustfs/backlog#1180 that the earlier hardening (#4726, #4729) left unpinned; the other two of the five (sharded cancel routing, bailout-handle error) already landed in rustfs/uring. - rename_data end-to-end invalidation: drive the real `LocalDisk::rename_data` commit (non-inline part, so `invalidate_part_paths` is non-empty) and assert the destination part descriptor is dropped — not merely `rename_file`/`delete`. Both production `rename_data` call sites share this invalidation, so a "fix one copy, miss the other" regression is now caught. The test is non-vacuous: it seeds the cache, removes the on-disk data dir out of band (the cached fd keeps the old inode alive and clears the path for the directory rename `rename_data` performs), and asserts a read still returns the OLD bytes before the commit — which fails outright if the cache is off, so it cannot pass without a live cache. - FD_CACHE_TTL backstop: an injected short TTL proves the cache self-evicts a descriptor with no explicit invalidation; a static check pins the 5s value. - zero-length read bounds parity on the cache-HIT path: a `length == 0` read past EOF must be rejected identically to the miss path and StdBackend, pinning the #1173 fix against regression. Refactors `FdCache::new` to delegate to a private `with_ttl(ttl)` helper so the TTL backstop can be exercised with a short TTL instead of a multi-second wait. Verified: `cargo test -p rustfs-ecstore` on Linux with real io_uring (seccomp=unconfined, RLIMIT_NOFILE raised, RUSTFS_URING_TESTS_MUST_RUN=1 so a degraded skip fails rather than passing vacuously). Co-authored-by: heihutu <heihutu@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Integration-layer half of the rustfs/backlog#1160 io_uring audit — the
UringBackend, its fd cache, and the degradation latches incrates/ecstore/src/disk/local.rs. The read backend stays default-off and probe-gated; every change preserves the byte-for-byte StdBackend fallback. The companion driver fixes are in rustfs/uring#12.Each commit maps to one backlog sub-issue.
Fixes
fd cache correctness
LocalDisk::delete; the primary paths (delete_version,delete_versions_internal,delete_paths) removed a version's data dir with no fd-cache invalidation, so a cached descriptor kept a deletedpart.Ninode readable for up to the TTL. Invalidate under the removed data dir at each site.delete_volume(whole bucket removal), a retiredLocalDiskinstance on reconnect, andrename_data's post-commit rollback could each keep serving a stale fd. Added per-volume and clear-all invalidation, wiredclose()to clear the cache, and invalidate on the streaming rollback paths.degradation / observability
is_io_uring_unsupported(drop EACCES), routed O_DIRECT read-side EINVAL/EOPNOTSUPP to the direct-only latch like StdBackend, and stopped permanently negative-caching transient probe failures.StatsSnapshotexport remain follow-ups needing metrics plumbing.)release hygiene
audit.ymllicense-allow rationale (rustfs-uring is on crates.io now, not a git dependency) and scoped the allow to the exact pinned version.Testing
The io_uring code is
cfg(target_os = "linux"), so it was built and tested in a Linux container.cargo test -p rustfs-ecstoreunderseccomp=unconfinedwithRUSTFS_IO_URING_READ_ENABLE=trueand the non-vacuity gateRUSTFS_URING_TESTS_MUST_RUN=1: all 11 uring tests (fd-cache invalidation incl. the newdelete_pathscase, latch classification, O_DIRECT parity, page-cache reclaim) pass against real io_uring — not the fallback.Release coupling
These changes use only the published
rustfs-uring = "0.1.0"API. The driver-side audit fixes (rustfs/uring#12) reach production only after that PR lands and a new version is published; a follow-up will bump the pin. See #1181.🤖 Generated with Claude Code