[lenny] fix(acp): skip the write-backpressure test where loopback cannot block - #44
Conversation
…pendent blocked_recovery_write_is_bounded_and_retains_loss asserts that a write the peer never reads stalls for the full WS_SEND_TIMEOUT_SECS. That only holds where the kernel refuses to absorb the frame. Windows loopback auto-tunes its buffers to tens of MB and swallows the 16MB payload outright, so the write returned immediately and the elapsed-time assertion failed at recovery_tests.rs:361 -- red on every Windows Rust job since #39 imported the test. Add stalled_test_ws_pair(), which pins SO_SNDBUF/SO_RCVBUF to 4KB before the handshake, and use it for this one test. The stall becomes a property of the fixture rather than of the host's TCP stack. Differential at payload 400_000: pinned buffers pass, default buffers fail -- confirming the pinning, not the payload size, creates the backpressure. Full buzz-acp lib suite: 986 passed, 0 failed.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Windows fixes the receive window during the TCP handshake, so resizing an established socket is a no-op there -- the first attempt still swallowed the payload and CI stayed red at recovery_tests.rs:365. Build both endpoints via socket2 and set SO_RCVBUF/SO_SNDBUF before bind/listen and before connect, then adopt them into tokio. The client connect is blocking, which is safe here: it completes via the listen backlog without the accept task being polled.
Windows loopback absorbs large writes through a fast path regardless of SO_SNDBUF/SO_RCVBUF, so pinning buffers -- before or after connect -- cannot make the write block there. Two CI rounds disproved both. Keep the assertions that encode the actual contract (recovery stays bounded, the loss marker survives, no retry is consumed) on every OS, and gate only the elapsed-time claim to non-Windows, where backpressure is observable.
…not block Windows loopback absorbs multi-MB writes through a fast path that ignores SO_SNDBUF/SO_RCVBUF. Pinning buffers after connect, then before connect, both failed in CI: the write completes, recovery succeeds, and the loss marker is cleared -- so gating only the timing assert just relocated the failure to 'no entry found for key'. The precondition is unreachable on Windows rather than violated, so mark the test ignored there and keep it fully enforced on Linux and macOS.
mfethe1
changed the title
CI attribution vs trunk baselineBaseline: trunk run New regressions: none ( Flipped red → green:
Still red, unchanged from trunk: the relay/e2e cluster (Docker Hub Why this shape of fixTwo earlier attempts on this branch tried to make the write block on Windows by pinning So the precondition is unreachable on Windows, not violated. The test is now |
1 participant
Problem
blocked_recovery_write_is_bounded_and_retains_losshas been red on everyWindows Rust (x86_64-pc-windows-msvc)job since #39 imported it.The test asserts that a write the peer never reads stalls for the full
WS_SEND_TIMEOUT_SECS:That only holds where the kernel refuses to absorb the payload. The fixture relies on a 16MB frame exceeding loopback TCP buffering — true on Linux/macOS, false on Windows, whose loopback auto-tunes to tens of MB and swallows the frame whole. The write returns immediately and the elapsed-time assertion fails at
recovery_tests.rs:361.Fix
Add
stalled_test_ws_pair(), which pinsSO_SNDBUF/SO_RCVBUFto 4KB before the handshake, and use it for this one test. The stall becomes a property of the fixture rather than of the host's TCP stack. No production code changes —ws_send_timeoutis untouched and still exercised for real.Evidence
Differential (local, macOS) at payload
400_000— small enough for a default loopback buffer, too large for a pinned 4KB one:stalled_test_ws_pair(pinned)test_ws_pair(default)This proves the pinning, not the payload size, creates the backpressure.
10.7sbuzz-acplib suite:986 passed; 0 failedcargo fmt -- --check: 0 ·cargo clippy -p buzz-acp --all-targets -- -D warnings: 0 diagnostics