Skip to content

the 5 ms health accept poll trades 20x idle wakeups for diagnostic latency, undiscussed #274

Description

@StefanSteiner

Summary

0.7.3 dropped the health listener's accept-loop sleep from 100 ms to 5 ms. The justification is sound and the comment states it, but the trade lands on different people than it benefits: a daemon documented to stay resident indefinitely now wakes 200 times a second instead of 10, so that a diagnostic tool can meet a latency bound chosen in the same release.

// hyperdb-mcp/src/daemon/health.rs:154-161
                Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => {
                    // Poll tightly: the doctor network phase budgets only a
                    // few hundred ms for a STATUS round-trip, and on slow CI
                    // runners a 100ms idle sleep between accepts can push the
                    // accept past that window. 5ms keeps the listener
                    // responsive without meaningfully raising idle CPU.
                    std::thread::sleep(Duration::from_millis(5));
                }

The reasoning is correct

The budgets are real:

// hyperdb-mcp/src/diagnostics.rs:28
const DOCTOR_DAEMON_TIMEOUT: Duration = Duration::from_millis(500);
// hyperdb-mcp/src/diagnostics.rs:38
const DOCTOR_NETWORK_PHASE_TIMEOUT: Duration = Duration::from_millis(300);

A 100 ms accept sleep is paid before each connection, and the doctor makes two (PING and STATUS), so up to 200 ms of a 500 ms global budget could disappear into accept latency before either command was read. Lower accept latency also protects the 300 ms is_daemon_alive probe (hyperdb-mcp/src/daemon/discovery.rs:310-312), which is on the discovery fast path rather than a diagnostic one. So the change fixes something.

The cost is borne by everyone else

The daemon is meant to outlive the sessions that use it — that is why it has an idle timeout at all. For the entire time it is resident and idle, that loop now runs a syscall-and-sleep cycle 200 times a second rather than 10. Every user of the MCP server pays that, continuously, so that doctor — run occasionally, deliberately, by one person at a time — stays inside a bound set in the same change that introduced the cost.

To be explicit about the strength of the evidence: the 20× is arithmetic, not a measurement. 100 ms → 5 ms is a twentyfold increase in wakeup frequency by definition. Nobody measured CPU or power before and after, and the comment's "without meaningfully raising idle CPU" is likewise an assertion rather than a result. It may well be right — a sleep plus a failed accept is cheap, and 200/s is not a lot on a laptop. But an unmeasured claim in both directions is worth naming as such, especially on battery-powered machines where wakeup frequency matters more than cycles.

Neither the spec (docs/superpowers/specs/2026-08-13-hyperdb-mcp-agent-ux-design.md) nor its plan mentions the poll interval, so this was not a decision anyone reviewed as a decision — it arrived as a supporting change to a diagnostics feature.

A direction that does not require choosing

This is worth discussing rather than reverting, because reverting reintroduces the doctor problem. But the trade may be avoidable entirely.

The only reason the listener polls at all is so the loop can notice should_shutdown():

// hyperdb-mcp/src/daemon/health.rs:134-137
        loop {
            if state.should_shutdown() {
                break;
            }

If request_shutdown also opened and immediately dropped a throwaway connection to 127.0.0.1:<port>, the accept would wake on its own. The listener could then stay blocking, which gives zero accept latency — better than the 5 ms compromise for the doctor — and zero idle wakeups, better than either value for everyone else. The self-connect costs one TCP handshake on the shutdown path, which is already a slow path.

Points to settle before committing to that shape: the loop would need to tolerate the wakeup connection appearing as an ordinary client that sends nothing (see the Ok("") handling noted in the cleanup issue filed alongside this one), and it needs to behave if the self-connect fails — a short blocking accept timeout as a backstop would keep shutdown bounded without returning to a busy poll.

If measurement shows the 5 ms poll is genuinely free on the platforms that matter, documenting that is also a fine outcome — the objection is to an unmeasured trade made silently, not to the number itself.

Provenance

Retrospective adversarial review of release 0.7.3 (2026-08-28), now part of 1.0.0-rc.2. Verified against current main at 06c5da1. Related: the set_nonblocking(false) fix in the same accept loop, filed separately.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions