Skip to content

fix(pool): back off when accept() fails on descriptor exhaustion - #718

Open
gimballock wants to merge 1 commit into
stratum-mining:mainfrom
marafoundation:fix-accept-emfile-hot-spin
Open

fix(pool): back off when accept() fails on descriptor exhaustion#718
gimballock wants to merge 1 commit into
stratum-mining:mainfrom
marafoundation:fix-accept-emfile-hot-spin

Conversation

@gimballock

Copy link
Copy Markdown
Contributor

When the pool runs out of file descriptors, the accept loop spins at CPU speed and writes a log line per iteration until the shortage clears.

Problem

The accept error arm logs and immediately retries, with no errno discrimination and no delay (channel_manager/mod.rs:412-414):

Err(e) => {
    error!(error = ?e, "Failed to accept new downstream connection");
}

When the process or the system is out of descriptors, accept() fails without consuming the pending connection, so the listener stays readable and the next iteration fails identically. Two costs follow: a core is burned doing nothing, and the log grows without bound.

The second cost is the dangerous one. Our pool log reached 227 MB at 200,000 connections in normal operation, on a volume without rotation, so a sustained spin turns "some connections fail" into "the disk fills and the pool stops."

Fix

  • EMFILE/ENFILE → delay the retry, starting at 5 ms and doubling to a 1 s ceiling
  • any other error → retry immediately as before, because those are per-connection faults and delaying them would slow admission for every miner
  • a successful accept, or any non-exhaustion error, resets the delay
  • the wait sits inside a select! on the cancellation token, so shutdown is never held up by a pending backoff

Discrimination goes through raw_os_error() because both errnos map to ErrorKind::Uncategorized — there is no stable variant to match on, which is precisely why the current arm cannot distinguish them. This needs no new dependency: two #[cfg(unix)] constants, whose values agree across Linux and the BSDs including macOS.

The 5 ms → 1 s schedule follows the precedent in Go's net/http.Server, which applies the same doubling bounds to temporary accept errors.

Tests

Four unit tests, deterministic, 0.00 s, with no descriptor or filesystem manipulation:

Test Asserts
descriptor_exhaustion_is_recognised EMFILE (24) and ENFILE (23) select the backoff path
other_failures_are_not_delayed ConnectionAborted, PermissionDenied, Interrupted do not
backoff_starts_at_the_floor_then_doubles_to_the_ceiling 5 ms floor, doubling, capped across 32 iterations
backoff_is_idempotent_at_the_ceiling a delay already at the ceiling stays there

The classification and the schedule are pure functions, so the tests construct io::Error::from_raw_os_error(24) directly rather than exhausting descriptors. setrlimit would be process-wide and unreliable under parallel test execution, and a listener trait would be invasive relative to what it buys.

What this does not demonstrate

We have not observed the spin. Our runs peaked at 75% of a 400,000 descriptor limit at 1.000 fd/conn, so the exhaustion path was never reached — this is read from the source, not reproduced. The 227 MB figure reflects our deployment and its rotation policy rather than yours.

Validation

Against main on the pinned 1.85.0 toolchain:

  • cargo +nightly fmt --all --manifest-path=pool-apps/Cargo.toml -- --check — pass
  • cargo clippy --manifest-path=pool-apps/Cargo.toml -- -D warnings — pass
  • cargo test --manifest-path=pool-apps/pool/Cargo.toml — 5 passed
  • No version bump: pool_sv2 is 0.7.0 with 0.6.0 published, so it is already bumped since the last release, and this change is API-compatible because both helpers are private

Related but independent: #717 covers the listen backlog on this same accept path.

The accept error arm logged the failure and immediately retried, with no errno
discrimination and no delay. When the process or the system is out of file
descriptors, `accept()` fails without consuming the pending connection, so the
listener stays readable and the next iteration fails identically. The loop then
spins at CPU speed for as long as the shortage lasts, writing one `error!` line
per iteration.

Two costs follow: a core is burned doing nothing, and the log grows without
bound. The second is the dangerous one, because a pool log can already reach
hundreds of megabytes at scale, so a sustained spin can fill the volume and take
the pool down rather than merely failing some connects.

Discriminate descriptor exhaustion and delay the retry: EMFILE and ENFILE back
off starting at 5 ms and doubling to a 1 s ceiling, while every other error
retries immediately as before, because those are per-connection faults and
delaying them would slow admission for every miner. A successful accept, or any
non-exhaustion error, resets the delay. The wait sits inside a `select!` on the
cancellation token so shutdown is never held up by a pending backoff. The
schedule follows the precedent in Go's `net/http.Server`.

Discrimination goes through `raw_os_error()` because both errnos map to
`ErrorKind::Uncategorized`, which is why the existing arm cannot tell them
apart. This adds no dependency: two `#[cfg(unix)]` constants, whose values agree
across Linux and the BSDs including macOS.

Adds four unit tests over the classification and the schedule, which are pure
functions. They construct `io::Error::from_raw_os_error(24)` directly rather
than exhausting descriptors, because `setrlimit` is process-wide and unreliable
under parallel test execution.

The spin has not been observed in practice: our own runs peaked at 75% of a
400,000 descriptor limit, so the exhaustion path was never reached. This is read
from the source rather than reproduced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant