Skip to content

admin: idle per-connection handlers block graceful drain until ceiling #30

Description

@amarok-60T

Context: follow-up from #25 (PR #29, Phase 8 graceful shutdown).

Problem

Per-connection handlers in RpcServer::serve (src/rpc/server.rs) and admin::serve_with_shutdown (src/admin/server.rs) loop on read_exact / framed.next() without checking the cancellation token between requests. When SIGTERM/SIGINT triggers graceful shutdown, the accept loops stop accepting and the drain awaits all spawned per-connection tasks. An idle client — a TCP keepalive connection on the NFS port, or an arcticwolfctl that connected and then went quiet — keeps its handler task alive indefinitely, blocking the drain until [shutdown] drain_timeout_seconds (default 30) fires.

This is mildly more urgent for the NFS port because it is exposed to potentially untrusted clients. A misbehaving or stale client can prolong the daemon's shutdown.

The current code is correct (daemon exits 0 after the timeout warn) but suboptimal — an operator restarting a healthy daemon should not have to wait for the timeout ceiling.

Suggested approach

Thread the CancellationToken into the per-connection handlers. Two patterns to consider:

  1. Wrap the read in tokio::select!: race read_exact / framed.next() against token.cancelled(). On cancel, close the connection cleanly with a "server closing" frame or just hang up. Requires care to not corrupt the wire mid-read.
  2. Send a connection-close hint when the token fires: spawn a small watcher that, on cancel, shuts down the read half of every active connection. Lets the existing read paths return Err(EOF) naturally without a select! rewrite.

Pattern (2) is cleaner; pattern (1) is simpler. Pick whichever fits the existing handler shape.

Acceptance criteria

  • After cancel, idle per-connection handlers return within a small bound (e.g. 1 second) regardless of the drain ceiling.
  • In-flight requests still complete successfully (no abort).
  • Wire protocol is not corrupted on cancel mid-read.
  • New regression test: open a connection, do not send anything, trigger cancel, assert the drain completes well under the ceiling.

References

  • TODO markers already in place: src/rpc/server.rs and src/admin/server.rs (search for TODO(follow-up to #25)).
  • Doc note in arcticwolf.example.toml [shutdown] section.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions