Skip to content

Heartbeat the worldtree control connection - #52

Open
sonkehahn-shopify wants to merge 1 commit into
mainfrom
sh-worldtreed-connections
Open

Heartbeat the worldtree control connection#52
sonkehahn-shopify wants to merge 1 commit into
mainfrom
sh-worldtreed-connections

Conversation

@sonkehahn-shopify

Copy link
Copy Markdown
Collaborator

Problem

worldtreed's control-plane RPC server reaps any connection that has been idle for
300 s (ServeConfig::idle_timeout, applied to the host socket and every scoped
listener). tecnix opens exactly one WorldtreeConn per EvalState and never
touches it again between requests — no keepalive, no health check, no reconnect.

A long evaluation that goes quiet for more than five minutes therefore loses its
connection and the next request dies with
worldtree: daemon closed the connection / write(): Broken pipe. Because
tectonix-worldtree-socket deliberately has no libgit2 fallback, that is a hard
eval failure.

Evidence: a whole-World tec architect get-all-targets --stream
(trig_be7c0e688f7af4a1807f3c38636229b0, 30m32s, 3895 records, 85 output
bursts) had exactly one inter-burst gap over 300 s — 678.5 s — immediately
followed by the EPIPE. The 181 s / 80 s / 66 s gaps in the same run all survived.

Fix

Heartbeat the control connection from the client. WorldtreeConn spawns a
keepalive thread that issues a cheap request every
tectonix-worldtree-keepalive-interval-ms (default 120 000, 0 disables and
spawns no thread). Any traffic resets the daemon's per-connection idle timer, so
the reaper never fires on an otherwise-idle eval.

The probe is scoped.resolve_ref of HEAD:

  • worldtreed has no ping/no-op verb (wt-proto::method is only host.*,
    scoped.*, tecnix.*).
  • resolve_ref is a pure overlay-then-base RefStore lookup — no working-tree
    walk, no side effects, and it's accounted under its own metric label.

Rejected alternatives:

  • Unknown method — works (the read resets the timer before dispatch, and the
    connection survives Error::Unsupported), but metric_label maps it to
    "unknown", so every beat records a failed op.
  • tecnix.zone_tree_shas with empty zones — triggers a
    ws.porcelain.dirty_zones() sweep.
  • A stray CANCEL frame — free and legal, but gets no reply, so it proves
    nothing about liveness.
  • Raising idle_timeout server-side — owned by //system/scm/worldtree,
    and leaves every other client exposed.

Details:

  • Any well-formed reply proves liveness, so a typed RpcError is swallowed;
    only a transport failure retires the loop, leaving the next real request to
    surface the error (this seam cannot re-dial).
  • The heartbeat takes the same mutex as real requests. A beat delayed behind a
    long request isn't late — that request's own traffic already reset the timer.
  • 120 s leaves room for a missed beat inside the 300 s window.

Testing

src/libexpr-tests/worldtree-keepalive.cc runs an in-process fake worldtreed on
an AF_UNIX socket speaking the real length-delimited protobuf wire and records
the methods seen per connection:

  • heartbeats_the_idle_control_connection — with a 25 ms interval, an idle
    connection accumulates scoped.resolve_ref beats.
  • keepalive_can_be_disabled — with 0, it does not.

Full nix-expr-tests suite green (510 passed).

worldtreed reaps a control-plane connection idle for its `idle_timeout`
(300 s in every listener today), and the client only learns at its next
request — which then dies mid-evaluation with `worldtree: daemon closed
the connection` / EPIPE. Tecnix reads source bytes from the FUSE
projection rather than the socket, so a wide evaluation routinely leaves
the control connection untouched for far longer than that window: one
678 s gap was enough to kill a whole-World eval.

Keep it warm instead. `WorldtreeConn` now runs a heartbeat that, every
`tectonix-worldtree-keepalive-interval-ms` (default 120 s, 0 disables),
sends one cheap request under the same mutex as real traffic. The daemon
has no ping verb, so the probe is `scoped.resolve_ref` of HEAD: a pure
overlay-then-base ref lookup that touches no working tree and is a verb
the daemon accounts for by name. Any well-formed reply proves liveness
and has already reset the daemon's timer, so a typed error is fine; a
transport failure retires the loop and leaves the next real request to
report it (this seam cannot re-dial).
@sonkehahn-shopify
sonkehahn-shopify marked this pull request as ready for review August 26, 2026 17:21
* leaves the next real request to surface the error with its own context.
*/
void keepalive(std::chrono::milliseconds interval)
{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried about this thread handling signals in line with the rest of nix, e.g. should this use ReceiveInterrupts as an additional trigger to stop?

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.

2 participants