Ensure daemon fallback obeys disable overrides#1876
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| pub(super) fn fallback_binary_configured() -> bool { | ||
| if override_disables_fallback(DAEMON_FALLBACK_ENV) | ||
| || override_disables_fallback(CLIENT_FALLBACK_ENV) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| configured_fallback_binary() |
There was a problem hiding this comment.
Respect daemon override precedence when client fallback is disabled
The new early return in fallback_binary_configured now disables delegation whenever CLIENT_FALLBACK_ENV is set to a disabling value, even if DAEMON_FALLBACK_ENV explicitly selects a fallback binary. configured_fallback_binary intentionally gives daemon overrides higher precedence than the client environment, so with both variables set (OC_RSYNC_DAEMON_FALLBACK=default and OC_RSYNC_CLIENT_FALLBACK=off), prior behaviour delegated to the selected binary, while the new logic returns false and skips delegation entirely. This regression prevents operators from disabling client fallback globally while still enabling daemon fallback via the daemon-specific override.
Useful? React with 👍 / 👎.
Specifies graduating the io_uring socket writer from IORING_OP_SEND to IORING_OP_SEND_ZC on kernel >= 6.0: two-CQE completion model, pin-counted buffer pool for the user-page lifetime contract, kernel probe via IORING_REGISTER_PROBE with fallback, daemon TCP integration (#1876), and the throughput vs latency trade-off with the 16 KiB threshold rule.
Rewrite the session-ring-pool design doc around the daemon-session framing: pool keyed by SessionId, lazy init, max-size and idle eviction, hazards around kernel-object lifetime and registered-fd reuse, and a bench plan that isolates session-startup latency. Recommend deferring implementation until #2243 (per-thread rings) lands so the pool layer does not inherit the SharedRing Arc<Mutex<RawIoUring>> bottleneck noted in #1876.
Restructure the SEND_ZC design doc around the explicit 7-section scope: current send path with file:line citations across daemon, transfer, and rsync_io; SEND_ZC semantics with the two-CQE notification model; registered-buffer ownership; kernel-6.0 runtime probe via IORING_REGISTER_PROBE; loopback + small-file bench plan; recommendation to defer until the daemon TCP wiring (#1876) lands; and cross-references to #4217 / #4218 / #4220 / #2243.
… (#4285) Audits whether prerequisites for #1876 have landed and proposes splitting the work into reviewable subtasks (#1876-a through e). - #1874 (shared ring + POLL_ADD) shipped via PR #3553. - #1937 (session ring pool) shipped via PR #4275. - #1935 (hybrid tokio listener) shipped via PR #4278. - No io_uring task carries id #1875; the original dependency note most plausibly meant the session-pool and listener work above. Missing surface enumerated: pooled-ring constructors on the socket adapters, daemon plumbing for SessionRingPool, sync and async worker swaps, IORING_OP_LINK_TIMEOUT integration, and async-cancel discipline. Trigger conditions (kernel >= 5.6, io_uring feature, opt-in flag) and a five-step implementation plan included.
Specifies graduating the io_uring socket writer from IORING_OP_SEND to IORING_OP_SEND_ZC on kernel >= 6.0: two-CQE completion model, pin-counted buffer pool for the user-page lifetime contract, kernel probe via IORING_REGISTER_PROBE with fallback, daemon TCP integration (#1876), and the throughput vs latency trade-off with the 16 KiB threshold rule.
Rewrite the session-ring-pool design doc around the daemon-session framing: pool keyed by SessionId, lazy init, max-size and idle eviction, hazards around kernel-object lifetime and registered-fd reuse, and a bench plan that isolates session-startup latency. Recommend deferring implementation until #2243 (per-thread rings) lands so the pool layer does not inherit the SharedRing Arc<Mutex<RawIoUring>> bottleneck noted in #1876.
Restructure the SEND_ZC design doc around the explicit 7-section scope: current send path with file:line citations across daemon, transfer, and rsync_io; SEND_ZC semantics with the two-CQE notification model; registered-buffer ownership; kernel-6.0 runtime probe via IORING_REGISTER_PROBE; loopback + small-file bench plan; recommendation to defer until the daemon TCP wiring (#1876) lands; and cross-references to #4217 / #4218 / #4220 / #2243.
… (#4285) Audits whether prerequisites for #1876 have landed and proposes splitting the work into reviewable subtasks (#1876-a through e). - #1874 (shared ring + POLL_ADD) shipped via PR #3553. - #1937 (session ring pool) shipped via PR #4275. - #1935 (hybrid tokio listener) shipped via PR #4278. - No io_uring task carries id #1875; the original dependency note most plausibly meant the session-pool and listener work above. Missing surface enumerated: pooled-ring constructors on the socket adapters, daemon plumbing for SessionRingPool, sync and async worker swaps, IORING_OP_LINK_TIMEOUT integration, and async-cancel discipline. Trigger conditions (kernel >= 5.6, io_uring feature, opt-in flag) and a five-step implementation plan included.
Rewrite the session-ring-pool design doc around the daemon-session framing: pool keyed by SessionId, lazy init, max-size and idle eviction, hazards around kernel-object lifetime and registered-fd reuse, and a bench plan that isolates session-startup latency. Recommend deferring implementation until #2243 (per-thread rings) lands so the pool layer does not inherit the SharedRing Arc<Mutex<RawIoUring>> bottleneck noted in #1876.
Restructure the SEND_ZC design doc around the explicit 7-section scope: current send path with file:line citations across daemon, transfer, and rsync_io; SEND_ZC semantics with the two-CQE notification model; registered-buffer ownership; kernel-6.0 runtime probe via IORING_REGISTER_PROBE; loopback + small-file bench plan; recommendation to defer until the daemon TCP wiring (#1876) lands; and cross-references to #4217 / #4218 / #4220 / #2243.
… (#4285) Audits whether prerequisites for #1876 have landed and proposes splitting the work into reviewable subtasks (#1876-a through e). - #1874 (shared ring + POLL_ADD) shipped via PR #3553. - #1937 (session ring pool) shipped via PR #4275. - #1935 (hybrid tokio listener) shipped via PR #4278. - No io_uring task carries id #1875; the original dependency note most plausibly meant the session-pool and listener work above. Missing surface enumerated: pooled-ring constructors on the socket adapters, daemon plumbing for SessionRingPool, sync and async worker swaps, IORING_OP_LINK_TIMEOUT integration, and async-cancel discipline. Trigger conditions (kernel >= 5.6, io_uring feature, opt-in flag) and a five-step implementation plan included.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_69060339d12883239a6de73da42a4b1a