mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
fix(ecstore): harden io_uring integration (#4726)
* fix(ecstore): close the fd-cache open-then-insert race with a generation 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> * fix(ecstore): invalidate the fd cache on the primary object-delete paths (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> * fix(ecstore): close remaining fd-cache invalidation gaps (rustfs/backlog#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> * fix(ecstore): narrow the io_uring latch classes to match StdBackend (rustfs/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> * feat(ecstore): log when a disk latches io_uring off at runtime (rustfs/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> * chore(audit): correct the stale rustfs-uring license-allow rationale (rustfs/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> * fix(ecstore): offload io_uring driver teardown off the tokio worker (rustfs/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> * fix(ecstore): restore StdBackend read parity on the uring paths (rustfs/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> * fix(ecstore): bound worst-case in-flight memory by chunking huge uring 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> * fix(ecstore): gate the io_uring fd cache on RLIMIT_NOFILE headroom (rustfs/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> * test(ecstore): make io_uring test skips visible and gate non-vacuity (rustfs/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> * test(ecstore): cover delete_paths fd-cache invalidation (rustfs/backlog#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> * io_uring audit follow-ups: O_DIRECT chunking, inline invalidation, metrics, 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> --------- Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -112,9 +112,9 @@ jobs:
|
||||
with:
|
||||
fail-on-severity: moderate
|
||||
allow-ghsas: GHSA-2f9f-gq7v-9h6m
|
||||
# rustfs-uring is a first-party Apache-2.0 crate pulled as a git
|
||||
# dependency (backlog#1104); its license cannot be read from the git
|
||||
# source, so allow it explicitly to avoid a spurious unknown-license
|
||||
# warning on every Cargo.lock change.
|
||||
allow-dependencies-licenses: pkg:cargo/rustfs-uring
|
||||
# rustfs-uring is a first-party Apache-2.0 crate, now published on
|
||||
# crates.io (backlog#1104) rather than pulled as a git dependency.
|
||||
# Scope the allow to the exact pinned version so a version bump forces a
|
||||
# conscious re-review of the license/provenance claim (backlog#1181).
|
||||
allow-dependencies-licenses: pkg:cargo/rustfs-uring@0.1.0
|
||||
comment-summary-in-pr: always
|
||||
|
||||
@@ -347,6 +347,39 @@ jobs:
|
||||
if-no-files-found: error
|
||||
retention-days: 1
|
||||
|
||||
uring-integration:
|
||||
name: io_uring Integration (real)
|
||||
# GitHub-hosted ubuntu-latest runs a recent kernel with io_uring and, unlike
|
||||
# a container, applies no seccomp filter that would block io_uring_setup — so
|
||||
# the probe succeeds and the tests exercise the real UringBackend/FdCache/
|
||||
# latch paths instead of the StdBackend fallback (rustfs/backlog#1179). The
|
||||
# self-hosted sm-standard runners cannot guarantee this.
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 30
|
||||
steps:
|
||||
- name: Checkout repository
|
||||
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
|
||||
|
||||
- name: Setup Rust environment
|
||||
uses: ./.github/actions/setup
|
||||
with:
|
||||
rust-version: stable
|
||||
cache-shared-key: ci-uring
|
||||
github-token: ${{ secrets.GITHUB_TOKEN }}
|
||||
cache-save-if: ${{ github.ref == 'refs/heads/main' }}
|
||||
|
||||
- name: Install build dependencies
|
||||
run: sudo apt-get update && sudo apt-get install -y protobuf-compiler
|
||||
|
||||
# RUSTFS_URING_TESTS_MUST_RUN=1 makes the io_uring tests fail instead of
|
||||
# silently skipping if io_uring is unavailable, so this leg can never pass
|
||||
# vacuously. RUSTFS_IO_URING_READ_ENABLE=true selects the UringBackend.
|
||||
- name: Run ecstore io_uring integration tests on real io_uring
|
||||
env:
|
||||
RUSTFS_IO_URING_READ_ENABLE: "true"
|
||||
RUSTFS_URING_TESTS_MUST_RUN: "1"
|
||||
run: cargo test -p rustfs-ecstore uring_ -- --test-threads=1 --nocapture
|
||||
|
||||
e2e-tests:
|
||||
name: End-to-End Tests
|
||||
needs: [ build-rustfs-debug-binary ]
|
||||
|
||||
Reference in New Issue
Block a user