Bound the per-poll drain, record the recvmmsg result, and prove the installed package works - #19
Merged
Conversation
Session::poll(), Swim::poll() and EventLoop::run_once() all drained the socket until EAGAIN. The loop is single-threaded and tick() runs only after poll() returns, so a peer that keeps datagrams arriving could hold the loop inside poll() indefinitely. What starves is the RTO retransmit timer and the SWIM failure detector: on a closed mesh that is a liveness bug against the 25 ms retransmit floor that is the whole point of this library, not a theoretical one. poll() now stops after max_recv_per_poll datagrams (default 64) and returns true when it did, so a caller that wants to keep draining can loop and gets to run its timers between passes. The bound is checked before the read rather than after: consuming a datagram and then breaking would drop it on the floor. The event loop uses the same ceiling as a file-scope constant since it has no Config, and level-triggered epoll re-reports the socket, so an early return loses nothing. The return type of poll() changes from void to bool on both Session and Swim. Existing callers that ignore it keep compiling and keep working; they simply drain 64 at a time instead of everything. tests/unit/drain_bound_test.cc pins three properties: one poll stops at the bound and says so, repeated polls still deliver the whole flood so the bound defers rather than discards, and tick() is reachable mid-flood. Verified, including the negative control rather than assuming it: raising the bound out of reach while leaving the code compiling fails all three DrainBound tests; with the bound in place the suite is 61/61 with no new warnings. src/loop.cc is Linux-only and is not compiled on macOS, so its arm of this change is covered by CI rather than by that local run.
…erted The hypothesis was reasonable and the premise checked out: strace -c under sustained load put sendto plus recvfrom at 99.9% of syscall time, and the drain averaged about 9 datagrams per poll. Collapsing a burst into one recvmmsg should have paid. It was implemented and it worked as a mechanism: 2.107 to 1.136 syscalls per message, a 46% cut, with receive syscalls down 88%. It bought nothing a user can see. Saturating throughput was 261.96 against 260.56 Mbit/s, inside noise with the batched arm marginally slower, and on the request-reply workload that is the actual tail-latency thesis, p999 got consistently worse, 1.19 ms to 1.31 ms, with p50 identical. The reason is in the numbers: that workload is send-bound, and after batching sendto was 88% of syscall time and is 1:1 with messages. sendmmsg could only help by waiting to coalesce, which is exactly the delay taut exists to avoid, so cutting the remaining 11% harder cannot show up. Reverted rather than kept. A 46% syscall reduction that moves no user-visible number, costs an extra virtual on the transport interface, and nudges the tail the wrong way is a worse design than not having it. The record also keeps the sharp edge found on the way, because it will bite the next person who tries: a UdpTransport decorator that does not forward a batched read silently inherits the base-class loop and collapses batching back to one syscall per datagram, with no error anywhere. The bench's CountingTransport did exactly that, which is why the first "after" measurement showed zero recvmmsg calls. Documentation only. 61/61 ctest unchanged.
Two things a first-time reader hits immediately. The quickstart snippet did not compile. It used the 2ms literal without pulling in std::chrono_literals, and it passed a `payload` that the snippet never defined. ByteSpan is std::span<const std::byte>, a borrowed view, so the reader also needs to know the bytes must outlive the send; the snippet now constructs one from a std::string and says so. The build section told macOS readers to go and set up a Linux VM before doing anything, which is wrong and expensive. Only the real UDP socket, the epoll loop and the netem bench are Linux-only. The codec, RTO estimator, window, timers, SWIM and SimNet are portable and every protocol test runs over SimNet, so the whole 61-test suite runs natively on Apple silicon. The Linux container is now presented as what it actually is: the way to run the netem soak and the file-transfer demos, not the price of entry. The container recipe carries the one mistake that wastes an afternoon. A CMake cache records the absolute paths it was configured with, so a build/dev written on the host is a build tree belonging to /Users/you/taut and the container mounting the same directory at /src refuses it with a complaint about a path the reader never chose. Mounting a volume over /src/build gives the container its own tree and lets the two alternate. The generator note is the same class of problem: the presets name Ninja and clang++ explicitly, so a missing Ninja stops the configure step outright rather than falling back to make and failing later somewhere less obvious. Verified rather than asserted: a Debug + ASan/UBSan build matching the dev preset passes 61/61 on this Apple silicon machine, with no warnings outside vendored googletest.
The README documented find_package and FetchContent and nothing checked that either works. An in-tree add_subdirectory build would not check it: it links against targets that exist only because taut's own build tree is present, so it can pass while the installed package is broken. examples/telemetry is a whole program, not a fragment: 500 readings pushed on class 2 across a 20% loss link, arriving exactly once and in order, with the retransmits and the backpressure stalls counted so a reader can see what the library absorbed for them, plus class 0 heartbeats where loss is allowed. It asserts its own invariants and exits nonzero when they disagree, so it is a test that also reads as documentation. It carries its own CMakeLists.txt with find_package(taut REQUIRED) and is configured against an install prefix and nothing else. The new consumer CI job runs exactly that sequence on every push: build, install to a prefix, configure the example against only that prefix, build it, run it. The format job's find now covers examples/ so it cannot drift out of style. One README correction while writing this up. The determinism note said `telemetry 42` reproduces the transcript. It does not: the default seed is 7, seed 42 gives 339 retransmits and 3,067 stalls against the transcript's 338 and 245, and CI runs the example with no argument. The claim now names the seed that actually produced the numbers printed above it. Verified end to end locally rather than only added to CI: install to a prefix, configure and build the example against only that prefix with no warnings, run it, output matches the README transcript line for line, exit 0, and two consecutive runs are byte-identical. clang-format --dry-run --Werror over include src tests examples is clean.
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.
Four commits, each buildable on its own.
What is here
Per-poll drain bound.
Session::poll(),Swim::poll()andEventLoop::run_once()drained the socket until EAGAIN. The loop is single-threaded andtick()runs only afterpoll()returns, so a peer that keeps datagrams arriving could hold the loop insidepoll()indefinitely and starve the RTO retransmit timer and the SWIM failure detector. On a closed mesh that is a liveness bug against the 25 ms retransmit floor that is the whole thesis, not a theoretical one.poll()now stops aftermax_recv_per_poll(default 64) and returnstruewhen it did, so a caller that wants to keep draining loops and gets to run its timers between passes. The bound is checked before the read: consuming a datagram and then breaking would drop it. Level-triggered epoll re-reports the socket, so an early return loses nothing.poll()changes fromvoidtoboolonSessionandSwim; callers that ignore the result keep compiling and keep working, they just drain 64 at a time.D29:
recvmmsg, measured, rejected, reverted. The premise held (strace -cputsendto+recvfromat 99.9% of syscall time) and the mechanism worked (2.107 to 1.136 syscalls per message, receive syscalls down 88%), and it bought nothing: throughput 261.96 vs 260.56 Mbit/s, and request-reply p999 got consistently worse, 1.19 ms to 1.31 ms. That workload is send-bound, andsendmmsgcould only help by waiting to coalesce, which is the delay taut exists to avoid. Reverted rather than kept. The record also keeps the trap found on the way: aUdpTransportdecorator that does not forward a batched read silently collapses batching back to one syscall per datagram, with no error.README quickstart and platform docs. The quickstart snippet did not compile:
2mswithoutstd::chrono_literals, and apayloadthe snippet never defined. The build section also told macOS readers to stand up a Linux VM before doing anything, which is wrong; the whole 61-test suite runs natively, and the container is only needed for thenetemsoak and the socket demos.examples/telemetryplus aconsumerCI job. A whole program, configured against an install prefix and nothing else, so CI checks the package that actually ships.One documentation bug found by running the thing
The determinism note claimed
telemetry 42reproduces the transcript printed above it. It does not. The default seed is 7, which is what CI runs; seed 42 gives 339 retransmits and 3,067 backpressure stalls against the transcript's 338 and 245. The claim now names the seed that produced the numbers.Verification
ctestdevpreset config),ctestDrainBoundtests fail, as the decision record claimsclang-format --dry-run --Werroroverinclude src tests examplesNo new compiler warnings in either build; the only one in the tree is in vendored googletest.
One honest gap in the local run:
src/loop.ccis Linux-only and is not compiled on macOS, so the event-loop arm of the drain bound is covered by CI onubuntu-latestrather than by anything I ran on this machine. That is also where the 62nd test (EventLoop.EchoesDatagramThroughEpoll) exists.Every action in
ci.ymlremains SHA-pinned, including thecheckoutadded by the new job.🤖 Generated with Claude Code