feat(uring): bound admission and harden read-driver progress - #16
Merged
Merged
Conversation
Terminate the owned process group even when its leader was already reaped. Cover surviving descendants with FIFO EOF and preserve failures when the group no longer exists. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
…rors Advance kernel overflow/task work even without new SQEs and retry remaining submission work after reaping. Return metadata errors rather than reporting an unconfirmed O_DIRECT tail as EOF. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Add opt-in shared byte admission with aligned allocation accounting, cancellation-safe permit ownership, and shutdown wakeup coverage. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Retry interrupted eventfd transfers and retain bounded wake diagnostics. Stop positioned continuation only after a read CQE confirms safe reclamation, preserving stream and no-cancel semantics. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Register shard count semaphores in shared admission state, synchronize registration with terminal close, and invoke semaphore wakeups outside the registry lock. Cover count-stage closure, late/concurrent registration, and partial FIFO byte cancellation. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Alternate bounded message, allocation, and completion batches while remembering pending local work after eventfd drains. Continue partial submissions only after positive progress to retain idle efficiency and avoid zero-progress retry spins. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Try actual count permits across healthy shards for positioned reads, retain round-robin by default and for streams, and keep the final owner fixed for waits and cancellation. Preserve shared-byte closure and account diagnostics only to the final shard. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Exercise a real pending pipe read with count-stage and byte-stage waiters under the dropped-CQE fault seam. Assert bounded bailout, retained Pending ownership, no waiter submission, and errors for all stranded handles. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
…rnings Extract submit-result classification for deterministic progress, reset, shutdown and ownership checks without injecting real kernel failures. Warn only on changed nonzero CQ overflow counters and explicitly handle counter wrap. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
…ions Reuse normal read validation, ownership and admission while emitting one eager wakeup per final owning shard. Deferred admissions and cancellation retain individual notifications. Add deterministic eventfd tests and native byte-exact/cancellation coverage. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Record native default and all-feature regression results and the rejected A/A calibration without attributing performance benefits. Keep later application and memory-pooling work explicitly pending. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Require identical artifacts and feature-off rows for opt-in A/A calibration. Gate each middle leg against the endpoint mean without candidate attribution while preserving comparison defaults. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Keep ordered prefetch example-local with bounded logical slots and bytes, cancellation-safe next polling, all-slot admission progress and ordered EOF/error termination. Add deterministic semaphore-race coverage and a read-only Linux byte oracle without production API changes or performance claims. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
…uisites Require byte-exact native example execution and precise CLI validation errors. Record current-main application budget, fallback, probe and descriptor-cache integration boundaries separately from the completed driver work. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Exercise registered admission waiters, repeated shutdown requests, new-read rejection and clean conservation on real Linux pipes. Cover eager async shutdown abandonment and isolate the nonclean thread-exit fault in a child process without global environment mutation. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Close count and byte admission before every shutdown request returns, expose advisory thread completion, and reuse the request path for consuming shutdown and Drop. Add an opt-in tokio-runtime adapter that transfers driver ownership before its returned future is polled, with deterministic private lifecycle tests. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Validate strict quota geometry, refusal classification, independent shutdown, deferred-handle retention and clean recycling. Cover capacity-aware batch ownership and isolate permanent whole-quota retention after a leaked one-byte read in a fault child. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Reserve each explicit local byte limit before probing, reject transient shared shortage without waiting, and retain the whole reservation through deferred admission and accepted reads. Keep local shutdown independent and leaked pending buffers charged, with atomic accounting and deterministic ownership tests. Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Co-Authored-By: heihutu <heihutu@gmail.com> Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
Collaborator
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
Author
|
@codex security review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The broad driver and API changes remain draft with performance acceptance outstanding, requiring final human review.
Review effort: Lite
Findings: None
What changed in this PR
This pull request hardens the Linux io_uring driver with bounded admission, shutdown and cancellation safety, recovery improvements, shared budgets, routing, batching, and ordered-prefetch validation.
Changes:
- Adds read limits, shared quotas, shutdown APIs, capacity-aware routing, and batching.
- Improves completion progress, direct-read integrity, cancellation, eventfd handling, and ownership.
- Adds tests, documentation, benchmark tooling, examples, and CI coverage.
| File | Summary |
|---|---|
tests/shutdown.rs |
Shutdown API and lifecycle tests |
tests/shared_budget.rs |
Shared read-budget integration tests |
tests/shard_policy.rs |
Shard policy integration tests |
tests/ordered_prefetch.rs |
Ordered-prefetch tests |
tests/fault_injection.rs |
Fault-recovery tests |
tests/cancel.rs |
Cancellation behavior tests |
tests/admission.rs |
Admission-limit tests |
src/shard_policy_tests.rs |
Shard-selection unit tests |
src/lib.rs |
Public API exports and feature wiring |
src/driver.rs |
Core driver admission, progress, shutdown, routing, and batching |
src/driver_submit_result_tests.rs |
Submission-result tests |
src/driver_loop_budget_tests.rs |
Driver-loop budget tests |
src/driver_fault_recovery_tests.rs |
Driver fault-recovery tests |
src/diagnostics.rs |
Final-owner diagnostic sampling |
src/batch_read_tests.rs |
Batch-read tests |
src/admission_tests.rs |
Admission unit tests |
scripts/test-ordered-prefetch-cli.sh |
Ordered-prefetch CLI gate |
scripts/test_bench_abba.py |
Benchmark behavior tests |
scripts/bench-abba.py |
Benchmark calibration and cleanup |
README.md |
Public API and behavior documentation |
examples/ordered_prefetch/reader.rs |
Ordered-prefetch reader example |
examples/ordered_prefetch/linux.rs |
Linux example support |
examples/ordered_prefetch.rs |
Ordered-prefetch example entry point |
docs/shutdown.md |
Shutdown contract |
docs/shared-read-budget.md |
Shared-budget contract |
docs/rustfs-integration.md |
RustFS integration guidance |
docs/ordered-prefetch.md |
Ordered-prefetch documentation |
docs/optimization-status.md |
Optimization and acceptance status |
docs/fault-recovery.md |
Fault-recovery documentation |
docs/driver-fairness.md |
Driver fairness documentation |
docs/cancellation-efficiency.md |
Cancellation-efficiency documentation |
docs/benchmarking.md |
Benchmarking guidance |
docs/batch-reads.md |
Batch-read documentation |
CHANGELOG.md |
Release notes |
Cargo.toml |
Optional Tokio runtime feature |
.github/workflows/ci.yml |
Ordered-prefetch CI coverage |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
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
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.
Summary
Follow-up to #15; advances steps 2, 3 and 4.1 of rustfs/backlog#2647.
Follow-up verification and examples
Three additional commits (
6927bb0,f135f1d,bfc2a5d) add explicit same-binary calibration, an example-only bounded ordered reader, and native CLI/CI gates. No production driver code, dependencies or defaults change in this follow-up.Shutdown API follow-up
Implementation/documentation head
a942ac1addsrequest_shutdown, advisoryis_finished, and default-offtokio-runtime/ eagershutdown_async(c7bb321, testsbe66d8e). Every shutdown request closes count/byte admission before returning. The adapter transfers driver ownership at method call time, not first poll; dropping its returned future detaches waiting during normal runtime operation. Runtime teardown can still synchronously drop rejected work; no hard cleanup deadline or early kernel-buffer release is promised.Two independent implementation/test/document reviews passed after correcting the zero-length admission wording. Local Linux-target default lib-only/all-target checks, all-feature Clippy, both feature-state warning-denying rustdoc, fmt and diff checks pass. The normal/build dependency graph confirms default Tokio sync-only support. Those local checks are compilation evidence. Subsequently, CI #55 at
a942ac1passed all three jobs: 127 native all-feature tests, mandatory O_DIRECT, ordered-prefetch and both benchmark smokes, Docker restricted/unrestricted legs, lint and 20 Python tests. Earlier documentation-only head1dfd81balso passed CI #56. The PR remains draft and unmerged. See shutdown contract.Shared whole-driver read quota follow-up
Head
b09b966addsSharedReadBudgetandprobe_and_start_with_shared_budget(source3931124, public tests7e94f36). Each driver reserves its explicit local byte quota before startup; temporary shortage returnsWouldBlock, not an unsupported-environment classification. A shared receipt follows eager/deferred reads through Msg/Pending and leak retention, so one driver cannot refund capacity still owned by kernel-visible resources. Closing one driver leaves its peers' admission open. Idle drivers also retain their whole quota: this is not dynamic per-read borrowing, retained-result accounting, fallback coverage, or an RSS cap.Two independent non-author reviews passed for code, seven private and nine public tests, and the reservation contract. Local Linux-target default/all-feature checks, all-feature Clippy, both rustdoc feature states with warnings denied, fmt/diff and the default sync-only dependency graph pass. The initial RED was missing-API compilation, not a behavioral test run. CI #57 at
b09b966passed all three jobs: 143 native all-feature tests plus one compiled no-run doc example, mandatory O_DIRECT, ordered-prefetch and both benchmark smokes, lint, and Docker restricted/unrestricted legs. All 16 shared-budget tests executed, including whole-quota leak retention and greater-than-u32 reservations. Follow-upe1992ceonly records this evidence; its workflow remains separate. Later-shard startup rollback remains statically reviewed RAII behavior, not a new targeted fault-injection result. No RustFS dependency source or production application wiring is changed.Safety and review
Published read buffers, FDs and permits remain owned until the terminal read CQE; cancel CQEs never release read resources. Bounded-drain leaks remain charged. Default routing stays round-robin; streams and
without_cancel_on_dropretain their contracts. Byte limits are not a whole-process RSS or retained-result bound.Independent component and integrated reviews covered ownership, cross-shard closure, admission, cancellation, final-owner routing, notification/unwind liveness, faults, test validity and documentation. The count-waiter closure gap and orphan-child cleanup issue found during review were fixed and regression-tested.
Validation
Code revision
7e2f1aa(the following commit only updates documentation/rustdoc comments):Real-kernel overflow/direct tests are distinguished in the docs from simulated syscall-return and metadata-error seams. This is not proof of hung-syscall recovery or advanced taskrun modes.
Performance / remaining gates
No performance improvement is claimed. Three-round same-binary A/A calibration was rejected: round 3 p99 drift was 6.35%, above the unchanged 5% gate. Candidate and diagnostics-overhead comparisons were not started. The control used the foundation baseline, not the hardening candidate. No thresholds were relaxed.
Previous head
bfc2a5dCI #54 passed: lint, 115 native all-feature tests, mandatory O_DIRECT marker, the new ordered-prefetch contract smoke, both benchmark smoke modes, and Docker restricted/unrestricted legs. Dedicated-host follow-up execution was refused by the active-worker/build guard; source artifacts were staged on its data disk without stopping the existing job. No performance rerun occurred. The PR remains draft with performance acceptance outstanding. Application probe offload, driver-thread budgets, logical chunks and exact FD invalidation are implemented in separate RustFS draft PRs #8072/#8074/#8075/#8076, with full checks/merge still outstanding. Application physical-byte/result/io-wq budgets, owned buffers/direct cache enablement, production streaming and advanced ring modes remain unimplemented/evidence-gated. The new ordered-reader example only establishes a tested consumer contract. See optimization status. This PR does not close the roadmap issue.