feat: accept until EAGAIN per poll round - #429
Open
BulaBula-zy wants to merge 2 commits into
Open
Conversation
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
The listener is registered with level-triggered epoll (
EPOLLIN, noEPOLLET), andserver.OnReadcurrently accepts exactly one connection perevent. Under a sustained burst of new connections, every pending connection
therefore costs one full poll-loop round trip (epoll wake-up + dispatch), so
accept throughput is bounded by that round trip rather than by the accept
syscall itself.
This change loops
Accept()until the accept queue is empty(EAGAIN | EWOULDBLOCK, reported by
Listener.Acceptas a nil connection withnil error), so one poll round drains the whole backlog.
Prior art: Go's net package, nginx (default) and libuv also accept only once
per event — but they register listeners with edge-triggered epoll, where each
newly queued connection generates a fresh event, so they don't pay a re-fire
round trip per pending connection. netpoll uses LT, so it does. (Switching
the listener to ET is a larger, riskier change; this PR is the smaller step
and works under either trigger mode.)
The existing error handling (out-of-FD delay-and-retry, closed-listener
shutdown) is preserved unchanged; only the accept path is looped.
Functional tests (in-repo, real loopback connections)
TestAcceptUntilEAGAIN_OneRoundAcceptsBurst: 64 fully-establishedconnections parked in the kernel accept queue are all accepted by a single
OnReadround. Fails on v0.7.5 (accepts 1 of 64), passes here — this isthe discriminating regression test.
TestAcceptUntilEAGAIN_NoInfiniteLoopOnEmptyQueue: OnRead returnsimmediately (nil) on an empty queue; no busy spin.
TestAcceptUntilEAGAIN_ConcurrentClientsAllServed: 200 concurrentdial-write-echo clients through the real poll loop, all succeed.
TestAcceptUntilEAGAIN_IdleConnectionNotStarved: a long-lived connectionstays responsive during a 100-connection accept burst.
Validation commands and results:
The same test file against v0.7.5 fails exactly the discriminating test:
Benchmark
BenchmarkAcceptSteadyThroughput(included in this PR): 400 workerscontinuously dial-and-close against a real EventLoop for a 3s window;
throughput is counted server-side in
OnConnect, so the metric is immuneto client-side ephemeral-port exhaustion.
Machine: Linux 6.8.0-136-generic (Ubuntu 22.04), x86_64, Intel Core i7-14700
(28 logical CPUs), Go 1.23.4, fd limit raised to 65535.
Command (per run):
Results, 10 runs each (server accepts/sec):
+48% mean; the ranges do not overlap (v0.7.5 max 20599 < this PR min 27392).
v0.7.5 also shows higher run-to-run variance; this PR is stable within ~7%.
Why count server-side, not client-side
A client-measured dial rate under this load shows ~0.9% "dial failures" on
this PR vs ~0% on v0.7.5. Those failures are
connect: cannot assign requested address— client ephemeral-portexhaustion: the faster-accepting server makes the client establish
connections faster until the client runs out of local ports. It is a client
artefact, not a server regression; server-side accept counting shows this PR
is strictly faster, never slower.
Safety
listener.Acceptreturns(nil, nil)on EAGAIN/EWOULDBLOCK, so the loopalways terminates: it returns on an empty queue, or breaks on a real error
into the pre-existing error path (out-of-FD retry, closed listener), which
is unchanged.
SetNonblockinCreateListener), soAcceptnever blocks inside the loop.Open questions
exceeds the accept rate, the loop never sees EAGAIN and the listener's
poll goroutine starves the established connections that share its poller
(
pollmanager.Pickassigns the listener and connections to the same pollpool). libuv caps its read/write loops at 32 iterations for exactly this
reason ("prevent loop starvation"). No starvation was observed in
TestAcceptUntilEAGAIN_IdleConnectionNotStarvedup to ~28K accepts/sec,but a per-round cap would be a trivial follow-up if reviewers prefer the
safety margin.
Go/nginx/libuv — a larger change with ET-specific concerns on the read
side as well. This PR keeps LT and just amortizes the wake-ups.