Multi-client takeover (last client wins)#82
Merged
Conversation
The server no longer rejects a second connection. It tracks all connected
channels and routes evaluations to the most recently connected one, so a new
client takes over while the others stay connected and their prints still flow.
To make coexistence correct the server routes replies per-channel: a client's
ping is ponged back to that client (not the active one, which would storm the
passive clients' heartbeats), and an evaluation is tied to the channel it was
sent to (one atom holding {:channel :promise}), so only that client's :result
satisfies it and a foreign or stale result can't answer the wrong eval. A client
that drops mid-eval unblocks the REPL with an error instead of hanging it.
The :clients/:ready lifecycle is guarded by a lock and is robust to connects,
disconnects and frames that race start/stop: a connection landing after stop is
ignored, a late on-close after stop can't re-arm readiness, a socket that drops
mid-handshake is not left as a zombie, and a send after stop raises the
documented IOException rather than an NPE.
The Node client can stamp a global id; the harness connects a second client, asserts the eval targets it, and then checks both clients keep their heartbeats answered so neither reconnects.
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
1 participant
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.
Feature 4a of the scoped work. The server stops rejecting a second client: it tracks all connected channels and routes evals to the most recently connected one, so a new client takes over the REPL while the others stay connected and keep streaming their prints. Pairs naturally with auto-reconnect.
Getting this right took some doing - three rounds of adversarial review. The non-obvious part is that coexistence forces per-channel reply routing: a client's heartbeat ping has to be ponged back to that client (routing it to the active one storms the passive clients), and an eval is tied to the channel it was sent to so a stale or foreign result can't answer the wrong expression. A client that drops mid-eval now unblocks the REPL with an error instead of hanging it. The
:clients/:readystate machine is lock-guarded and hardened against frames that race start/stop.Tested end to end: the Node integration harness connects a second client and asserts takeover plus heartbeat coexistence (no reconnect storm), and there are unit tests for result correlation, disconnect-unblock, pong routing, and the stop/restart lifecycle.
Known minor limitations (documented, out of scope): no eval-id correlation so a misbehaving client sending duplicate results could confuse a result; an out-of-band
stopconcurrent with an in-flight eval can strand that eval (normal REPL teardown is sequential and unaffected).