From 7bd84af48d5d8d70978dc00804a38b4283106a97 Mon Sep 17 00:00:00 2001 From: iceBear67 Date: Sat, 15 Aug 2026 17:31:35 +0800 Subject: [PATCH] impl connection recovery --- CLAUDE.md | 163 ++++++++ PROTOCOL.md | 191 +++++++++- README.md | 31 +- client/client.go | 125 +++++- client/config.example.json | 4 + client/config.go | 169 ++++++++- client/pool_test.go | 4 +- client/resume.go | 332 ++++++++++++++++ client/resume_bench_test.go | 104 +++++ client/shaper.go | 268 +++++++++++++ client/shaper_test.go | 231 ++++++++++++ client/stats.go | 223 +++++++++++ client/unacked.go | 64 ++++ client/unacked_test.go | 112 ++++++ client/worker.go | 344 +++++++++++++++-- docs/architecture.md | 107 +++++- e2e/bandwidth_test.go | 147 ++++++++ e2e/control_outage_test.go | 144 +++++++ e2e/e2e_test.go | 13 +- e2e/harness_test.go | 24 +- e2e/resilience_test.go | 33 ++ e2e/resume_test.go | 270 +++++++++++++ server/config.example.json | 7 +- .../java/io/icybear/redapricot/Config.java | 33 +- .../main/java/io/icybear/redapricot/Hub.java | 355 ++++++++++++++++-- .../io/icybear/redapricot/HubConnection.java | 45 ++- .../java/io/icybear/redapricot/HubServer.java | 17 + .../io/icybear/redapricot/PendingPlayer.java | 17 +- .../io/icybear/redapricot/PlayerStream.java | 88 +++++ .../java/io/icybear/redapricot/Protocol.java | 22 ++ .../io/icybear/redapricot/UnackedBytes.java | 95 +++++ .../io/icybear/redapricot/WorkerConn.java | 273 ++++++++++---- .../icybear/redapricot/CryptoCodecTest.java | 3 +- 33 files changed, 3858 insertions(+), 200 deletions(-) create mode 100644 CLAUDE.md create mode 100644 client/resume.go create mode 100644 client/resume_bench_test.go create mode 100644 client/shaper.go create mode 100644 client/shaper_test.go create mode 100644 client/stats.go create mode 100644 client/unacked.go create mode 100644 client/unacked_test.go create mode 100644 e2e/bandwidth_test.go create mode 100644 e2e/control_outage_test.go create mode 100644 e2e/resume_test.go create mode 100644 server/src/main/java/io/icybear/redapricot/PlayerStream.java create mode 100644 server/src/main/java/io/icybear/redapricot/UnackedBytes.java diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..1c1ea92 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,163 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +redapricot is a central-hub P2P tunnel that speaks an *extension* of the Minecraft Java +Edition protocol. A **hub** (Java/Vert.x, one public TCP port) forwards players to a +**client** (Go) sitting next to a real Minecraft server behind NAT. Everything — players +and clients alike — arrives on the same port; the hub tells them apart by the handshake +`Intent` field (`17` = redapricot, `18` reserved, anything else = player). + +Two implementations of one wire protocol live in this repo, so **most changes are +cross-language**. `PROTOCOL.md` is the normative spec; `docs/architecture.md` explains the +design rationale. + +## Build + +`scripts/build.sh` builds both sides (hub via `installDist`, client into `bin/`). It +auto-discovers a JDK/Gradle under `~/.sdkman/candidates/`. + +**Gradle needs an explicit `JAVA_HOME` in this environment** — it is not inherited: + +```bash +JAVA_HOME=$HOME/.sdkman/candidates/java/current gradle -p server installDist +JAVA_HOME=$HOME/.sdkman/candidates/java/current gradle -p server test +JAVA_HOME=$HOME/.sdkman/candidates/java/current gradle -p server shadowJar # fat jar +go build ./... # Go side +``` + +## Test + +```bash +./scripts/e2e.sh # build both, Go unit tests, then e2e +go test ./client/... -count=1 # Go unit tests (codec, crypto, pool, shaper, velocity) +go test ./e2e/... -count=1 -v -timeout 300s +go test ./e2e/ -run TestRoundTrip -v # one e2e test +go test ./client/ -run TestShaperEnforcesRate -v +``` + +The e2e suite spawns the **real Java hub as a subprocess** from +`server/build/install/redapricot-server` (via `-cp lib/* io.icybear.redapricot.Main`) and +runs the Go client in-process against a mock destination. It fails fast if the hub has not +been installed, so **rebuild the hub after touching Java** or e2e silently tests stale +bytecode. `resolveJava` skips the suite when no JDK is found (`JAVA_HOME`, SDKMAN, `PATH`). + +Timing-sensitive suites (`e2e/bandwidth_test.go`, `client/shaper_test.go`, +`e2e/slowstream_test.go`, `e2e/resilience_test.go`, `e2e/resume_test.go`) assert on rates +and recovery deadlines; give them slack rather than tightening thresholds. The resume tests +are load-sensitive — a busy machine (a gradle daemon in the background) shifts the races +they exercise, so run them a few times rather than trusting a single pass. + +## Architecture + +### Connection kinds (all on one port) + +| First handshake | Becomes | +|---|---| +| `Intent 17`, rekey magic `0x01` | **control session** — pattern registration, `ControlRequest`, Ping/Pong | +| `Intent 17`, rekey magic `0x02` | **worker conn** — multiplexed player streams | +| any other intent | **player** — hostname regex-matched, then tunneled | + +Session establishment (both kinds): plaintext handshake whose `Server Address` is +`hex(SHA3-224(PSK))` → one frame encrypted with PSK-derived keys carrying +`magic ‖ rand ‖ timestamp ‖ flags ‖ window` → **both sides switch ciphers** to keys derived +from `rand‖ts` → hub replies `SessionReady` echoing accepted flags and its window. + +### Player handoff + +The hub pauses and buffers a matched player socket, mints a random 16-byte **CID**, and +sends `ControlRequest(CID, matchedPattern, ip, port)` down the control session. The client +allocates a worker stream, `SYN`s it with that CID, dials the destination, and the hub +binds the pending player to that stream. The CID's secrecy (it only ever travels encrypted +over the control session) is what authorizes the takeover — there is no other client +identity check. The hub echoes **the matched pattern string, not the player's hostname**, +so the client can look it up directly in its route table; the buffered handshake is +forwarded verbatim so the backend sees the original hostname. + +### Code map + +- `server/src/main/java/io/icybear/redapricot/` — `HubConnection` (per-socket state machine: + handshake parse → dispatch → rekey → control/worker/player), `Hub` (pattern registry, CID + table, pending players), `ControlSession`, `WorkerConn` (mux demux + per-stream state), + `net/EncryptedFrames`, `crypto/Crypto`, `util/` (VarInt, ProtoReader/Writer, Hex). +- `client/` — `client.go` (control session, reconnect, dispatch), `worker.go` (pool, + allocation, `WorkerConn`, `Stream` and its two goroutines), `shaper.go` (egress rate cap), + `velocity.go` (Velocity modern-forwarding interception), `proxyproto.go` (HAProxy v2), + `config.go` (config + all protocol constants), `wire/` (VarInt/MC codec, SHA3+ChaCha20, + `FramedConn`). +- `cmd/redapricot-client/` — binary entrypoint; `e2e/` — integration harness. + +### Threading + +- **Hub:** a single Vert.x verticle instance, so the pattern registry, CID table, and all + per-connection state are touched by one event loop and need no locking. Never introduce a + blocking call there. +- **Client:** one goroutine reads each connection; `WriteFrame` is mutex-serialized. Each + stream has exactly two goroutines — `run` (destination → hub) and `writeLoop` (the *only* + writer to the destination, draining a queue fed by the worker readLoop). The readLoop must + never write to a destination, or a stalled backend blocks frame dispatch for every other + stream on that conn. + +## Invariants to preserve when editing + +- **Constants are mirrored** in `client/config.go` and `server/.../Protocol.java`. Changing + one without the other is a silent protocol break; update `PROTOCOL.md` too. +- **The frame length prefix is plaintext, the payload is encrypted.** This is deliberate: a + reader always knows how many ciphertext bytes belong to the current frame, which makes the + rekey cipher switch unambiguous. Do not encrypt the length. +- **Per-direction keys, fixed zero nonce** (`SHA3-256(phaseKey ‖ 0x01)` c2s, + `‖ 0x02` s2c). Both directions must never share a keystream. Go `x/crypto/chacha20` and + Java JCE `ChaCha20` are byte-identical here, and unit tests on both sides pin the same + SHA3-224 vector — keep that pinning if you touch crypto. +- **Per-stream flow control is mandatory.** The hub rejects a session whose rekey lacks + `FLAG_STREAM_FC`; the client rejects a hub that does not echo it. Credit is granted back + (`WND`) only as bytes are actually written to the terminal socket, batched at half-window. +- **Only stream ids allocated by the client exist** (a per-conn counter starting at 1); + sid `0` is reserved for connection-scoped `PING`/`PONG`. +- **Only DATA is shaped** by `client/shaper.go`. Delaying `FIN`, `WND`, or `PONG` would trip + the very liveness detection the heartbeat exists for. The shaper is client-local and + invisible on the wire. +- **The pool grows breadth-first** (`StreamsBeforeGrowing = 1`): dial up to `maxConn` before + stacking streams, so one TCP connection is never the shared point of failure for every + player. `SaturationThreshold = 8` only logs, once the pool is already at `maxConn`. + A dial is never performed while holding the pool lock. +- **Liveness is explicit everywhere:** every session heartbeats (drop after + `3 × pingIntervalMs`), every socket write is bounded, establishment has a deadline. A + silently blackholed path (NAT forgetting a flow, no FIN/RST) must recover without operator + action — `TestBlackholedPathRecovers` guards this. +- **Stream resumption is byte-exact or it is nothing** (`PROTOCOL.md §7.5`, + `client/resume.go`, `WorkerConn.handleResume`). A worker-conn drop hangs the player and + reattaches the stream over a fresh conn. Three offsets are tracked per direction and are + *not* interchangeable: replay from the peer's **accepted** offset, restate the window from + its **delivered** offset, and never size the window from **credited** — the grants in + flight when the conn died are gone for good, and a window derived from them can be + permanently zero, which deadlocks. `TestResumePreservesByteStream` guards this. +- **The retained region needs no cap of its own.** Flow control already bounds outstanding + bytes to one window, so the retransmit buffer *is* the outstanding region. Anything that + lets a sender exceed its window silently makes it unbounded. +- **Hub-side socket handlers must route through `st.worker`, never through a captured + conn.** A lambda installed in `handleSyn` closes over `this`; after a reattach it would + write into the dead conn's transport, where sends are dropped silently and the player goes + mute with nothing logged. +- **A closed control session orphans its routes rather than deleting them** + (`PROTOCOL.md §5.2`, `Hub.removeSession`). Players arriving during the client's + reconnect are held and replayed once it re-registers, instead of being told there + is no such server. Two traps: `Hub.match` must surface the orphaned state rather + than hand back a dead `ControlSession` (`EncryptedFrames.send` drops silently on a + closed transport, so the player would hang with no request ever sent), and a held + player's deadline is the registration grace, not `pendingTimeoutMs` — whichever is + shorter fires first and closes the socket. +- **The off switches must reach the hot path, not just the wire.** `streamResume: false` + allocates no retained region and `statsIntervalMs: 0` allocates no counters, so both cost + one predictable branch. `TestResumeDisabledAllocatesNothing` guards the first. + +## Conventions + +- Comments here explain *why*, often citing the failure they prevent, and reference + `PROTOCOL.md §N`. Match that when adding code on either side. +- Wire-visible behaviour changes need: both implementations, `PROTOCOL.md`, an e2e test, and + usually a note in `docs/architecture.md` and the README config tables. +- Java uses Lombok (freefair plugin) and Log4j2; JUL is routed through Log4j2 both via + `applicationDefaultJvmArgs` and programmatically in `Main` for the `java -jar` path. diff --git a/PROTOCOL.md b/PROTOCOL.md index 8950da3..845895f 100644 --- a/PROTOCOL.md +++ b/PROTOCOL.md @@ -150,6 +150,7 @@ RecvWindow: VarInt # client's per-stream receive window, bytes (§7.3) |-----|------|---------| | `0x01` | STREAM_FC | **Per-stream flow control** (§7.3). Mandatory. | | `0x02` | WORKER_HEARTBEAT | Mux-level `PING`/`PONG` on worker conns (§7.4). Optional. | +| `0x04` | STREAM_RESUME | **Stream resumption** (§7.5): a worker-conn drop hangs the player rather than closing it. Optional. | STREAM_FC is mandatory: `RecvWindow` advertises the client's per-stream receive window in bytes and must be positive. The hub closes the connection if the flag @@ -176,13 +177,22 @@ frame (both directions) is Phase B, counters reset to 0. The hub then sends one Phase-B frame to confirm success: ``` -SessionReady : payload = [ 0x00, Flags: VarInt, RecvWindow: VarInt ] +SessionReady : payload = [ 0x00, Flags: VarInt, RecvWindow: VarInt, + ResumeGraceMs: VarInt ] # only when STREAM_RESUME is set ``` The hub echoes the accepted flags (STREAM_FC set) followed by its own per-stream receive window. A client must reject a SessionReady without the STREAM_FC flag or without a positive window (an unsupported hub). +`ResumeGraceMs` is present only when the hub accepts STREAM_RESUME, and states +how long it will hang a player waiting for that player's stream to be reattached +(§7.5). The client clamps its own retry budget below this value. Advertising it +rather than assuming matching configuration is deliberate: the client must always +give up first, and if the hub instead dropped a hung player while the client was +still reattaching, the failure would be a silent hang rather than an error. A hub +that sets the flag but omits the field is treated as not supporting resumption. + A hub that rejects the session simply closes the TCP connection (optionally after a Phase-B `Error` frame, §6). After `SessionReady`: @@ -246,6 +256,40 @@ Because the pattern is a regex, a literal dot must be escaped (`mc\.example\.com an unescaped `.` is the regex "any character" wildcard. A pattern that fails to compile is rejected at `Register` time with `RegisterAck` status `1`. +### 5.2 Orphaned routes (control-session outage) + +When a control session closes, its registrations are **not** deleted straight +away. They are marked *orphaned* and kept for `registrationGraceMs`. + +This costs nothing on the wire — it is entirely hub-side behaviour — but it +closes a gap that is otherwise very visible. A client whose control session dies +reconnects with backoff, and until it re-registers the hub has no route for it, +so every player arriving in that window is told there is no such server. The +players already tunneled are unaffected, since they ride worker conns, which a +control-session close never touches. + +While a route is orphaned: + +* a player matching it is **held** — paused, with its handshake buffered exactly + as for a normal pending player — and no `ControlRequest` is sent, because there + is no session to send it to; +* a player that was already pending when the session closed is moved into the + same held state rather than being dropped; +* when any client registers that pattern again, the hub delivers the + `ControlRequest` it never sent and the player proceeds normally. The held + player's deadline switches from the registration grace to `pendingTimeoutMs` + at that point, since it is now waiting for a worker rather than for a route. + +If the grace expires with no client having re-registered, the route and every +player held on it are dropped. `registrationGraceMs: 0` disables the mechanism +and restores the immediate-drop behaviour. + +Note the hub cannot distinguish "this client is reconnecting" from "this client +is gone for good" — that is what the grace period is a bet on. It is bounded on +the client side too: the reference client retries immediately on a control-session +drop and caps its backoff at 10s, so the bet is usually settled in well under a +second. + ## 6. Error frame (any redapricot connection) At any time either side may send, then close: @@ -281,10 +325,26 @@ Data : Bytes[...] # remainder of the frame payload | `0x04` | WND | both | `Delta: VarInt` — flow-control credit grant (§7.3). | | `0x05` | PING | both | `Nonce: I64` — liveness probe on reserved StreamID `0` (§7.4). | | `0x06` | PONG | both | `Nonce: I64` — echoes the probe's nonce (§7.4). | +| `0x07` | RESUME | C → S | `CID: Bytes[16]`, `Accepted: I64`, `Delivered: I64` — reattach a hung stream to this conn (§7.5). | +| `0x08` | RESUME_ACK | S → C | `Accepted: I64`, `Delivered: I64`, `NewCID: Bytes[16]` — the reattach succeeded (§7.5). | There is no explicit SYN-ACK: success is implied by the hub forwarding the buffered Handshake as the stream's first `DATA`; failure is an `RST`. +`RST` reason codes. The byte remains optional — a peer that predates it sends +none, and a receiver must tolerate its absence — but distinguishing the reasons +matters for resumption, where "this stream is gone" and "someone else already +took it" call for opposite responses. + +| Code | Name | Meaning | +|------|------|---------| +| `0x00` | UNSPECIFIED | No reason given (also the meaning of an absent byte). | +| `0x01` | UNKNOWN_STREAM | CID unknown or expired, or the hub restarted. Terminal: stop retrying. | +| `0x02` | ALREADY_BOUND | Another reattach already claimed this stream. Do **not** tear down. | +| `0x03` | RESUME_ABANDONED | The peer gave up reattaching. | +| `0x04` | FLOW_CONTROL | The peer exceeded its advertised window. | +| `0x05` | DIAL_FAILED | The client could not reach the destination. | + ### 7.1 Stream allocation (client side) The client keeps a pool of `1 ≤ N ≤ max_conn` worker conns (`max_conn` @@ -372,8 +432,9 @@ Every established session is therefore covered by a heartbeat: closes the session, which triggers its normal reconnect with backoff. * **Worker conns** — when WORKER_HEARTBEAT was negotiated, the same exchange runs as mux `PING`/`PONG` frames on the reserved StreamID `0`. On timeout the - client closes the conn; its streams are reset and it is dropped from the pool, - so the next player gets a freshly dialed connection. + client closes the conn and drops it from the pool. Its streams are reset, + unless STREAM_RESUME was negotiated, in which case they are hung and reattached + over a fresh conn instead (§7.5). * **Hub side** — an established redapricot session that receives no frame for `sessionIdleTimeoutMs` (default 90000, `0` disables) is closed. Player connections are never subject to this. @@ -381,11 +442,72 @@ Every established session is therefore covered by a heartbeat: Both ends also enable TCP keepalive, which catches the narrower case of a peer that has become unreachable at the IP layer. -Session establishment (§3.2) is bounded by a single deadline covering the dial, +Session establishment (§4) is bounded by a single deadline covering the dial, the `Rekey` write and the `SessionReady` read, and every frame write is bounded too — a peer that stops reading must not be able to park a whole multiplexed connection inside one write. +### 7.5 Stream resumption (STREAM_RESUME) + +A worker conn is only the middle leg of every stream it carries. When it dies, +both terminal sockets — the player's and the destination's — are usually still +healthy, so resetting the streams discards working connections because a +replaceable transport failed. One conntrack expiry disconnects every player on +that conn. + +With STREAM_RESUME negotiated, a worker-conn drop instead **hangs** each stream: + +* the hub pauses the player socket, keeps its state, and holds it for + `ResumeGraceMs` from the moment of the *first* hang (an absolute deadline, so a + flapping client cannot extend it indefinitely); +* the client keeps the destination socket open and reattaches the stream over a + fresh worker conn by sending `RESUME` with the stream's CID; +* the hub answers `RESUME_ACK`, or `RST(UNKNOWN_STREAM)` if it holds no such + stream — which is also what a client gets from a hub that has restarted. + +**Resumption is byte-exact, and must be.** Frames handed to a dying socket are +lost with no notification, and the frame cipher cannot be resynchronized, so each +side replays whatever the other did not receive. Splicing the stream even one +byte off corrupts the tunneled protocol. + +Three offsets are tracked per direction, and they are not interchangeable: + +| Offset | Meaning | Used for | +|--------|---------|----------| +| `Sent` | bytes handed to the wire | the end of the retained region | +| `Accepted` | bytes taken off the wire toward the terminal socket | **where to replay from** | +| `Delivered` | bytes actually written to the terminal socket | **how to restate the window** | + +Each side retains the bytes between what the peer has credited and what it has +sent. This costs no new bound: flow control (§7.3) already caps outstanding bytes +at one window, so the retained region *is* the outstanding region. + +On reattach both sides replay `[peer's Accepted, Sent)` and set +`SendWindow = W − (Sent − peer's Delivered)`, then discard their own pending +credit — the exchanged `Delivered` already carries everything those deltas would +have, so emitting both would grant the same bytes twice. + +Two rules deserve emphasis, because the obvious simplifications are wrong: + +* *Accepted*, not *Delivered*, is the replay point. Delivery is signalled + asynchronously on both sides and stops being reported exactly when a connection + dies; replaying from it would re-send bytes the peer already has. +* *Delivered*, not *credited*, sizes the window. Credit travels as deltas, and + the grants in flight when the connection died are gone for good. A window + derived from them is permanently short — and if a full window was outstanding + at the drop, permanently zero, which deadlocks: nothing can be sent, so no + credit can ever come back. + +`RESUME_ACK` carries a freshly minted `NewCID`, which replaces the old one. A CID +therefore stays single-use even though a stream may be reattached many times, so +a leaked CID grants no more than the outage in which it was observed. + +Resumption is **hub-instance-affine**: a CID means nothing to a second hub behind +an L4 load balancer, which answers `RST(UNKNOWN_STREAM)` and lets the client tear +down at once. Because the grace period holds player sockets and their buffers, a +hub bounds the number of hung streams and the bytes they retain, dropping the +oldest first when either cap is reached. + ## 8. HAProxy protocol v2 (optional) When a mapping has `proxyProtocol: true`, the client prepends a PROXY v2 header @@ -415,7 +537,12 @@ big-endian. "timestampWindowMs": 30000, "pendingTimeoutMs": 10000, "streamWindowBytes": 262144, - "sessionIdleTimeoutMs": 90000 + "sessionIdleTimeoutMs": 90000, + "streamResume": true, + "resumeGraceMs": 20000, + "maxParkedStreams": 256, + "statsIntervalMs": 0, + "registrationGraceMs": 15000 } ``` @@ -426,6 +553,27 @@ the hub's advertised per-stream receive window (§7.3). session or worker conn that has gone silent for that long (§7.4). It must stay comfortably above the client's `pingIntervalMs`; `0` disables the watchdog. +`streamResume` (optional, default true) offers STREAM_RESUME (§7.5). With it +false the hub never echoes the flag and behaves exactly as a hub that predates +the feature, allocating no retained regions. + +`resumeGraceMs` (optional, default 20000) is how long a hung player is held, and +is advertised in `SessionReady`. It must exceed the client's own grace by at +least one dial, which is why the client clamps itself against the advertised +value rather than its own configuration. + +`maxParkedStreams` (optional, default 256) and `maxParkedBytes` (default +`maxParkedStreams × 2 × streamWindowBytes`) bound what hung players may cost; +past either the oldest are dropped. + +`statsIntervalMs` (optional, default 0 = off) logs a periodic line with live and +parked stream counts, retained bytes, and the pattern count. + +`registrationGraceMs` (optional, default 15000) is how long a closed control +session's routes are kept as **orphaned** rather than deleted (§5.2). `0` +disables it, restoring the behaviour of dropping routes the instant a session +closes. + ### 9.2 Client — JSON ```json @@ -435,6 +583,10 @@ comfortably above the client's `pingIntervalMs`; `0` disables the watchdog. "maxConn": 4, "pingIntervalMs": 20000, "streamWindowBytes": 262144, + "maxBandwidth": "20mbps", + "streamResume": true, + "resumeGraceMs": 15000, + "statsIntervalMs": 0, "mappings": [ { "pattern": "mc\\.example\\.com", "destination": "127.0.0.1:25566", "proxyProtocol": true } ] @@ -444,6 +596,30 @@ comfortably above the client's `pingIntervalMs`; `0` disables the watchdog. `streamWindowBytes` (optional, default 262144, clamped to [32768, 8388608]) is the client's advertised per-stream receive window (§7.3). +`streamResume` (optional, default true) offers STREAM_RESUME (§7.5). With it +false the client never offers the flag, never retains a byte for retransmission, +and behaves exactly as a client that predates the feature. + +`resumeGraceMs` (optional, default 15000, minimum 2000) is how long a hung stream +keeps trying to reattach, clamped below the hub's advertised grace. The default +is chosen against the *backend*, not the tunnel: a hung player stops answering +the game server's KeepAlive, and vanilla disconnects a silent client at 30s, so a +longer grace would only resume sessions the backend then kicks. + +`statsIntervalMs` (optional, default 0 = off) logs a periodic diagnostics line +and a per-stream summary at close, reporting how long each stream spent blocked +on the flow-control window versus the bandwidth cap, and the heartbeat round-trip +time per conn. These distinguish a slow backend from a saturated uplink from a +bad path, which throughput alone cannot. + +`maxBandwidth` (optional, default unlimited) caps the aggregate rate at which the +client sends `DATA` to the hub, shared fairly across streams. Accepts `"20mbps"` +(decimal bit units), `"2MB/s"` (binary byte units), or a bare number of bytes per +second; the minimum is 8192 B/s. **This is a purely local policy and has no +effect on the wire format** — a shaped client is indistinguishable from a slow +one, and the hub needs no support for it. Only `DATA` is paced; control frames +are never delayed. + Each `pattern` is a regular expression (§5.1) matched against the whole normalized player hostname, case-insensitively. Escape literal dots (`mc\.example\.com`, which is `mc\\.example\\.com` in JSON); an unescaped `.` matches any character. @@ -483,6 +659,9 @@ not appear on the tunnel wire, and the exchange is invisible to the player. | heartbeat timeout | `3 × pingIntervalMs` | | hub session idle timeout | 90000 ms (`0` disables) | | stream window default / bounds | 256 KiB, clamped to [32 KiB, 8 MiB] | +| feature flag: stream resumption | 0x04 (negotiated) | +| mux RESUME / RESUME_ACK | 0x07 / 0x08 | +| resume grace: hub / client default | 20000 ms / 15000 ms (hub value advertised) | +| retained region per stream per direction | bounded by the stream window | | WND grant batching (reference) | one grant per window/2 consumed | | DATA chunk cap (reference) | 32 KiB | -``` diff --git a/README.md b/README.md index b5150b8..b2220e9 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,12 @@ secrets. The base image and build flags live in `.ko.yaml`. | `timestampWindowMs` | `30000` | Allowed clock skew for a client's rekey timestamp. | | `pendingTimeoutMs` | `10000` | How long a matched player waits for a worker to take over. | | `sessionIdleTimeoutMs` | `90000` | Close an established control/worker session that receives no frame for this long. Must exceed the client's `pingIntervalMs`; `0` disables. Player connections are unaffected. | +| `streamWindowBytes` | `262144` | Advertised per-stream receive window, clamped to [32 KiB, 8 MiB]. | +| `streamResume` | `true` | Hang a player when its worker connection drops, so the client can reattach the stream instead of the player being disconnected. `false` restores the previous behaviour exactly and retains nothing. | +| `resumeGraceMs` | `20000` | How long a hung player is held. Advertised to clients, which clamp their own retry budget below it. Must exceed the client's grace by at least one dial. | +| `maxParkedStreams` | `256` | Cap on simultaneously hung players; `maxParkedBytes` (default `maxParkedStreams × 2 × streamWindowBytes`) caps what they retain. Past either, the oldest are dropped. | +| `statsIntervalMs` | `0` (off) | Log a periodic line with live/hung stream counts, retained bytes and pattern count. | +| `registrationGraceMs` | `15000` | Keep a closed control session's routes as *orphaned* for this long, holding players that arrive on them instead of refusing them, and replaying their requests once the client re-registers. `0` disables it. | ### Client (`client/config.example.json`) @@ -178,6 +184,11 @@ secrets. The base image and build flags live in `.ko.yaml`. | `psk` | *(required)* | Shared secret; must match the hub. | | `maxConn` | `1` (clamped 1–8) | Max worker connections in the pool. | | `pingIntervalMs` | `20000` (min 1000) | Heartbeat interval for the control session and every worker conn. A session with no reply for `3×` this is dropped and re-established. | +| `maxBandwidth` | *(unlimited)* | Caps what the client uploads to the hub, summed over every player — the direction carrying the game server's output, and the one a home uplink runs out of first. `"20mbps"`, `"512kbps"`, `"2MB/s"`, or a bare number of bytes/sec. **Bit units are decimal (`20mbps` = 20,000,000 bit/s); byte units are binary (`2MB/s` = 2 MiB/s).** Set it slightly below your real upload speed — framing and TCP/IP overhead are not counted. The budget is shared fairly across players, so one person loading chunks cannot time the others out. | +| `streamWindowBytes` | `262144` | Advertised per-stream receive window, clamped to [32 KiB, 8 MiB]. | +| `streamResume` | `true` | Reattach streams over a fresh connection when a worker connection drops, instead of disconnecting those players. `false` restores the previous behaviour exactly: nothing is retained and the send path is unchanged. | +| `resumeGraceMs` | `15000` (min 2000) | How long a stream keeps trying to reattach, clamped below the hub's advertised grace. Sized against the *backend*: a hung player stops answering the game server's KeepAlive, and vanilla disconnects a silent client at 30s, so a longer grace only resumes sessions the backend then kicks. | +| `statsIntervalMs` | `0` (off) | Log a periodic diagnostics line, plus a summary per stream at close: bytes each way, how long the stream was blocked on the flow-control window versus the bandwidth cap, receive-queue high-water mark, and heartbeat round-trip time per connection. Those distinguish a slow backend from a saturated uplink from a bad path, which throughput alone cannot. | | `mappings[]` | *(≥1 required)* | Route table (below). | | `mappings[].pattern` | — | Regex matched against the whole player hostname, case-insensitively. Escape dots (`mc\.example\.com`); `.` is a wildcard. | | `mappings[].destination` | — | Real server `host:port` to forward to. | @@ -211,8 +222,13 @@ streams spreading across multiple worker connections, HAProxy v2 source-address propagation, Velocity modern-forwarding interception (signed player-info handoff to a mock Paper backend), player- and destination-initiated disconnect propagation, wrong-PSK rejection, dropping of unmatched hostnames, stream isolation under a slow -player and under a slow destination (no head-of-line blocking), and rejection -of pre-flow-control peers. The Go and Java crypto layers are +player and under a slow destination (no head-of-line blocking), rejection +of pre-flow-control peers, and stream resumption — a tunnel hard-reset +mid-transfer with the player connection held open, asserting the byte stream +neither gains nor loses a byte, across concurrent streams, plus grace expiry and +the resume-disabled path, and control-outage handling — a player arriving while +the client's control session is down is held and then served once it +re-registers, with the grace-disabled and grace-expired paths pinned too. The Go and Java crypto layers are independently pinned to the same SHA3-224 test vector so they cannot silently drift apart. @@ -234,6 +250,17 @@ drift apart. detected within `3 × pingIntervalMs`, the dead connection is dropped from the pool, and service is restored without operator action. TCP keepalive is on as a second line of defence. +* **A control-session reconnect no longer refuses new players.** While a client + is reconnecting the hub has no route for it, so arriving players used to be + told there is no such server. Those routes are now held briefly and the + players with them, then served once the client re-registers. +* **A dropped tunnel no longer drops the players.** A worker connection is only + the middle leg of the streams it carries; when it dies both terminal sockets + are usually still healthy. The hub now hangs those players while the client + reattaches their streams over a fresh connection, replaying byte-exactly from + the offset the peer reports, so a conntrack expiry costs a stall rather than + disconnecting everyone on that connection. Negotiated, and `streamResume: + false` on either side restores the old behaviour. * **Single hub event loop.** The hub deploys one Vert.x verticle, so all state is confined to one event loop (no locking). Throughput is bounded by one core; ample for hundreds of players, not designed for tens of thousands. diff --git a/client/client.go b/client/client.go index d0ee552..5dd447b 100644 --- a/client/client.go +++ b/client/client.go @@ -25,7 +25,9 @@ type Client struct { mappings map[string]Mapping // normalized pattern -> mapping pool *WorkerPool - streamWnd int // our advertised per-stream receive window (bytes) + streamWnd int // our advertised per-stream receive window (bytes) + shaper *Shaper // caps aggregate egress to the hub; nil when unlimited + chunk int // DATA payload cap; shrinks below DataChunkSize at low rates mu sync.Mutex ctrl *wire.FramedConn @@ -48,6 +50,19 @@ func New(cfg *Config) *Client { c.mappings[NormalizeAddress(m.Pattern)] = m } c.streamWnd = clampWindow(cfg.StreamWindowBytes) + // Parsed here rather than in LoadConfig because a Config may also be built + // directly (tests). LoadConfig has already rejected a malformed value on the + // file path, so a failure here can only come from a hand-built Config. + bps, err := parseBandwidth(cfg.MaxBandwidth) + if err != nil { + log.Printf("client: %v; continuing without a bandwidth limit", err) + } + c.shaper = NewShaper(bps) + c.chunk = c.shaper.chunkSize() + if c.shaper != nil { + log.Printf("egress shaped to %d B/s (burst %d B, chunk %d B)", + bps, int64(c.shaper.burst), c.shaper.chunk) + } c.pool = newWorkerPool(c, cfg.MaxConn) return c } @@ -71,6 +86,10 @@ type session struct { fc *wire.FramedConn peerWnd int // hub's advertised per-stream receive window heartbeat bool // hub accepted mux-level PING/PONG on worker conns + resume bool // hub accepted stream resumption (§7.5) + // hubGrace is how long the hub will hang a parked player, as advertised in + // SessionReady. Zero when resumption was not negotiated. + hubGrace time.Duration } // dialSession opens a TCP connection, performs the Intent-17 handshake, the @@ -120,6 +139,9 @@ func (c *Client) dialSession(magic byte) (sess *session, err error) { } ts := time.Now().UnixMilli() offered := FlagStreamFC | FlagWorkerHeartbeat + if c.cfg.resumeEnabled() { + offered |= FlagStreamResume + } rekeyMsg := wire.NewWriter().U8(magic).VarInt(len(rnd)).Bytes(rnd).I64(ts). VarInt(offered).VarInt(c.streamWnd).Out() if err := fc.WriteFrame(rekeyMsg); err != nil { @@ -155,6 +177,22 @@ func (c *Client) dialSession(magic byte) (sess *session, err error) { if hubWnd > MaxStreamWindow { hubWnd = MaxStreamWindow } + // Resumption is negotiated per connection, and the hub's grace period rides + // along when it accepts. Our own grace is clamped strictly under the hub's: + // the client must always give up first, or the hub drops a hanging player + // while we are still mid-reattach. A hub that accepts the flag but omits the + // grace is treated as not supporting it at all rather than guessed at. + resume := flags&FlagStreamResume != 0 + var hubGrace time.Duration + if resume { + graceMs, gerr := r.VarInt() + if gerr != nil || graceMs <= 0 { + log.Printf("hub accepted stream resume without advertising a grace period; disabling resume") + resume = false + } else { + hubGrace = time.Duration(graceMs) * time.Millisecond + } + } // The session is live: drop the establishment deadline. From here on // liveness is the heartbeat's job (and WriteFrame bounds each write). @@ -162,14 +200,30 @@ func (c *Client) dialSession(magic byte) (sess *session, err error) { return nil, err } ok = true - return &session{fc: fc, peerWnd: hubWnd, heartbeat: flags&FlagWorkerHeartbeat != 0}, nil + return &session{ + fc: fc, + peerWnd: hubWnd, + heartbeat: flags&FlagWorkerHeartbeat != 0, + resume: resume, + hubGrace: hubGrace, + }, nil } +// statsOn reports whether performance diagnostics are enabled. When off, no +// counter struct is ever allocated and the instrumentation is a single branch. +func (c *Client) statsOn() bool { return c.cfg.StatsIntervalMs > 0 } + // Start establishes the control session and registers all patterns. It returns // once the initial connection succeeds; subsequent drops are handled in the // background with reconnect. func (c *Client) Start(ctx context.Context) error { - return c.connectControl(ctx) + if err := c.connectControl(ctx); err != nil { + return err + } + if c.statsOn() { + go c.statsLoop(ctx.Done()) + } + return nil } func (c *Client) connectControl(ctx context.Context) error { @@ -220,17 +274,33 @@ func (c *Client) serveControl(ctx context.Context, ctrl *ctrlSession) { if ctx.Err() != nil { return } - // Reconnect with backoff. - for backoff := 500 * time.Millisecond; ctx.Err() == nil; backoff *= 2 { - if backoff > 10*time.Second { - backoff = 10 * time.Second + // Reconnect with backoff, but try immediately first. While the control + // session is down the hub has no live route for this client, so every + // millisecond of delay is a player arriving to be told there is no such + // server — and a session usually dies to a transient blip that the very next + // dial would have survived. Sleeping first spent that window unconditionally. + // + // The wait is on ctx rather than time.Sleep so shutdown is not held up by a + // backoff that has grown to the cap. + for backoff := time.Duration(0); ctx.Err() == nil; { + if backoff > 0 { + select { + case <-ctx.Done(): + return + case <-time.After(backoff): + } } - time.Sleep(backoff) if err := c.connectControl(ctx); err == nil { return } else { log.Printf("control reconnect failed: %v", err) } + switch { + case backoff == 0: + backoff = 500 * time.Millisecond + case backoff < maxControlBackoff: + backoff = min(backoff*2, maxControlBackoff) + } } } @@ -299,20 +369,38 @@ func (c *Client) handleControlRequest(cid []byte, pattern, ip string, port int) return } log.Printf("player %s:%d joined via pattern %q -> %s", ip, port, pattern, mapping.Destination) - wc, sid, err := c.pool.Allocate() - if err != nil { - log.Printf("worker allocate failed: %v", err) + + // Allocate and publish must agree on a live conn: Allocate hands out a + // (conn, sid) pair that can die before we register on it, which would strand + // the stream in a map nothing iterates. registerStream reports that, and we + // simply pick another conn. + var st *Stream + var lg *leg + for attempt := 0; attempt < allocateAttempts; attempt++ { + wc, sid, err := c.pool.Allocate() + if err != nil { + log.Printf("worker allocate failed: %v", err) + return + } + st = newStream(c, wc, sid, cid, mapping, ip, port) + // Register before SYN so inbound DATA can never race ahead of the table, + // and start the pumps before the (bounded) SYN write so a failed or slow + // SYN cannot strand a stream that nothing would ever tear down. + if wc.registerStream(sid, st) { + lg = st.conn() + break + } + st = nil + } + if st == nil { + log.Printf("worker allocate failed: no live conn after %d attempts", allocateAttempts) return } - st := newStream(wc, sid, cid, mapping, ip, port) - // Register before SYN so inbound DATA can never race ahead of the table, - // and start the pumps before the (bounded) SYN write so a failed or slow - // SYN cannot strand a stream that nothing would ever tear down. - wc.registerStream(sid, st) + go st.writeLoop() go st.run() - if err := wc.sendSyn(sid, cid); err != nil { - log.Printf("stream %d: SYN failed: %v", sid, err) + if err := lg.wc.sendSyn(lg.sid, cid); err != nil { + log.Printf("stream %d: SYN failed: %v", lg.sid, err) st.teardown(false) } } @@ -330,4 +418,5 @@ func (c *Client) Close() { _ = fc.Close() } c.pool.closeAll() + c.shaper.Stop() } diff --git a/client/config.example.json b/client/config.example.json index 8d873ee..36afedb 100644 --- a/client/config.example.json +++ b/client/config.example.json @@ -4,6 +4,10 @@ "maxConn": 4, "pingIntervalMs": 20000, "streamWindowBytes": 262144, + "maxBandwidth": "", + "streamResume": true, + "resumeGraceMs": 15000, + "statsIntervalMs": 0, "mappings": [ { "pattern": "mc\\.example\\.com", diff --git a/client/config.go b/client/config.go index 0fb75be..f6ba090 100644 --- a/client/config.go +++ b/client/config.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "strconv" "strings" "time" ) @@ -33,11 +34,26 @@ const ( MuxWnd = 0x04 MuxPing = 0x05 MuxPong = 0x06 + // MuxResume reattaches a parked stream to this conn (CID + our accepted + // offset); MuxResumeAck carries the hub's accepted offset and a fresh CID. + MuxResume = 0x07 + MuxResumeAck = 0x08 // MuxCtlSid is the reserved stream id carrying connection-scoped mux frames // (PING/PONG). Real streams are numbered from 1. MuxCtlSid = 0 + // RST reason codes (optional trailing byte; absence means "unspecified"). + // Distinguishing them matters for resume: an unknown stream is terminal, + // while "already bound" means a racing attempt won and this one must leave + // the stream alone rather than tear down a player the hub just rebound. + RstUnspecified = 0x00 + RstUnknownStream = 0x01 + RstAlreadyBound = 0x02 + RstResumeAbandoned = 0x03 + RstFlowControl = 0x04 + RstDialFailed = 0x05 + FrameError = 0x7F // SaturationThreshold caps how many streams share one worker conn once the @@ -52,6 +68,12 @@ const ( // firewall) is never detected: the read loop parks forever, the dead conn // stays in the pool, and no player can be served until the client restarts. FlagWorkerHeartbeat = 0x02 + // FlagStreamResume enables stream resumption (PROTOCOL.md §7.5): a worker + // conn drop parks its streams instead of killing them, the hub hangs the + // player sockets, and the client reattaches each stream byte-exactly over a + // fresh conn. Negotiated, so either side may decline and get today's + // behaviour (immediate teardown) unchanged. + FlagStreamResume = 0x04 // Per-stream flow-control window bounds (bytes). The advertised window is the // receiver's promise of how much un-credited DATA it will buffer per stream. @@ -64,6 +86,32 @@ const ( DataChunkSize = 32 * 1024 ) +// Egress bandwidth shaping (see shaper.go and docs/architecture.md §6). These +// are entirely client-local: nothing here appears on the wire. +const ( + // MinBandwidth floors a configured cap. Below this the tunnel cannot carry a + // Minecraft session at all, so such a value is a unit typo ("20bps" for + // "20mbps") and is rejected rather than silently clamped. + MinBandwidth = 8 * 1024 + + // ShaperBurstSeconds is how much transmission time the token bucket banks + // while idle. Big enough to absorb a chunk-load spike; small enough that + // releasing it cannot overrun the physical uplink and rebuild the standing + // queue the cap exists to prevent. + ShaperBurstSeconds = 0.2 + + // MinShaperBurst must exceed DataChunkSize: a request larger than the bucket + // could never be afforded and would park forever. + MinShaperBurst = 64 * 1024 + MaxShaperBurst = 4 << 20 + + // ShaperSliceSeconds bounds how long one stream holds the link before the + // scheduler can switch, by sizing the send chunk to that much transmission + // time. Above ~13 Mbps this yields DataChunkSize and nothing changes. + ShaperSliceSeconds = 0.02 + MinShaperChunk = 4 * 1024 +) + // Timeouts. Every tunnel socket is covered by one of these: without them a // silently dropped path (no FIN/RST) leaves the client parked forever. const ( @@ -85,6 +133,32 @@ const ( // heartbeat timeout can never be short enough to cause spurious drops. // Applied in LoadConfig, i.e. to configs that come from disk. MinPingIntervalMs = 1000 + + // DefaultResumeGraceMs is how long a parked stream keeps trying to reattach + // before giving up and closing the destination. + // + // Chosen against the backend, not the tunnel: a hung player stops answering + // the game server's KeepAlive, and vanilla disconnects a silent client at + // 30s. A longer grace would resume sessions the backend then kicks anyway. + DefaultResumeGraceMs = 15000 + + // MinResumeGraceMs floors the grace so it can always fit at least one dial; + // a grace shorter than HandshakeTimeout could never complete an attempt. + MinResumeGraceMs = 2000 + + // ResumeRetryDelay paces reattach attempts after a failure. Short, because + // the player is hanging for the whole grace period. + ResumeRetryDelay = 500 * time.Millisecond + + // maxControlBackoff caps the control-session reconnect delay. The hub holds + // this client's routes only for its own registration grace, so a backoff that + // grew past that would strand players it is hanging on our behalf. + maxControlBackoff = 10 * time.Second + + // ResumeAckTimeout bounds the wait for RESUME_ACK on a conn that completed + // its handshake but then went quiet, so a wedged hub does not consume the + // entire grace budget in one attempt. + ResumeAckTimeout = 10 * time.Second ) // heartbeatTimeout is how long a session may go without a reply before it is @@ -111,12 +185,87 @@ type Mapping struct { // Config is the client configuration (PROTOCOL.md §9.2). type Config struct { - Server string `json:"server"` - PSK string `json:"psk"` - MaxConn int `json:"maxConn"` - PingIntervalMs int `json:"pingIntervalMs"` - StreamWindowBytes int `json:"streamWindowBytes"` // per-stream receive window; 0 = default - Mappings []Mapping `json:"mappings"` + Server string `json:"server"` + PSK string `json:"psk"` + MaxConn int `json:"maxConn"` + PingIntervalMs int `json:"pingIntervalMs"` + StreamWindowBytes int `json:"streamWindowBytes"` // per-stream receive window; 0 = default + // MaxBandwidth caps what the client sends to the hub, aggregated over every + // stream on every worker conn — the direction that carries the game server's + // output to the players, and the one a residential uplink runs out of first. + // Empty means no limit. See parseBandwidth for the accepted syntax. + MaxBandwidth string `json:"maxBandwidth"` + // StreamResume enables stream resumption (PROTOCOL.md §7.5). A pointer so an + // absent key means "on" while an explicit false disables it: with it off the + // client never offers the flag, allocates no retransmit buffers, and behaves + // exactly as a pre-resume client. + StreamResume *bool `json:"streamResume"` + // ResumeGraceMs bounds how long a parked stream keeps trying to reattach. + // Clamped below the hub's advertised grace so the client always gives up + // first and the hub is never left holding a player nobody will claim. + ResumeGraceMs int `json:"resumeGraceMs"` + // StatsIntervalMs enables the periodic performance summary; 0 (the default) + // disables it and costs nothing. + StatsIntervalMs int `json:"statsIntervalMs"` + Mappings []Mapping `json:"mappings"` +} + +// resumeEnabled reports whether stream resumption is configured on. +func (c *Config) resumeEnabled() bool { return c.StreamResume == nil || *c.StreamResume } + +// resumeGrace is how long a parked stream may keep trying to reattach. +func (c *Config) resumeGrace() time.Duration { + ms := c.ResumeGraceMs + if ms <= 0 { + ms = DefaultResumeGraceMs + } + if ms < MinResumeGraceMs { + ms = MinResumeGraceMs + } + return time.Duration(ms) * time.Millisecond +} + +// bandwidthUnits maps a rate suffix to its value in bytes per second. Bit units +// are decimal because that is what ISPs quote; byte units are binary to match +// streamWindowBytes. Ordered longest-suffix-first so "kbps" is not read as +// "bps", nor "gb/s" as "b/s". +var bandwidthUnits = []struct { + suffix string + mul float64 +}{ + {"gbps", 1e9 / 8}, {"gbit", 1e9 / 8}, + {"mbps", 1e6 / 8}, {"mbit", 1e6 / 8}, + {"kbps", 1e3 / 8}, {"kbit", 1e3 / 8}, + {"gb/s", 1 << 30}, {"mb/s", 1 << 20}, {"kb/s", 1 << 10}, + {"bps", 1.0 / 8}, + {"b/s", 1}, +} + +// parseBandwidth converts a human-readable rate to bytes per second. The empty +// string means "no limit" and yields 0. +// +// "20mbps" 20 megabits/s = 2500000 B/s +// "512kbps" 512 kilobits/s = 64000 B/s +// "2MB/s" 2 mebibytes/s = 2097152 B/s +// "1500000" a bare number is already bytes per second +func parseBandwidth(s string) (int64, error) { + s = strings.TrimSpace(s) + if s == "" { + return 0, nil + } + lower := strings.ToLower(s) + num, mul := lower, 1.0 + for _, u := range bandwidthUnits { + if strings.HasSuffix(lower, u.suffix) { + num, mul = strings.TrimSpace(lower[:len(lower)-len(u.suffix)]), u.mul + break + } + } + v, err := strconv.ParseFloat(num, 64) + if err != nil || v <= 0 { + return 0, fmt.Errorf(`maxBandwidth: cannot read %q as a rate (try "20mbps", "2MB/s", or bytes per second)`, s) + } + return int64(v * mul), nil } // LoadConfig reads and validates a JSON config file. @@ -147,6 +296,14 @@ func LoadConfig(path string) (*Config, error) { if c.PingIntervalMs < MinPingIntervalMs { c.PingIntervalMs = MinPingIntervalMs } + // Parsed here only to fail fast on a bad value; New does the real conversion. + bps, err := parseBandwidth(c.MaxBandwidth) + if err != nil { + return nil, err + } + if bps > 0 && bps < MinBandwidth { + return nil, fmt.Errorf("maxBandwidth %q is only %d B/s; the minimum is %d B/s", c.MaxBandwidth, bps, MinBandwidth) + } if len(c.Mappings) == 0 { return nil, fmt.Errorf("at least one mapping is required") } diff --git a/client/pool_test.go b/client/pool_test.go index a4a5152..df5d9b0 100644 --- a/client/pool_test.go +++ b/client/pool_test.go @@ -85,7 +85,9 @@ func TestAllocateSpreadsAcrossConns(t *testing.T) { return &WorkerConn{pool: p, streams: make(map[int]*Stream), nextSid: 1, done: make(chan struct{})} } p.conns = []*WorkerConn{newConn()} - p.conns[0].registerStream(1, &Stream{sid: 1}) + if !p.conns[0].registerStream(1, &Stream{}) { + t.Fatal("registerStream refused on a live conn") + } // One conn holding a stream, pool below maxConn: growth is warranted. _, bestCount := p.leastLoadedLocked() diff --git a/client/resume.go b/client/resume.go new file mode 100644 index 0000000..585921c --- /dev/null +++ b/client/resume.go @@ -0,0 +1,332 @@ +package client + +import ( + "errors" + "log" + "time" + + "github.com/iceBear67/redapricot/client/wire" +) + +// Stream resumption (PROTOCOL.md §7.5). +// +// A worker conn carries many players but is only the middle leg of each: when +// it dies, both terminal sockets are usually still perfectly healthy. Tearing +// the streams down therefore throws away working connections because a +// replaceable transport failed — one conntrack expiry disconnects everyone on +// that conn. +// +// Instead the stream parks: the destination socket stays open, the hub hangs the +// player socket, and the client reattaches over a fresh conn. Correctness rests +// on retransmission being byte-exact. Bytes handed to a dying socket are lost +// with no notification, and the frame cipher cannot be resynchronized, so each +// side replays from the offset the other reports it accepted. + +var ( + errResumeUnknown = errors.New("hub does not know this stream") + errResumeRaced = errors.New("another reattach already bound this stream") + errResumeRefused = errors.New("hub refused the reattach") + errResumeTimeout = errors.New("no RESUME_ACK from the hub") + errResumeConnLost = errors.New("the conn carrying the reattach died") + errResumeTooOld = errors.New("hub accepted past what we still hold") + errResumeNoResume = errors.New("hub does not support stream resumption") +) + +// resumeGrace is how long a stream parked from a conn may keep trying, clamped +// under what the hub advertised. The client must always give up first: a hub +// that drops the player while we are still reattaching would leave us pumping a +// destination nobody is reading. +func (c *Client) resumeGrace(hubGrace time.Duration) time.Duration { + grace := c.cfg.resumeGrace() + if hubGrace > 0 && hubGrace < grace { + grace = hubGrace + } + return grace +} + +// park suspends a stream whose worker conn died instead of destroying it, and +// starts trying to reattach. Reports false when the stream cannot be parked, in +// which case the caller tears it down as before. +// +// A stream already closing (finPending) is not parked: the hub has said the +// player is gone, so there is nothing left to preserve. +func (s *Stream) park(grace time.Duration) bool { + if !s.resumable { + return false + } + s.mu.Lock() + if s.closed || s.finPending { + s.mu.Unlock() + return false + } + already := s.parked + s.parked = true + if s.stats != nil && !already { + s.parkedAt = time.Now() + } + s.mu.Unlock() + + if already { + // A reattach was already in flight and had registered this stream on the + // conn that just died — which is how it got here at all. That attempt + // still owns the stream, so reporting failure would have the caller tear + // down a player that is mid-recovery. Fail its wait immediately rather + // than let it sit out the ack timeout: the grace budget is small, and + // spending ten seconds of it waiting on a socket that is already gone is + // the difference between reattaching and dropping the player. + s.deliverResume(resumeResult{err: errResumeConnLost}) + return true + } + go s.resumeLoop(grace) + return true +} + +// resumeLoop reattaches the stream, retrying until it succeeds or the grace +// period runs out. +// +// Attempts are started right up to the deadline rather than reserving a whole +// dial's worth of budget for the last one. Reserving it would be self-defeating +// — the grace and HandshakeTimeout are the same order of magnitude, so the +// reservation can consume the entire budget and leave no attempt at all — and +// overshooting is safe: an attempt that lands after the hub has dropped the +// player is answered with RST(unknown stream) and tears down cleanly. +func (s *Stream) resumeLoop(grace time.Duration) { + deadline := time.Now().Add(grace) + for { + if s.isClosed() { + return + } + if !time.Now().Before(deadline) { + break + } + err := s.tryResume() + if err == nil { + return + } + if errors.Is(err, errResumeRaced) { + // Another attempt owns the stream now; leaving it alone is the whole + // point — tearing down here would kill a player the hub considers live. + return + } + if errors.Is(err, errResumeUnknown) || errors.Is(err, errResumeTooOld) { + // Terminal: the hub has no state for this stream (it restarted, the + // grace expired, or a load balancer sent us to a different instance). + log.Printf("stream resume abandoned: %v", err) + s.teardown(false) + return + } + select { + case <-s.done: + return + case <-time.After(ResumeRetryDelay): + } + } + log.Printf("stream resume gave up after %s; closing destination", grace) + // No FIN: the only conns we could send it on are the ones that just failed + // us. The hub drops the hanging player when its own grace expires. + s.teardown(false) +} + +// tryResume performs one reattach attempt: find a live conn, claim a stream id +// on it, send RESUME, and replay from wherever the hub says it got to. +func (s *Stream) tryResume() error { + wc, sid, err := s.allocateForResume() + if err != nil { + return err + } + + s.mu.Lock() + cid := s.cid + accepted := s.acceptedOffset + delivered := s.deliveredOffset + wait := make(chan resumeResult, 1) + s.resumeWait = wait + s.mu.Unlock() + + msg := wire.NewWriter().U8(MuxResume).VarInt(sid).Bytes(cid). + I64(accepted).I64(delivered).Out() + if err := wc.fc.WriteFrame(msg); err != nil { + s.abandonAttempt(wc, sid) + return err + } + + var res resumeResult + select { + case res = <-wait: + case <-s.done: + // Torn down while waiting. teardown only deregisters the leg the stream + // was bound to, which is not this one, so the claim made above has to be + // withdrawn here or it stays in the new conn's table forever. + s.abandonAttempt(wc, sid) + return errResumeRefused + case <-time.After(ResumeAckTimeout): + s.abandonAttempt(wc, sid) + return errResumeTimeout + } + if res.err != nil { + s.abandonAttempt(wc, sid) + return res.err + } + return s.completeResume(wc, sid, res) +} + +// allocateForResume picks a live conn that will honour a reattach. +func (s *Stream) allocateForResume() (*WorkerConn, int, error) { + for attempt := 0; attempt < allocateAttempts; attempt++ { + wc, sid, err := s.client.pool.Allocate() + if err != nil { + return nil, 0, err + } + // Re-checked per conn, not assumed from the dead one: this may be a + // different or restarted hub. Sending RESUME to a hub that does not know + // the frame type would hang the player for the rest of the grace waiting + // for an answer that is never coming. + if !wc.resume { + return nil, 0, errResumeNoResume + } + if wc.registerStream(sid, s) { + return wc, sid, nil + } + } + return nil, 0, errResumeRefused +} + +// abandonAttempt withdraws a failed attempt from the conn it was made on, so a +// retry can never leave two RESUMEs outstanding for one stream. +func (s *Stream) abandonAttempt(wc *WorkerConn, sid int) { + wc.removeStream(sid) + s.mu.Lock() + s.resumeWait = nil + s.mu.Unlock() +} + +// completeResume rebinds the stream to its new conn and replays what the hub is +// missing, holding sendMu throughout so live traffic cannot overtake the replay. +func (s *Stream) completeResume(wc *WorkerConn, sid int, res resumeResult) error { + s.sendMu.Lock() + // Delivery is a strictly stronger fact than credit — the hub only credits what + // it has delivered — so the reported offset can be adopted wholesale. Doing so + // also repairs the ledger: the grants destroyed by the outage are exactly the + // gap between the two, and without this the retained region would carry that + // dead prefix for the rest of the stream's life. + s.ackedOffset.Store(res.delivered) + s.un.advance(res.delivered) + replay := s.un.from(res.accepted) + if replay == nil { + s.sendMu.Unlock() + s.abandonAttempt(wc, sid) + return errResumeTooOld + } + // Three offsets, three jobs, and conflating any two of them breaks something + // different. + // + // What to replay is measured from what the hub *accepted* — the bytes it + // never received. What the window should be is measured from what it + // *delivered*, because the window is a promise about undelivered bytes. + // It cannot be measured from what it *credited*: credit arrives as deltas, + // and the grants in flight when the connection died are gone for good, so a + // window derived from them would be permanently short — and, when a full + // window was outstanding at the drop, permanently zero. That is a deadlock, + // not a slowdown: no credit can arrive because nothing can be sent. + outstanding := s.un.length() + replayed := s.un.end() - res.accepted + + // Publish the new binding before any frame goes out on it, and as one value: + // stream ids restart at 1 per conn, so a half-updated pair would address a + // different player's stream. + s.leg.Store(&leg{wc: wc, sid: sid}) + + s.mu.Lock() + // Restated, not patched. The window is a delta ledger and the outage tore a + // hole in it; deriving it afresh from the delivered offset closes the hole + // exactly, whatever was lost. + s.sendWnd = wc.sendWndInit - int(outstanding) + if s.sendWnd < 0 { + s.sendWnd = 0 + } + // Symmetrically, our own pending credit is discarded rather than flushed: + // the delivered offset we reported already tells the hub everything those + // deltas would have, and sending both would grant the same bytes twice. + // Counting resumes from this baseline. + s.consumed = 0 + if len(res.cid) == CIDLen { + s.cid = res.cid // fresh capability, so a CID is never reusable twice + } + s.parked = false + s.resumeWait = nil + owedFin := s.finToHub + if s.stats != nil { + s.stats.resumes++ + s.stats.hung += time.Since(s.parkedAt) + s.stats.replayBytes += replayed + } + s.cond.Broadcast() // release acquireSendWnd and any parked writer + s.mu.Unlock() + + for len(replay) > 0 { + n := len(replay) + if n > s.client.chunk { + n = s.client.chunk + } + if err := wc.sendData(sid, replay[:n]); err != nil { + s.sendMu.Unlock() + return err + } + replay = replay[n:] + } + s.sendMu.Unlock() + + // A destination that closed while we were parked owed the hub a FIN that had + // nowhere to go at the time. + if owedFin { + wc.sendFin(sid) + s.teardown(false) + return nil + } + log.Printf("stream %d resumed (%d bytes replayed, %d outstanding)", sid, replayed, outstanding) + return nil +} + +// deliverResume hands an answer to a reattach that is waiting for one. Reports +// false when no attempt was in flight, so the caller can treat the frame as it +// would on any live stream. +func (s *Stream) deliverResume(res resumeResult) bool { + s.mu.Lock() + ch := s.resumeWait + s.resumeWait = nil + s.mu.Unlock() + if ch == nil { + return false + } + ch <- res // buffered, and read at most once per attempt + return true +} + +// onRst applies an RST, using the reason to tell a stream that is genuinely gone +// from one that a racing reattach has taken over. +func (s *Stream) onRst(reason int) { + err := errResumeRefused + switch reason { + case RstUnknownStream: + err = errResumeUnknown + case RstAlreadyBound: + err = errResumeRaced + } + if s.deliverResume(resumeResult{err: err}) { + return + } + s.teardown(false) +} + +// noteFinWhileParked records a FIN the stream owes the hub but cannot send, +// because the only conn it has is the one that just died. Reports false when the +// stream is not parked and the caller should send it normally. +func (s *Stream) noteFinWhileParked() bool { + s.mu.Lock() + defer s.mu.Unlock() + if !s.parked { + return false + } + s.finToHub = true + return true +} diff --git a/client/resume_bench_test.go b/client/resume_bench_test.go new file mode 100644 index 0000000..c42b102 --- /dev/null +++ b/client/resume_bench_test.go @@ -0,0 +1,104 @@ +package client + +import ( + "testing" +) + +// The retained region is the one real cost stream resumption adds to the send +// path: a chunk has to survive past the frame write, so it is copied. These +// pin both halves of that claim — that the copy is the only cost, and that +// disabling the feature removes it entirely rather than merely shrinking it. + +func benchStream(resumable bool) *Stream { + s := &Stream{resumable: resumable} + return s +} + +// BenchmarkRetainChunk measures what emit adds over a bare frame write: the +// trim-and-append into the retained region. Compare the two variants; the delta +// is the per-byte copy the feature costs. +func BenchmarkRetainChunk(b *testing.B) { + chunk := make([]byte, DataChunkSize) + window := int64(DefaultStreamWindow) + + b.Run("resume-on", func(b *testing.B) { + s := benchStream(true) + b.SetBytes(int64(len(chunk))) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + // Model the steady state: credit trails one window behind, so the + // buffer trims about as fast as it grows and stays bounded. + acked := s.un.end() - window + if acked < 0 { + acked = 0 + } + s.un.advance(acked) + s.un.append(chunk) + } + if got := int64(s.un.length()); got > window+int64(len(chunk)) { + b.Fatalf("retained region grew past one window: %d", got) + } + }) + + b.Run("resume-off", func(b *testing.B) { + s := benchStream(false) + b.SetBytes(int64(len(chunk))) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if s.resumable { + s.un.advance(0) + s.un.append(chunk) + } + } + }) +} + +// TestResumeDisabledAllocatesNothing pins the off switch at the level that +// matters. It is easy for a feature flag to stop the wire behaviour while +// leaving the bookkeeping running, which would keep the memory cost and the +// per-byte copy for a user who explicitly turned it off — a partial revert that +// nobody would notice. +func TestResumeDisabledAllocatesNothing(t *testing.T) { + chunk := make([]byte, DataChunkSize) + s := benchStream(false) + + allocs := testing.AllocsPerRun(1000, func() { + if s.resumable { + s.un.advance(0) + s.un.append(chunk) + } + }) + if allocs != 0 { + t.Fatalf("resume disabled still allocated %.1f times per send", allocs) + } + if s.un.buf != nil { + t.Fatalf("resume disabled still allocated a retained region of %d bytes", cap(s.un.buf)) + } +} + +// TestRetainedRegionStaysWithinWindow is the memory bound the design rests on: +// flow control already caps outstanding bytes at one window, so the retained +// region needs no cap of its own. If that ever stopped holding, a busy stream +// would grow without limit and the hub would be the first to notice. +func TestRetainedRegionStaysWithinWindow(t *testing.T) { + const window = DefaultStreamWindow + chunk := make([]byte, DataChunkSize) + var u unackedBuf + + for i := 0; i < 5000; i++ { + // A sender may never have more than one window outstanding, which is + // exactly what acquireSendWnd enforces before emit is ever reached. + if u.length()+len(chunk) > window { + u.advance(u.base() + int64(len(chunk))) + } + u.append(chunk) + if u.length() > window { + t.Fatalf("round %d: retained %d bytes for a %d-byte window", i, u.length(), window) + } + } + if cap(u.buf) > 4*window { + t.Fatalf("backing array grew to %d for a %d-byte window", cap(u.buf), window) + } +} diff --git a/client/shaper.go b/client/shaper.go new file mode 100644 index 0000000..b23f5d2 --- /dev/null +++ b/client/shaper.go @@ -0,0 +1,268 @@ +package client + +import ( + "sync" + "time" +) + +// minShaperWait floors the dispatcher's sleep so floating-point dust in the +// token arithmetic cannot spin it. +const minShaperWait = time.Millisecond + +// Shaper caps the aggregate rate at which the client writes DATA to the hub and +// divides that budget across streams. +// +// The credit windows of PROTOCOL.md §7.3 bound how many bytes may be *in flight* +// per stream; they say nothing about bytes per *second*. That is the gap this +// fills. On a residential uplink one player loading chunks will otherwise +// saturate the line and push every other player's keepalive past its timeout. +// +// Two mechanisms are layered: +// +// - A token bucket sets the long-run rate and the size of the burst that may +// be spent after an idle period. +// - Start-time fair queueing decides who spends those tokens. A global virtual +// clock advances with each grant; every stream remembers the virtual time at +// which its last request finished. A request is stamped +// max(share.vfinish, vclock) and the lowest stamp is served first, so a +// stream that keeps sending pushes its own stamp further out and yields to +// quieter streams. The clamp to vclock is what keeps bursts cheap: a stream +// returning from idle is pulled back to the head of the clock, so it cannot +// hoard credit while it was idle, but it is not punished for the idleness +// either. One stream alone gets the whole rate. +// +// A nil *Shaper means "no limit"; every method short-circuits, so call sites do +// not branch. +type Shaper struct { + rate float64 // bytes per second + burst float64 // token bucket capacity, bytes + chunk int // how much a caller should request at a time + + mu sync.Mutex + tokens float64 + last time.Time + vclock float64 // virtual time, in bytes of service granted + waiting []*shaperReq // unordered; the dispatcher scans for the lowest vstart + + wake chan struct{} // cap 1, non-blocking: nudges the dispatcher + done chan struct{} + once sync.Once +} + +// shaperShare is one stream's position in the fair queue. It lives on the +// Stream and dies with it; a fresh share starts at zero and is clamped up to +// the current virtual clock on its first request. +type shaperShare struct{ vfinish float64 } + +// shaperReq is one pending Acquire. granted and membership in Shaper.waiting +// are both guarded by Shaper.mu. +type shaperReq struct { + n int + vstart float64 + grant chan struct{} + granted bool +} + +// NewShaper builds a shaper for the given rate. A non-positive rate returns nil, +// which every method treats as "unlimited". +func NewShaper(bytesPerSec int64) *Shaper { + if bytesPerSec <= 0 { + return nil + } + rate := float64(bytesPerSec) + + burst := rate * ShaperBurstSeconds + // The floor is a correctness constraint, not a preference: a request larger + // than the bucket could never be afforded and would park forever. + if burst < MinShaperBurst { + burst = MinShaperBurst + } + if burst > MaxShaperBurst { + burst = MaxShaperBurst + } + + chunk := int(rate * ShaperSliceSeconds) + if chunk < MinShaperChunk { + chunk = MinShaperChunk + } + if chunk > DataChunkSize { + chunk = DataChunkSize + } + + sh := &Shaper{ + rate: rate, + burst: burst, + chunk: chunk, + tokens: burst, + last: time.Now(), + wake: make(chan struct{}, 1), + done: make(chan struct{}), + } + go sh.dispatch() + return sh +} + +// chunkSize is how many bytes a sender should offer per request. It is sized to +// ShaperSliceSeconds of transmission so no stream holds the link for long before +// the scheduler can switch: at 1 Mbps a full 32 KiB chunk takes ~256 ms, which is +// enough dead air to drag other players towards a keepalive timeout. +func (sh *Shaper) chunkSize() int { + if sh == nil { + return DataChunkSize + } + return sh.chunk +} + +// Acquire blocks until n bytes of bandwidth budget are available for the stream +// owning share. It returns false only when cancel fires first, in which case +// nothing was charged. +// +// cancel is the stream's done channel: a stream torn down while parked here must +// not keep a goroutine (and its Stream) alive waiting for tokens it will never +// use. +func (sh *Shaper) Acquire(share *shaperShare, n int, cancel <-chan struct{}) bool { + if sh == nil || n <= 0 { + return true + } + req := &shaperReq{n: n, grant: make(chan struct{})} + + sh.mu.Lock() + // Stamp the request and reserve this stream's slot in virtual time up front, + // so a stream cannot queue many requests at the same cheap stamp. + req.vstart = share.vfinish + if req.vstart < sh.vclock { + req.vstart = sh.vclock + } + share.vfinish = req.vstart + float64(n) + sh.waiting = append(sh.waiting, req) + sh.mu.Unlock() + sh.nudge() + + select { + case <-req.grant: + return true + case <-sh.done: + // Shaping stopped: let live traffic through rather than stalling it. + sh.mu.Lock() + sh.removeLocked(req) + sh.mu.Unlock() + return true + case <-cancel: + sh.mu.Lock() + granted := req.granted + if !granted { + sh.removeLocked(req) + } + sh.mu.Unlock() + return granted + } +} + +// Stop shuts the dispatcher down and releases everyone parked in Acquire. +func (sh *Shaper) Stop() { + if sh == nil { + return + } + sh.once.Do(func() { close(sh.done) }) +} + +// dispatch is the single goroutine that hands out tokens. It sleeps exactly as +// long as the next waiter needs rather than polling on a fixed tick, so an idle +// shaper costs nothing. +func (sh *Shaper) dispatch() { + for { + wait := sh.grantReady() + var tick <-chan time.Time + var timer *time.Timer + if wait > 0 { + timer = time.NewTimer(wait) + tick = timer.C + } + select { + case <-tick: + case <-sh.wake: + case <-sh.done: + if timer != nil { + timer.Stop() + } + return + } + if timer != nil { + timer.Stop() + } + } +} + +// grantReady refills the bucket and grants every waiter it can afford, lowest +// virtual start time first. It returns how long until the next waiter becomes +// affordable, or 0 when nothing is pending. +func (sh *Shaper) grantReady() time.Duration { + sh.mu.Lock() + defer sh.mu.Unlock() + + now := time.Now() + if elapsed := now.Sub(sh.last); elapsed > 0 { + sh.tokens += sh.rate * elapsed.Seconds() + if sh.tokens > sh.burst { + sh.tokens = sh.burst + } + sh.last = now + } + + for { + req := sh.headLocked() + if req == nil { + return 0 + } + // Callers stay under chunkSize, which NewShaper keeps below the bucket. + // Should a future caller not, wait for a full bucket rather than for a + // token count that can never be reached, and let the balance go negative: + // the debt is repaid by the next refill, so the long-run rate still holds. + need := min(float64(req.n), sh.burst) + if need > sh.tokens { + wait := time.Duration((need - sh.tokens) / sh.rate * float64(time.Second)) + if wait < minShaperWait { + wait = minShaperWait + } + return wait + } + sh.tokens -= float64(req.n) + // The clock follows the request being served, never runs ahead of it. + if req.vstart > sh.vclock { + sh.vclock = req.vstart + } + req.granted = true + sh.removeLocked(req) + close(req.grant) + } +} + +// headLocked returns the pending request with the lowest virtual start time. +// A linear scan is deliberate: the queue holds at most one entry per live +// stream (tens, not thousands), so a heap would cost more in complexity than it +// saves in comparisons. +func (sh *Shaper) headLocked() *shaperReq { + var best *shaperReq + for _, w := range sh.waiting { + if best == nil || w.vstart < best.vstart { + best = w + } + } + return best +} + +func (sh *Shaper) removeLocked(req *shaperReq) { + for i, w := range sh.waiting { + if w == req { + sh.waiting = append(sh.waiting[:i], sh.waiting[i+1:]...) + return + } + } +} + +func (sh *Shaper) nudge() { + select { + case sh.wake <- struct{}{}: + default: + } +} diff --git a/client/shaper_test.go b/client/shaper_test.go new file mode 100644 index 0000000..8873893 --- /dev/null +++ b/client/shaper_test.go @@ -0,0 +1,231 @@ +package client + +import ( + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestParseBandwidth(t *testing.T) { + cases := []struct { + in string + want int64 + }{ + {"", 0}, + {"20mbps", 2_500_000}, + {"20Mbps", 2_500_000}, + {"20 mbps", 2_500_000}, + {"1.5mbit", 187_500}, + {"512kbps", 64_000}, + {"1gbps", 125_000_000}, + {"2MB/s", 2 << 20}, + {"500kb/s", 500 << 10}, + {"1GB/s", 1 << 30}, + {"8bps", 1}, + {"4096b/s", 4096}, + {"1500000", 1_500_000}, // bare number is already bytes/sec + } + for _, c := range cases { + got, err := parseBandwidth(c.in) + if err != nil { + t.Errorf("parseBandwidth(%q): unexpected error %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("parseBandwidth(%q) = %d, want %d", c.in, got, c.want) + } + } + + for _, bad := range []string{"fast", "20megabits", "-5mbps", "0", "0mbps", "mbps", "20 mb ps"} { + if _, err := parseBandwidth(bad); err == nil { + t.Errorf("parseBandwidth(%q): expected an error", bad) + } + } +} + +// A nil shaper is the "unlimited" case and must be safe on every path, because +// call sites deliberately do not branch on it. +func TestNilShaperIsUnlimited(t *testing.T) { + var sh *Shaper + if sh = NewShaper(0); sh != nil { + t.Fatal("NewShaper(0) should return nil") + } + if got := sh.chunkSize(); got != DataChunkSize { + t.Errorf("nil chunkSize = %d, want %d", got, DataChunkSize) + } + if !sh.Acquire(&shaperShare{}, 1<<20, nil) { + t.Error("nil Acquire should always succeed") + } + sh.Stop() // must not panic +} + +// The virtual-time bookkeeping is what makes the shaper fair, so assert it +// directly. The rate is high enough that tokens never bind, leaving only the +// stamping under test — no timing, no flakiness. +func TestShaperIdleStreamCannotHoardCredit(t *testing.T) { + sh := NewShaper(1 << 30) + defer sh.Stop() + + var heavy, light shaperShare + for i := 0; i < 10; i++ { + if !sh.Acquire(&heavy, 1000, nil) { + t.Fatal("acquire failed") + } + } + if heavy.vfinish != 10000 { + t.Errorf("heavy.vfinish = %v, want 10000", heavy.vfinish) + } + sh.mu.Lock() + vclock := sh.vclock + sh.mu.Unlock() + if vclock != 9000 { + t.Errorf("vclock = %v, want 9000 (the stamp of the last request served)", vclock) + } + + // light was idle for all of it. Its stale vfinish of 0 must be clamped up to + // the current clock: it may not bank the virtual time it never spent, which + // is what would let it starve heavy on return. + if !sh.Acquire(&light, 1000, nil) { + t.Fatal("acquire failed") + } + if light.vfinish != vclock+1000 { + t.Errorf("light.vfinish = %v, want %v (clamped to the clock, not 1000)", light.vfinish, vclock+1000) + } +} + +func TestShaperEnforcesRate(t *testing.T) { + const rate = 1 << 20 // 1 MiB/s + sh := NewShaper(rate) + defer sh.Stop() + + var share shaperShare + const total = 512 << 10 + const chunk = 8 << 10 + + start := time.Now() + for sent := 0; sent < total; sent += chunk { + if !sh.Acquire(&share, chunk, nil) { + t.Fatal("acquire failed") + } + } + elapsed := time.Since(start) + + // The bucket starts full, so the burst is free and only the remainder is + // paced: (512 KiB - 200 KiB) / 1 MiB/s ≈ 300 ms. Bounds are wide on purpose. + if elapsed < 200*time.Millisecond { + t.Errorf("sent %d bytes at %d B/s in only %v; the cap is not being enforced", total, rate, elapsed) + } + if elapsed > time.Second { + t.Errorf("took %v, far longer than the ~300ms the rate implies", elapsed) + } +} + +// An idle stream must be able to spend the banked burst at once, otherwise a +// player joining pays for the cap in visible chunk-loading latency. +func TestShaperAllowsBurst(t *testing.T) { + sh := NewShaper(1 << 20) + defer sh.Stop() + + var share shaperShare + start := time.Now() + for i := 0; i < 6; i++ { + if !sh.Acquire(&share, 32<<10, nil) { // 192 KiB, inside the 200 KiB bucket + t.Fatal("acquire failed") + } + } + if elapsed := time.Since(start); elapsed > 100*time.Millisecond { + t.Errorf("burst of 192 KiB took %v; the bucket should have covered it instantly", elapsed) + } +} + +// The point of the whole exercise: a stream that never stops asking must not +// crowd another one out. +func TestShaperSharesFairlyBetweenStreams(t *testing.T) { + sh := NewShaper(1 << 20) + defer sh.Stop() + + stop := make(chan struct{}) + var counts [2]atomic.Int64 + var wg sync.WaitGroup + for i := range counts { + wg.Add(1) + go func(i int) { + defer wg.Done() + var share shaperShare + for { + if !sh.Acquire(&share, 4<<10, stop) { + return + } + counts[i].Add(4 << 10) + } + }(i) + } + + // Spend the token bucket before measuring, the way the idle-credit test + // raises the rate so tokens never bind: isolate the property under test. + // + // While the bucket has tokens there is no queue to arbitrate — every request + // is granted the moment it arrives, and being the stream that is owed service + // only helps when both are enqueued at the same instant. The burst is + // therefore first-come-first-served by construction, and at 0.2s of + // transmission it is a quarter of a 600ms window, enough to swamp the result: + // a run where one goroutine happened to win the bucket landed at 520192 vs + // 315392, which is exactly "one took the whole burst, then the two split the + // remainder evenly". + // + // Fairness here is a steady-state property, and that is what matters in + // practice — the bucket is empty whenever the link is actually busy. + time.Sleep(250 * time.Millisecond) + counts[0].Store(0) + counts[1].Store(0) + + time.Sleep(600 * time.Millisecond) + close(stop) + wg.Wait() + + a, b := counts[0].Load(), counts[1].Load() + if a == 0 || b == 0 { + t.Fatalf("one stream was starved entirely: %d vs %d", a, b) + } + lo, hi := min(a, b), max(a, b) + t.Logf("steady-state split: %d vs %d bytes (%.3fx)", a, b, float64(hi)/float64(lo)) + if float64(hi) > 1.35*float64(lo) { + t.Errorf("unfair split: %d vs %d bytes (>35%% apart)", a, b) + } +} + +// A stream torn down while parked must release immediately and leave no trace +// in the queue, or its goroutine (and the Stream it closes over) leaks. +func TestShaperAcquireCancels(t *testing.T) { + sh := NewShaper(MinBandwidth) // 8 KiB/s: a parked request would wait seconds + defer sh.Stop() + + var share shaperShare + if !sh.Acquire(&share, MinShaperBurst, nil) { // drain the bucket + t.Fatal("acquire failed") + } + + cancel := make(chan struct{}) + result := make(chan bool, 1) + go func() { result <- sh.Acquire(&share, 32<<10, cancel) }() + + time.Sleep(50 * time.Millisecond) + close(cancel) + + select { + case ok := <-result: + if ok { + t.Error("Acquire returned true after cancellation") + } + case <-time.After(time.Second): + t.Fatal("Acquire did not return after its cancel channel closed") + } + + sh.mu.Lock() + n := len(sh.waiting) + sh.mu.Unlock() + if n != 0 { + t.Errorf("%d cancelled request(s) left in the queue", n) + } +} diff --git a/client/stats.go b/client/stats.go new file mode 100644 index 0000000..6c11387 --- /dev/null +++ b/client/stats.go @@ -0,0 +1,223 @@ +package client + +import ( + "fmt" + "log" + "sort" + "strings" + "sync" + "sync/atomic" + "time" +) + +// Performance diagnostics. +// +// The question an operator actually has is "why is this tunnel slow?", and +// nothing here could answer it before. A stream that is not moving bytes is +// blocked on exactly one of three things: +// +// - the flow-control window — the peer is not draining to its terminal +// socket, so the bottleneck is past the tunnel (a struggling game server, a +// player on a bad link); +// - the shaper — the configured bandwidth cap is the binding constraint, and +// raising it is the fix; +// - the peer socket itself — bytes move, but slowly, which points at the path +// rather than at either end. +// +// Those three are indistinguishable from throughput alone and call for +// completely different responses, so they are counted apart. +// +// Cost. Both structs are nil unless statsIntervalMs is set, so the default is a +// single predictable branch per event and no allocation at all. When enabled, +// counters sit under locks the code already holds; only the frame counters use +// atomics, because the read loop must never queue behind a send. A clock is read +// only when a goroutine is about to block, never per chunk — if nothing stalls, +// nothing is timed. + +// streamStats accumulates one stream's lifetime. Guarded by Stream.mu. +type streamStats struct { + opened time.Time + + bytesUp int64 // destination -> hub + bytesDown int64 // hub -> destination + + windowStall time.Duration // blocked with no send credit + shaperStall time.Duration // blocked on the bandwidth cap + qPeak int // high-water mark of the receive queue + + resumes int + hung time.Duration // total time parked awaiting a reattach + replayBytes int64 +} + +// connStats accumulates one worker conn's lifetime. +type connStats struct { + opened time.Time + framesIn atomic.Int64 + framesOut atomic.Int64 + writeErrs atomic.Int64 + + // Round-trip time of the mux heartbeat. The probe already carries a + // timestamp that the peer echoes and both sides currently throw away, so + // this measures tunnel latency for no added cost — and it is the best signal + // available for head-of-line blocking, where one stream's backlog delays + // every other stream sharing the connection. + mu sync.Mutex + rttLast time.Duration + rttMin time.Duration + rttMax time.Duration + rttSum time.Duration + rttN int64 +} + +func (cs *connStats) observeRTT(d time.Duration) { + if cs == nil || d < 0 { + return // a nonce we cannot read as one of our own timestamps + } + cs.mu.Lock() + defer cs.mu.Unlock() + cs.rttLast = d + if cs.rttN == 0 || d < cs.rttMin { + cs.rttMin = d + } + if d > cs.rttMax { + cs.rttMax = d + } + cs.rttSum += d + cs.rttN++ +} + +func (cs *connStats) rtt() (last, min, avg, max time.Duration) { + cs.mu.Lock() + defer cs.mu.Unlock() + if cs.rttN == 0 { + return 0, 0, 0, 0 + } + return cs.rttLast, cs.rttMin, cs.rttSum / time.Duration(cs.rttN), cs.rttMax +} + +// stallClock times a block without charging the path that does not block: the +// clock is read only once a wait is actually about to happen. +type stallClock struct{ start time.Time } + +func (t *stallClock) begin(on bool) { + if on && t.start.IsZero() { + t.start = time.Now() + } +} + +func (t *stallClock) elapsed() time.Duration { + if t.start.IsZero() { + return 0 + } + return time.Since(t.start) +} + +// statsLoop prints one aggregate line per interval. Never started when +// statsIntervalMs is 0, which is the default. +func (c *Client) statsLoop(stop <-chan struct{}) { + ticker := time.NewTicker(time.Duration(c.cfg.StatsIntervalMs) * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-stop: + return + case <-ticker.C: + log.Print(c.StatsLine()) + } + } +} + +// StatsLine renders the current pool and per-conn state as one greppable line. +// Exported so tests and embedders can sample it without waiting for the ticker. +func (c *Client) StatsLine() string { + conns := c.pool.snapshot() + var b strings.Builder + fmt.Fprintf(&b, "stats conns=%d", len(conns)) + + live, parked := 0, 0 + for _, wc := range conns { + wc.mu.Lock() + streams := make([]*Stream, 0, len(wc.streams)) + for _, s := range wc.streams { + streams = append(streams, s) + } + wc.mu.Unlock() + live += len(streams) + for _, s := range streams { + s.mu.Lock() + if s.parked { + parked++ + } + s.mu.Unlock() + } + + fmt.Fprintf(&b, " | conn%d streams=%d", wc.id, len(streams)) + if cs := wc.stats; cs != nil { + _, mn, avg, mx := cs.rtt() + fmt.Fprintf(&b, " frames=%d/%d rtt=%s/%s/%s", + cs.framesIn.Load(), cs.framesOut.Load(), round(mn), round(avg), round(mx)) + if n := cs.writeErrs.Load(); n > 0 { + fmt.Fprintf(&b, " writeErrs=%d", n) + } + } + } + fmt.Fprintf(&b, " | streams=%d parked=%d", live, parked) + return b.String() +} + +// logSummary reports a stream's lifetime as it closes. This is the artifact that +// answers a specific complaint after the fact, once the periodic line has +// scrolled away. +func (s *Stream) logSummary() { + s.mu.Lock() + st := s.stats + if st == nil { + s.mu.Unlock() + return + } + line := fmt.Sprintf("stream closed after %s: up=%s down=%s stalled(window=%s shaper=%s) qPeak=%s", + round(time.Since(st.opened)), bytesHuman(st.bytesUp), bytesHuman(st.bytesDown), + round(st.windowStall), round(st.shaperStall), bytesHuman(int64(st.qPeak))) + if st.resumes > 0 { + line += fmt.Sprintf(" resumes=%d hung=%s replayed=%s", + st.resumes, round(st.hung), bytesHuman(st.replayBytes)) + } + s.mu.Unlock() + log.Print(line) +} + +func (p *WorkerPool) snapshot() []*WorkerConn { + p.mu.Lock() + conns := append([]*WorkerConn(nil), p.conns...) + p.mu.Unlock() + sort.Slice(conns, func(i, j int) bool { return conns[i].id < conns[j].id }) + return conns +} + +// round trims a duration to something readable in a log line. +func round(d time.Duration) time.Duration { + switch { + case d <= 0: + return 0 + case d < time.Millisecond: + return d.Round(time.Microsecond) + case d < time.Second: + return d.Round(time.Millisecond) + default: + return d.Round(10 * time.Millisecond) + } +} + +func bytesHuman(n int64) string { + const unit = 1024 + if n < unit { + return fmt.Sprintf("%dB", n) + } + div, exp := int64(unit), 0 + for v := n / unit; v >= unit; v /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f%ciB", float64(n)/float64(div), "KMGT"[exp]) +} diff --git a/client/unacked.go b/client/unacked.go new file mode 100644 index 0000000..c95ad84 --- /dev/null +++ b/client/unacked.go @@ -0,0 +1,64 @@ +package client + +// unackedBuf holds the bytes a stream has sent but the peer has not yet +// credited — exactly the region a reattach may have to retransmit +// (PROTOCOL.md §7.5). +// +// It needs no cap of its own: credit is only granted as bytes reach the peer's +// terminal socket, so flow control already bounds the outstanding region to one +// window. That is what makes byte-exact resumption affordable at all. +// +// A read offset rather than a copy-down on every trim. Credit arrives once per +// half-window, and copying the live remainder each time would add a second +// per-byte copy to the whole send path; compacting only once the dead prefix +// dominates makes it amortized O(1). +type unackedBuf struct { + buf []byte + head int // bytes at the front already credited, awaiting reclamation + baseOff int64 // stream offset of buf[head] +} + +// length is how many bytes are still outstanding. +func (u *unackedBuf) length() int { return len(u.buf) - u.head } + +// base is the offset of the first byte still held. +func (u *unackedBuf) base() int64 { return u.baseOff } + +// end is the offset one past the last byte sent. +func (u *unackedBuf) end() int64 { return u.baseOff + int64(u.length()) } + +func (u *unackedBuf) append(p []byte) { u.buf = append(u.buf, p...) } + +// advance drops everything the peer has credited up to off. +func (u *unackedBuf) advance(off int64) { + drop := int(off - u.baseOff) + if drop <= 0 { + return + } + if n := u.length(); drop > n { + drop = n // only reachable from a peer crediting bytes it was never sent + } + u.head += drop + u.baseOff += int64(drop) + switch { + case u.head == len(u.buf): + u.buf, u.head = u.buf[:0], 0 // fully drained: restart at the front + case u.head > len(u.buf)/2: + u.buf = append(u.buf[:0], u.buf[u.head:]...) + u.head = 0 + } +} + +// from returns the outstanding bytes at and after off, or nil when off falls +// outside what is still held — which means the peer reported an offset we can no +// longer satisfy, and the stream cannot be resumed. +func (u *unackedBuf) from(off int64) []byte { + skip := off - u.baseOff + if skip < 0 || skip > int64(u.length()) { + return nil + } + return u.buf[u.head+int(skip):] +} + +// reset releases the buffer once a stream can no longer be resumed. +func (u *unackedBuf) reset() { u.buf, u.head = nil, 0 } diff --git a/client/unacked_test.go b/client/unacked_test.go new file mode 100644 index 0000000..76c2130 --- /dev/null +++ b/client/unacked_test.go @@ -0,0 +1,112 @@ +package client + +import ( + "bytes" + "testing" +) + +// The retained region is what a reattach replays from, so an off-by-one here is +// not a dropped byte but a spliced stream: the peer resumes mid-packet and the +// session dies in a way no round-trip test would attribute to this code. + +func TestUnackedTracksOffsets(t *testing.T) { + var u unackedBuf + u.append([]byte("hello")) + u.append([]byte("world")) + + if got := u.length(); got != 10 { + t.Fatalf("length = %d, want 10", got) + } + if got := u.end(); got != 10 { + t.Fatalf("end = %d, want 10", got) + } + if got := u.from(0); !bytes.Equal(got, []byte("helloworld")) { + t.Fatalf("from(0) = %q", got) + } + // A reattach replays from wherever the peer got to, which lands anywhere — + // including the middle of a chunk boundary. + if got := u.from(3); !bytes.Equal(got, []byte("loworld")) { + t.Fatalf("from(3) = %q", got) + } + if got := u.from(10); len(got) != 0 { + t.Fatalf("from(end) = %q, want empty", got) + } +} + +func TestUnackedAdvanceDropsCreditedBytes(t *testing.T) { + var u unackedBuf + u.append([]byte("abcdefghij")) + + u.advance(4) + if got := u.length(); got != 6 { + t.Fatalf("length after advance = %d, want 6", got) + } + if got := u.end(); got != 10 { + t.Fatalf("end must not move when bytes are dropped: got %d, want 10", got) + } + if got := u.from(4); !bytes.Equal(got, []byte("efghij")) { + t.Fatalf("from(4) = %q", got) + } + // Below the retained region: the peer named an offset we can no longer + // satisfy, which must be reported rather than silently clamped — replaying + // the wrong range is worse than refusing to replay. + if got := u.from(3); got != nil { + t.Fatalf("from(3) below base = %q, want nil", got) + } + if got := u.from(11); got != nil { + t.Fatalf("from(11) past end = %q, want nil", got) + } +} + +// Interleaving appends and advances is the steady-state pattern: credit arrives +// every half window while the sender keeps writing. The buffer must stay exact +// across the compaction that eventually triggers. +func TestUnackedSurvivesInterleavedAppendAndAdvance(t *testing.T) { + var u unackedBuf + var sent []byte + var acked int64 + + for i := 0; i < 200; i++ { + chunk := bytes.Repeat([]byte{byte(i)}, 97) + sent = append(sent, chunk...) + // emit's order: reclaim what has been credited so far, then retain the + // new chunk. The base therefore trails the credit that arrived since. + base := acked + u.advance(base) + u.append(chunk) + + if got, want := u.end(), int64(len(sent)); got != want { + t.Fatalf("round %d: end = %d, want %d", i, got, want) + } + if got, want := u.length(), len(sent)-int(base); got != want { + t.Fatalf("round %d: length = %d, want %d", i, got, want) + } + if got, want := u.from(base), sent[base:]; !bytes.Equal(got, want) { + t.Fatalf("round %d: retained region diverges from what was sent", i) + } + + // The peer can only ever credit bytes it has actually received. + if i%3 == 0 { + if acked += 61; acked > int64(len(sent)) { + acked = int64(len(sent)) + } + } + } +} + +// Compaction reuses the backing array, so a stream that runs for hours must not +// grow one: this is a full window per stream, on both sides. +func TestUnackedReclaimsBackingArray(t *testing.T) { + var u unackedBuf + chunk := bytes.Repeat([]byte{7}, 4096) + for i := 0; i < 500; i++ { + u.advance(u.end()) // fully credited every round + u.append(chunk) + } + if u.length() != len(chunk) { + t.Fatalf("length = %d, want %d", u.length(), len(chunk)) + } + if cap(u.buf) > 8*len(chunk) { + t.Fatalf("backing array grew to %d bytes for a %d-byte window", cap(u.buf), len(chunk)) + } +} diff --git a/client/worker.go b/client/worker.go index 1464d8a..c183f3f 100644 --- a/client/worker.go +++ b/client/worker.go @@ -17,16 +17,24 @@ import ( // exactly how a whole server's worth of players used to drop at once. const StreamsBeforeGrowing = 1 +// allocateAttempts bounds how many times a caller retries Allocate when the +// conn it was handed dies before the stream could be registered on it. The race +// is narrow and each retry picks a different conn, so a small bound is enough; +// an unbounded loop would spin against a hub that is refusing every connection. +const allocateAttempts = 3 + // WorkerPool manages up to maxConn worker connections and allocates streams // using the least-loaded strategy (PROTOCOL.md §7.1). type WorkerPool struct { client *Client maxConn int + connSeq atomic.Int64 // conn ids, for log correlation + mu sync.Mutex cond *sync.Cond conns []*WorkerConn - dialing int // dials currently in flight (foreground + background) + dialing int // dials currently in flight (foreground + background) dialGen uint64 dialErr error // most recent dial failure } @@ -147,11 +155,16 @@ func (p *WorkerPool) dialWorker() (*WorkerConn, error) { pool: p, fc: sess.fc, sendWndInit: sess.peerWnd, - recvWndInit: p.client.streamWnd, + resume: sess.resume, + grace: p.client.resumeGrace(sess.hubGrace), + id: int(p.connSeq.Add(1)), streams: make(map[int]*Stream), nextSid: 1, done: make(chan struct{}), } + if p.client.statsOn() { + wc.stats = &connStats{opened: time.Now()} + } wc.lastPong.Store(time.Now().UnixMilli()) go wc.readLoop() if sess.heartbeat { @@ -160,8 +173,8 @@ func (p *WorkerPool) dialWorker() (*WorkerConn, error) { log.Printf("worker conn: hub does not support the mux heartbeat; " + "a silently dropped path will only be caught by TCP keepalive") } - log.Printf("opened worker conn (send window %d, recv window %d, heartbeat %v)", - wc.sendWndInit, wc.recvWndInit, sess.heartbeat) + log.Printf("opened worker conn (send window %d, recv window %d, heartbeat %v, resume %v)", + wc.sendWndInit, p.client.streamWnd, sess.heartbeat, wc.resume) return wc, nil } @@ -195,19 +208,34 @@ func (p *WorkerPool) closeAll() { } // WorkerConn is one multiplexed worker connection to the hub. +// +// Only genuinely per-connection state lives here. Client-wide values (the +// shaper, the advertised receive window, the DATA chunk cap) belong to Client: +// reading them through a connection pointer would make every such read a +// re-parenting hazard once a stream can migrate between conns. type WorkerConn struct { pool *WorkerPool fc *wire.FramedConn sendWndInit int // hub's advertised per-stream receive window (our send budget) - recvWndInit int // our advertised per-stream receive window (bounds each recv queue) + + // resume is whether this conn negotiated stream resumption, and grace how + // long a stream parked from it may keep trying to reattach. Both are + // per-conn: a reattach may land on a different (or restarted) hub, so the + // flag must be re-checked on the conn that will carry the RESUME. + resume bool + grace time.Duration done chan struct{} // closed when readLoop exits lastPong atomic.Int64 // unix ms of the most recent PONG + id int // for log correlation only + stats *connStats // nil unless diagnostics are enabled + mu sync.Mutex streams map[int]*Stream nextSid int + closed bool // readLoop has exited; registerStream must refuse } // heartbeatLoop proves the worker conn is still carrying frames end to end. TCP @@ -251,10 +279,23 @@ func (wc *WorkerConn) newSid() int { return sid } -func (wc *WorkerConn) registerStream(sid int, st *Stream) { +// registerStream publishes a stream in the conn's table, or reports false if +// the conn has already died. +// +// The check is not advisory. Allocate hands out a (conn, sid) pair under the +// pool lock, and the conn's readLoop can exit before the caller gets here — it +// has then already swapped the stream map, so a blind insert would land in a map +// nothing iterates and the stream would never be torn down. That normally hides +// behind a failing SYN, but not on a half-open conn whose readLoop died on a +// framing error while the socket is still writable. Callers must re-Allocate. +func (wc *WorkerConn) registerStream(sid int, st *Stream) bool { wc.mu.Lock() + defer wc.mu.Unlock() + if wc.closed { + return false + } wc.streams[sid] = st - wc.mu.Unlock() + return true } func (wc *WorkerConn) getStream(sid int) *Stream { @@ -280,6 +321,9 @@ func (wc *WorkerConn) readLoop() { if err != nil { break } + if wc.stats != nil { + wc.stats.framesIn.Add(1) + } r := wire.NewReader(payload) ftype, err := r.U8() if err != nil { @@ -306,14 +350,37 @@ func (wc *WorkerConn) readLoop() { st.gracefulFin() } case MuxRst: + // The reason is an optional trailing byte; older peers send none. + reason := RstUnspecified + if b, err := r.U8(); err == nil { + reason = int(b) + } if st := wc.removeStream(sid); st != nil { - st.teardown(false) + st.onRst(reason) + } + case MuxResumeAck: + accepted, aerr := r.I64() + delivered, derr := r.I64() + cid, cerr := r.Bytes(CIDLen) + if aerr != nil || derr != nil || cerr != nil { + continue + } + if st := wc.getStream(sid); st != nil { + st.deliverResume(resumeResult{accepted: accepted, delivered: delivered, cid: cid}) } case MuxPing: nonce, _ := r.I64() _ = wc.fc.WriteFrame(wire.NewWriter().U8(MuxPong).VarInt(MuxCtlSid).I64(nonce).Out()) case MuxPong: - wc.lastPong.Store(time.Now().UnixMilli()) + now := time.Now() + wc.lastPong.Store(now.UnixMilli()) + // The probe's nonce is the timestamp we sent, echoed back, so the + // round trip is free to measure and nobody was reading it. + if wc.stats != nil { + if sent, err := r.I64(); err == nil { + wc.stats.observeRTT(now.Sub(time.UnixMilli(sent))) + } + } default: log.Printf("worker: unknown mux type %d", ftype) } @@ -322,14 +389,22 @@ func (wc *WorkerConn) readLoop() { close(wc.done) wc.pool.remove(wc) wc.mu.Lock() + // Marked before the map is swapped, under the same lock, so a concurrent + // registerStream either lands in the map we are about to drain or is refused. + wc.closed = true streams := make([]*Stream, 0, len(wc.streams)) for _, st := range wc.streams { streams = append(streams, st) } wc.streams = make(map[int]*Stream) wc.mu.Unlock() + // Only the tunnel leg died. Where the session negotiated resumption the + // destination sockets are kept open and each stream reattaches over a fresh + // conn (§7.5); otherwise this is the old, unconditional teardown. for _, st := range streams { - st.teardown(false) + if !st.park(wc.grace) { + st.teardown(false) + } } } @@ -338,7 +413,14 @@ func (wc *WorkerConn) sendSyn(sid int, cid []byte) error { } func (wc *WorkerConn) sendData(sid int, data []byte) error { - return wc.fc.WriteFrame(wire.NewWriter().U8(MuxData).VarInt(sid).Bytes(data).Out()) + err := wc.fc.WriteFrame(wire.NewWriter().U8(MuxData).VarInt(sid).Bytes(data).Out()) + if wc.stats != nil { + wc.stats.framesOut.Add(1) + if err != nil { + wc.stats.writeErrs.Add(1) + } + } + return err } func (wc *WorkerConn) sendFin(sid int) { @@ -353,6 +435,16 @@ func (wc *WorkerConn) sendWndUpdate(sid, delta int) { _ = wc.fc.WriteFrame(wire.NewWriter().U8(MuxWnd).VarInt(sid).VarInt(delta).Out()) } +// leg binds a stream to one worker conn. The two fields are only meaningful +// together: stream ids are per-conn and restart at 1, so conn A's sid 3 and +// conn B's sid 3 belong to different players. A torn read across the two would +// credit, reset or FIN a stranger's stream, so the pair is swapped as one +// immutable value rather than as two fields. +type leg struct { + wc *WorkerConn + sid int +} + // Stream bridges one player (via the hub) to one destination connection. // // Data from the hub is queued and written to the destination by a dedicated @@ -360,24 +452,74 @@ func (wc *WorkerConn) sendWndUpdate(sid, delta int) { // the hub never sends more un-credited bytes, so overflow is a protocol // violation and resets the stream. type Stream struct { - wc *WorkerConn - sid int - cid []byte + client *Client + leg atomic.Pointer[leg] mapping Mapping srcIP string srcPort int vel *velocityForwarder // non-nil when the mapping sets velocitySecret + share shaperShare // this stream's position in the egress fair queue + done chan struct{} // closed on teardown; unparks a shaper wait + + // resumable is fixed at creation from the conn's negotiated flag. With it + // false none of the bookkeeping below runs and no buffer is ever allocated, + // so a client with resumption disabled pays exactly what it used to. + resumable bool + + stats *streamStats // nil unless diagnostics are enabled + + // ackedOffset is the running sum of WND deltas received. Credit is granted + // only as bytes reach the peer's terminal socket, so it is a sound lower + // bound on what has been delivered. Atomic because the worker readLoop + // advances it and must never block behind the send path. + ackedOffset atomic.Int64 + + // sendMu serializes the send path — buffer the chunk, advance the offset, + // write the frame — against a reattach's retransmit, so replayed bytes can + // never interleave with live ones. + // + // Deliberately not s.mu: deliverFromHub takes s.mu from the worker readLoop, + // and holding s.mu across a WriteFrame would stall frame dispatch for every + // other stream on the connection. + sendMu sync.Mutex + un unackedBuf // guarded by sendMu + mu sync.Mutex cond *sync.Cond + cid []byte // takeover capability; re-minted by the hub on each resume dest net.Conn connected bool closed bool - finPending bool // hub sent FIN; close the destination once the queue drains - q []qentry // hub/local -> destination, waiting for writeLoop - qBytes int // hub bytes only: bounds the peer against its window - sendWnd int // flow control: budget for destination -> hub DATA - consumed int // flow control: drained bytes not yet credited back to the hub + parked bool // worker conn died; awaiting reattach on a fresh one + parkedAt time.Time // when the current hang began; diagnostics only + finPending bool // hub sent FIN; close the destination once the queue drains + finToHub bool // destination closed while parked; FIN owed once reattached + q []qentry // hub/local -> destination, waiting for writeLoop + qBytes int // hub bytes only: bounds the peer against its window + // acceptedOffset counts hub bytes enqueued toward the destination. This, not + // "bytes written", is what a reattach reports: acceptance is synchronous and + // stable at park time, whereas delivery is signalled asynchronously and goes + // silent exactly when the connection dies — which would under-report and make + // the hub retransmit bytes the player already has. + acceptedOffset int64 + // deliveredOffset counts hub bytes actually written to the destination. + // Distinct from acceptedOffset and needed for a different job: a reattach + // replays from what the peer *accepted*, but sizes the window from what it + // *delivered*, because the window is a promise about undelivered bytes. + deliveredOffset int64 + sendWnd int // flow control: budget for destination -> hub DATA + consumed int // flow control: drained bytes not yet credited back to the hub + resumeWait chan resumeResult +} + +// resumeResult is the hub's answer to a RESUME: how far it got in both senses, +// plus a fresh CID — or the reason the reattach was refused. +type resumeResult struct { + accepted int64 + delivered int64 + cid []byte + err error } // qentry is one queued write towards the destination. Only hub-originated @@ -389,8 +531,13 @@ type qentry struct { fromHub bool } -func newStream(wc *WorkerConn, sid int, cid []byte, m Mapping, ip string, port int) *Stream { - s := &Stream{wc: wc, sid: sid, cid: cid, mapping: m, srcIP: ip, srcPort: port, sendWnd: wc.sendWndInit} +func newStream(c *Client, wc *WorkerConn, sid int, cid []byte, m Mapping, ip string, port int) *Stream { + s := &Stream{client: c, cid: cid, mapping: m, srcIP: ip, srcPort: port, + resumable: wc.resume, sendWnd: wc.sendWndInit, done: make(chan struct{})} + s.leg.Store(&leg{wc: wc, sid: sid}) + if c.statsOn() { + s.stats = &streamStats{opened: time.Now()} + } if m.VelocitySecret != "" { s.vel = newVelocityForwarder(m.VelocitySecret, ip) } @@ -398,14 +545,20 @@ func newStream(wc *WorkerConn, sid int, cid []byte, m Mapping, ip string, port i return s } +// conn returns the stream's current binding. Every caller must take exactly one +// snapshot and use both fields from it; re-loading mid-operation reintroduces +// the torn-pair hazard the leg exists to prevent. +func (s *Stream) conn() *leg { return s.leg.Load() } + // run dials the destination, optionally writes the PROXY v2 header, then pumps // destination -> hub (respecting the stream send window when negotiated). func (s *Stream) run() { dest, err := net.DialTimeout("tcp", s.mapping.Destination, 10*time.Second) if err != nil { - log.Printf("stream %d: dial %s failed: %v", s.sid, s.mapping.Destination, err) - s.wc.removeStream(s.sid) - s.wc.sendRst(s.sid) + lg := s.conn() + log.Printf("stream %d: dial %s failed: %v", lg.sid, s.mapping.Destination, err) + lg.wc.removeStream(lg.sid) + lg.wc.sendRst(lg.sid) s.teardown(false) return } @@ -416,7 +569,7 @@ func (s *Stream) run() { if s.mapping.ProxyProtocol { if hdr := s.buildProxyHeader(dest); hdr != nil { if _, err := dest.Write(hdr); err != nil { - log.Printf("stream %d: proxy header write: %v", s.sid, err) + log.Printf("stream %d: proxy header write: %v", s.conn().sid, err) } } } @@ -462,26 +615,88 @@ func (s *Stream) run() { s.teardown(true) } -// sendToHub forwards destination bytes to the hub in DATA frames of at most -// DataChunkSize, honoring the stream send window. Returns false once the -// stream closed or the worker conn failed. +// sendToHub forwards destination bytes to the hub in bounded DATA frames, +// honoring both the stream send window and the client-wide bandwidth cap. +// Returns false once the stream closed or the worker conn failed. func (s *Stream) sendToHub(data []byte) bool { for len(data) > 0 { n := len(data) - if n > DataChunkSize { - n = DataChunkSize + if n > s.client.chunk { + n = s.client.chunk } + // Credit first, bandwidth second. The reverse order would charge the + // budget for bytes still parked on an exhausted window, so the client + // would throttle itself below the configured rate. Holding credit while + // waiting for tokens is free — credit is per-stream, and the hub returns + // it as it drains data to the player, independent of our pacing. if !s.acquireSendWnd(n) { return false } - if err := s.wc.sendData(s.sid, data[:n]); err != nil { + var shaperStall stallClock + shaperStall.begin(s.stats != nil) + if !s.client.shaper.Acquire(&s.share, n, s.done) { return false } + if !s.emit(data[:n]) { + return false + } + if s.stats != nil { + s.mu.Lock() + // Separated from the window stall on purpose: this one says the + // configured cap is the binding constraint, and raising maxBandwidth + // is the fix. The window stall says the opposite. + s.stats.shaperStall += shaperStall.elapsed() + s.stats.bytesUp += int64(n) + s.mu.Unlock() + } data = data[n:] } return true } +// emit records a chunk for possible retransmission and writes it to the current +// worker conn. Returns false once the stream is finished with. +// +// The record is taken first and unconditionally. A write that fails on a dying +// connection has already spent window and may have put part of the frame on the +// wire, so the only trustworthy account of what the peer still owes us is the +// one taken before the attempt. That also makes a failure survivable: while the +// stream can still be resumed the bytes are already safe, and the reattach +// replays them from wherever the hub says it got to. +// +// Note the old code let a failed write drop the rest of the chunk on the floor — +// the caller's slice advance sat after the error return. +func (s *Stream) emit(chunk []byte) bool { + s.sendMu.Lock() + if s.resumable { + // Reclaim what the hub has credited before growing the buffer, so the + // outstanding region stays bounded by one window. + s.un.advance(s.ackedOffset.Load()) + s.un.append(chunk) + } + lg := s.conn() + err := lg.wc.sendData(lg.sid, chunk) + s.sendMu.Unlock() + + if err == nil { + return true + } + if !s.resumable { + return false + } + // The conn is gone but the stream is not: readLoop parks it and a reattach + // replays the buffer. Keep pumping the destination — acquireSendWnd stops us + // once a full window is outstanding, so nothing is lost and nothing grows + // without bound. + return !s.isClosed() +} + +func (s *Stream) isClosed() bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.closed +} + // writeLoop is the only writer to the destination. It drains the receive queue, // credits the hub as bytes land on the destination socket, and performs the // deferred graceful close when a FIN arrived with data still queued. @@ -513,6 +728,12 @@ func (s *Stream) writeLoop() { return } if e.fromHub { + s.mu.Lock() + s.deliveredOffset += int64(len(e.data)) + if s.stats != nil { + s.stats.bytesDown += int64(len(e.data)) + } + s.mu.Unlock() s.credit(len(e.data)) } } @@ -530,16 +751,26 @@ func (s *Stream) deliverFromHub(data []byte) { s.mu.Unlock() return } - if s.qBytes+len(data) > s.wc.recvWndInit { + if s.qBytes+len(data) > s.client.streamWnd { s.mu.Unlock() - log.Printf("stream %d: peer exceeded flow-control window; resetting", s.sid) - s.wc.removeStream(s.sid) - s.wc.sendRst(s.sid) + lg := s.conn() + log.Printf("stream %d: peer exceeded flow-control window; resetting", lg.sid) + lg.wc.removeStream(lg.sid) + lg.wc.sendRst(lg.sid) s.teardown(false) return } s.q = append(s.q, qentry{data: data, fromHub: true}) s.qBytes += len(data) + // Accepted, not delivered: from here the bytes are ours to write, and the + // only way we fail to is by destroying the stream — which also ends any + // prospect of resuming it. That makes this a sound reattach coordinate. + s.acceptedOffset += int64(len(data)) + if s.stats != nil && s.qBytes > s.stats.qPeak { + // How close the receive queue came to the advertised window: near it + // means the destination is the slow party. + s.stats.qPeak = s.qBytes + } s.cond.Broadcast() s.mu.Unlock() } @@ -562,9 +793,18 @@ func (s *Stream) injectToDest(data []byte) { func (s *Stream) acquireSendWnd(n int) bool { s.mu.Lock() defer s.mu.Unlock() + // Timed only when it actually blocks, so a stream that never runs out of + // credit never reads the clock. A large windowStall is the signal that the + // peer is not draining to its terminal socket — the bottleneck is past the + // tunnel, not in it. + var stall stallClock for !s.closed && s.sendWnd < n { + stall.begin(s.stats != nil) s.cond.Wait() } + if s.stats != nil { + s.stats.windowStall += stall.elapsed() + } if s.closed { return false } @@ -573,6 +813,11 @@ func (s *Stream) acquireSendWnd(n int) bool { } func (s *Stream) grantSendWnd(delta int) { + // The running sum doubles as the acked offset: the hub grants credit exactly + // as bytes reach the player socket, so a byte that has been credited can + // never need retransmitting. Advanced without a lock so the worker readLoop + // never blocks behind a send in progress. + s.ackedOffset.Add(int64(delta)) s.mu.Lock() s.sendWnd += delta s.cond.Broadcast() @@ -581,17 +826,22 @@ func (s *Stream) grantSendWnd(delta int) { // credit accounts bytes drained to the destination and grants the hub more // window once half of our receive window has been consumed. +// While parked the grant is only withheld, never dropped: consumed keeps +// accumulating and a reattach flushes it on the new conn. Resetting it would +// destroy up to half a window of credit per outage, and after a few flaps the +// stream would throttle to a crawl. func (s *Stream) credit(n int) { s.mu.Lock() s.consumed += n - if s.closed || s.consumed*2 < s.wc.recvWndInit { + if s.closed || s.parked || s.consumed*2 < s.client.streamWnd { s.mu.Unlock() return } delta := s.consumed s.consumed = 0 s.mu.Unlock() - s.wc.sendWndUpdate(s.sid, delta) + lg := s.conn() + lg.wc.sendWndUpdate(lg.sid, delta) } func (s *Stream) buildProxyHeader(dest net.Conn) []byte { @@ -629,6 +879,13 @@ func (s *Stream) gracefulFin() { // teardown closes the stream immediately; notifyHub sends a FIN when true. // Idempotent; wakes every goroutine parked on the stream. func (s *Stream) teardown(notifyHub bool) { + // A parked stream owes the hub a FIN it cannot send: the only conn it has is + // the one that just failed. Keep it alive so the reattach can deliver it and + // the player gets a clean disconnect, rather than hanging until the hub's + // grace expires. + if notifyHub && s.noteFinWhileParked() { + return + } s.mu.Lock() if s.closed { s.mu.Unlock() @@ -636,14 +893,23 @@ func (s *Stream) teardown(notifyHub bool) { } s.closed = true dest := s.dest + close(s.done) // guarded by the idempotence check above, so exactly once s.cond.Broadcast() s.mu.Unlock() + // The stream can no longer be resumed, so the retransmit buffer is dead + // weight — up to a full window of it per stream. + s.sendMu.Lock() + s.un.reset() + s.sendMu.Unlock() + if dest != nil { _ = dest.Close() } - s.wc.removeStream(s.sid) + lg := s.conn() + lg.wc.removeStream(lg.sid) if notifyHub { - s.wc.sendFin(s.sid) + lg.wc.sendFin(lg.sid) } + s.logSummary() } diff --git a/docs/architecture.md b/docs/architecture.md index 6ab96a9..7cbce6d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -114,21 +114,34 @@ per-connection counter with no coordination. ### 3.1 Pool & allocation The client keeps 1…`maxConn` worker connections and places each new stream on -the **least-loaded** one. It opens an additional connection only when the -least-loaded connection is *saturated* (more than 8 active streams) and the pool -is below `maxConn`: +the **least-loaded** one. The pool grows **breadth-first**: it dials out to +`maxConn` before stacking streams, so that no single TCP connection ever becomes +the shared point of failure for every player on the tunnel (PROTOCOL.md §7.1): ``` -pick least-loaded conn -if leastLoaded.streams > 8 and pool.size < maxConn: - dial a new worker conn and use it -else: - use leastLoaded +pick least-loaded conn; use it +if leastLoaded.streams >= 1 and pool.size + dialsInFlight < maxConn: + dial another worker conn in the background # the stream just placed does not wait ``` +The new connection becomes the least-loaded one and picks up subsequent streams. +Once the pool is at `maxConn`, streams stack on the least-loaded connection; +going past 8 active streams there is logged as pool saturation but is not an +error. + +Two properties of the dialing path matter as much as the placement rule: + +* A dial is **never performed while holding the pool lock** — session + establishment is network I/O, and one unresponsive hub must not park every + other player behind it. +* Only when the pool is *empty* does a caller dial synchronously, and then + exactly one caller dials while the others wait on its result, so a burst of + arrivals cannot open a burst of redundant connections. + The e2e test `TestConcurrentStreamsUseMultipleConns` drives 20 simultaneous -streams with `maxConn=4` and observes them deterministically spread over 3 -connections (9 + 9 + 2), confirming the algorithm. +streams with `maxConn=4` and confirms they spread over more than one connection +without exceeding the cap; `TestAllocateDoesNotWedgePoolOnStalledHub` covers the +stalled-dial path. ## 4. Encryption @@ -162,7 +175,7 @@ connections (9 + 9 + 2), confirming the algorithm. ## 6. Back-pressure & flow control -Two mechanisms operate at different granularities: +Three mechanisms operate at different granularities: * **Per-stream credit windows** (PROTOCOL.md §7.3; the windows are exchanged at session establishment): each stream direction has an independent byte @@ -178,10 +191,44 @@ Two mechanisms operate at different granularities: socket itself is congested (total bandwidth, not one stream), the hub parks all sending players until it drains, and the client's `WriteFrame` blocks. This is fair — when the pipe is genuinely full, everyone should slow down. +* **Client egress shaping** (optional, `maxBandwidth`; `client/shaper.go`): a + rate cap on everything the client sends to the hub, across all worker conns. + +The first two mechanisms have no time dimension. A credit window bounds how many +bytes are *in flight*, and TCP back-pressure only reacts once the pipe is already +full — which on a residential uplink is too late. One player loading chunks fills +the line, the standing queue grows to seconds, and every other player's keepalive +times out. Nothing in §7.3 prevents that: each stream is individually +well-behaved, and collectively they still overrun the link. + +The shaper closes that gap with a token bucket for the rate and start-time fair +queueing for the split. A global virtual clock advances with each grant; every +stream remembers where its last request finished, and a new request is stamped +`max(stream.vfinish, vclock)`. Lowest stamp wins. A stream that keeps sending +pushes its own stamp further out and yields; a stream returning from idle is +clamped back to the clock, so it cannot bank credit for time it did not use, but +is not penalised for the idleness either. A stream sending a few hundred bytes +gets a nearer stamp than one sending a full chunk, so keepalives and chat overtake +bulk terrain data for free. One stream alone still gets the entire rate. + +Two details keep bursts cheap. The bucket banks 200 ms of transmission, so a +player joining spends it at once instead of paying for the cap in visible +chunk-loading latency. And the DATA chunk shrinks to ~20 ms of transmission when +the rate is low (floor 4 KiB), because a fixed 32 KiB chunk is a 256 ms slot at +1 Mbps — long enough dead air to drag the other players towards the very timeout +the cap exists to prevent. Above ~13 Mbps the chunk stays at the usual 32 KiB. + +This is entirely client-local: nothing about it appears on the wire, and the hub +is unaware. Only DATA is shaped — delaying a `FIN`, `WND` or `PONG` would cause +the false-death detection §7.4 exists to avoid. The window also bounds memory: a stream can hold at most one window of undelivered data per direction (the client's pre-connect handshake buffer is -covered by the same bound). +covered by the same bound). With stream resumption enabled (§7) the *sender* +holds a second window — the bytes it has sent but the peer has not yet credited, +kept so they can be retransmitted after an outage. That is not a new bound so +much as the existing one made symmetric: the region is exactly what flow control +already declared outstanding, which is why resumption needs no cap of its own. Per-stream flow control is mandatory: the hub rejects a session whose Rekey lacks the STREAM_FC flag, and the client rejects a hub that does not echo it — @@ -194,17 +241,41 @@ transport (QUIC) would be the escape hatch if it ever matters. ## 7. Failure & recovery -* **Control session drop:** the client reconnects with capped exponential - backoff and re-registers all patterns. Existing worker connections and their - live streams are unaffected. -* **Worker connection drop:** every stream on it is torn down (destinations - closed); the hub closes the corresponding player sockets; the client removes - the connection from the pool and will dial a fresh one on the next allocation. +* **Control session drop:** the client retries immediately, then backs off to a + 10s cap, and re-registers all patterns. Existing worker connections and their + live streams are unaffected — they ride worker conns, which a control-session + close never touches. The hub meanwhile keeps that session's routes as + *orphaned* for `registrationGraceMs` (PROTOCOL.md §5.2) and **holds** players + arriving on them instead of refusing them, replaying the control request once + a client re-registers the pattern. Without that, the reconnect window is one + in which every new player is told there is no such server. +* **Worker connection drop:** the connection leaves the pool either way. What + happens to its streams depends on whether STREAM_RESUME was negotiated: + * *without it* — every stream is torn down (destinations closed) and the hub + closes the corresponding player sockets, as it always did; + * *with it* — the streams are **hung** instead (PROTOCOL.md §7.5). The + destination sockets stay open, the hub pauses the player sockets and holds + them for its grace period, and the client reattaches each stream over a + freshly dialed connection, replaying byte-exactly from the offset the peer + reports. Players see a stall rather than a disconnect. * **Pending timeout:** if no worker takes over a matched player within `pendingTimeoutMs`, the hub drops the pending entry and closes the player. * **Bad PSK / bad timestamp / bad magic:** the hub closes the TCP connection; the client's session establishment fails fast. +Resumption is worth the machinery because a worker connection is only the +*middle* leg of every stream it carries. When it dies both terminal sockets are +usually still healthy, so the old behaviour discarded working connections +because a replaceable transport failed — one conntrack expiry disconnected every +player sharing that connection. It also has to be byte-exact rather than +best-effort: bytes handed to a dying socket are lost with no notification and the +frame cipher cannot be resynchronized, so an approximate reattach would splice +the tunneled protocol mid-packet, which is worse than a clean close. + +Notably, resumption does not depend on the control session. A blip usually kills +both, and the reattach path needs only a worker connection, so recovery does not +wait on the control reconnect backoff. + ## 8. Known limitations 1. No AEAD — payload integrity/authenticity is not cryptographically guaranteed. diff --git a/e2e/bandwidth_test.go b/e2e/bandwidth_test.go new file mode 100644 index 0000000..4678b6b --- /dev/null +++ b/e2e/bandwidth_test.go @@ -0,0 +1,147 @@ +package e2e + +import ( + "context" + "fmt" + "io" + "net" + "testing" + "time" + + "github.com/iceBear67/redapricot/client" +) + +// startClientWithBandwidth is startClient with an egress cap, for the shaping +// tests. Only the client→hub direction is shaped, which in this harness is the +// leg carrying the mock destination's echo back to the player. +func startClientWithBandwidth(t *testing.T, hubAddr, psk string, maxConn int, bandwidth string, mappings []client.Mapping) *client.Client { + t.Helper() + cfg := &client.Config{ + Server: hubAddr, + PSK: psk, + MaxConn: maxConn, + PingIntervalMs: 20000, + MaxBandwidth: bandwidth, + Mappings: mappings, + } + c := client.New(cfg) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(func() { + cancel() + c.Close() + }) + if err := c.Start(ctx); err != nil { + t.Fatalf("client start: %v", err) + } + time.Sleep(200 * time.Millisecond) + return c +} + +// drain reads a connection until it fails, so a greedy player keeps pulling +// bytes instead of stalling on its own receive window. +func drain(conn net.Conn) { + go func() { _, _ = io.Copy(io.Discard, conn) }() +} + +// TestBandwidthCapIsEnforced pushes a payload that cannot fit in the burst and +// asserts the transfer takes at least as long as the configured rate implies. +// This is the first timing assertion in the suite, so the bounds are wide: the +// unshaped path completes this in well under a second, making the lower bound +// an unambiguous signal rather than a tight measurement. +func TestBandwidthCapIsEnforced(t *testing.T) { + const psk = "e2e-bwcap" + port := freePort(t) + hubAddr := fmt.Sprintf("127.0.0.1:%d", port) + startHub(t, port, psk) + dest := newMockDest(t, modeEcho) + startClientWithBandwidth(t, hubAddr, psk, 1, "1MB/s", []client.Mapping{ + {Pattern: "mc.local", Destination: dest.addr}, + }) + + pc := dialPlayer(t, hubAddr, "mc.local") + defer pc.Close() + + payload := make([]byte, 2*1024*1024) + for i := range payload { + payload[i] = byte(i*31 + 7) + } + + start := time.Now() + writeErr := make(chan error, 1) + go func() { + _, err := pc.Write(payload) + writeErr <- err + }() + got := make([]byte, len(payload)) + _ = pc.SetReadDeadline(time.Now().Add(60 * time.Second)) + if _, err := io.ReadFull(pc, got); err != nil { + t.Fatalf("read echo: %v", err) + } + elapsed := time.Since(start) + if err := <-writeErr; err != nil { + t.Fatalf("write payload: %v", err) + } + + // 2 MiB at 1 MiB/s, minus the 200 KiB the bucket has banked, is ~1.8 s. + const floor = 1200 * time.Millisecond + if elapsed < floor { + t.Errorf("2 MiB echoed back in %v under a 1MB/s cap; expected at least %v, so the cap is not taking effect", elapsed, floor) + } + if elapsed > 30*time.Second { + t.Errorf("transfer took %v, far beyond the ~1.8s the rate implies", elapsed) + } +} + +// TestCappedBandwidthDoesNotStarveLightStreams is the point of the fair queue: +// with the link deliberately capped and three players saturating it, a fourth +// player exchanging small messages must keep round-tripping promptly. A plain +// FIFO token bucket would leave it queued behind the heavy players' backlog, +// which for a real Minecraft client means a keepalive timeout — the exact +// failure this feature exists to prevent. +func TestCappedBandwidthDoesNotStarveLightStreams(t *testing.T) { + const psk = "e2e-bwfair" + port := freePort(t) + hubAddr := fmt.Sprintf("127.0.0.1:%d", port) + startHub(t, port, psk) + dest := newMockDest(t, modeEcho) + // maxConn=1 forces every stream onto one worker conn, so nothing but the + // shaper is deciding who gets the link. + startClientWithBandwidth(t, hubAddr, psk, 1, "1MB/s", []client.Mapping{ + {Pattern: "mc.local", Destination: dest.addr}, + }) + + // Three greedy players: each writes megabytes and keeps draining the echo, + // so they compete for the cap for the whole test rather than parking on + // their own flow-control windows. + for i := 0; i < 3; i++ { + heavy := dialPlayer(t, hubAddr, "mc.local") + defer heavy.Close() + drain(heavy) + go func() { _, _ = heavy.Write(make([]byte, 8*1024*1024)) }() + } + time.Sleep(500 * time.Millisecond) // let them saturate the shaper + + light := dialPlayer(t, hubAddr, "mc.local") + defer light.Close() + payload := make([]byte, 4*1024) + for i := range payload { + payload[i] = byte(i*13 + 5) + } + + var worst time.Duration + for i := 0; i < 10; i++ { + start := time.Now() + playerEcho(t, light, payload) + if d := time.Since(start); d > worst { + worst = d + } + } + + // Fair sharing puts a 4 KiB round at roughly 4 KiB / (1 MiB/s ÷ 4) ≈ 16 ms of + // link time. The bound is two orders of magnitude looser so only genuine + // starvation trips it. + const limit = 3 * time.Second + if worst > limit { + t.Errorf("slowest small round-trip took %v (limit %v); heavy streams are crowding out the light one", worst, limit) + } +} diff --git a/e2e/control_outage_test.go b/e2e/control_outage_test.go new file mode 100644 index 0000000..6a1c32f --- /dev/null +++ b/e2e/control_outage_test.go @@ -0,0 +1,144 @@ +package e2e + +import ( + "errors" + "fmt" + "io" + "net" + "testing" + "time" + + "github.com/iceBear67/redapricot/client" + "github.com/iceBear67/redapricot/client/wire" +) + +// A control session dying takes the client's routes with it. Players already +// tunneled are unaffected — they ride worker conns — but anyone *arriving* +// during the reconnect used to be told there is no such server, even though the +// tunnel was a second from being back. +// +// The hub now keeps those routes as orphaned for its registration grace and +// hangs arriving players on them, replaying the control request it never sent +// once a client re-registers the pattern. + +// outageRelay starts a hub, an echoing destination, and a client reaching the +// hub only through a relay, so the tunnel can be cut without touching the +// players — who connect to the hub directly, as they would from the internet. +func outageRelay(t *testing.T, psk string, hubCfg map[string]any) (hubAddr string, relay *blackholeRelay) { + t.Helper() + hubPort := freePort(t) + hubAddr = fmt.Sprintf("127.0.0.1:%d", hubPort) + startHubCfg(t, hubPort, psk, hubCfg) + dest := newMockDest(t, modeEcho) + + relay = newBlackholeRelay(t, hubAddr) + startClientWithPing(t, relay.addr, psk, 1, 400, []client.Mapping{ + {Pattern: "mc.local", Destination: dest.addr}, + }) + return hubAddr, relay +} + +// TestControlOutageHangsArrivingPlayer is the point of the feature: a player +// that shows up while the client is reconnecting gets held and then served, +// rather than refused. +func TestControlOutageHangsArrivingPlayer(t *testing.T) { + const psk = "e2e-ctl-hang" + hubAddr, relay := outageRelay(t, psk, nil) + + // Cut the tunnel and keep it cut, so the client cannot re-register. + relay.stop() + relay.dropAll() + // Let the hub see the close and orphan the route before the player arrives. + time.Sleep(500 * time.Millisecond) + + pc := resumePlayer(t, hubAddr, "mc.local") + if _, err := pc.Write([]byte("held")); err != nil { + t.Fatalf("player write during outage: %v", err) + } + + // Nothing can come back yet — but the socket must still be open. Before this + // change the hub had already closed it. + _ = pc.SetReadDeadline(time.Now().Add(700 * time.Millisecond)) + if _, err := pc.Read(make([]byte, 1)); err == nil { + t.Fatal("player was served while the route was orphaned") + } else if !isTimeout(err) { + t.Fatalf("player was dropped during the control outage instead of being held: %v", err) + } + + relay.restore() + + // The client reconnects, re-registers, and the hub replays the request it + // held — so the bytes written during the outage arrive at the destination and + // echo back on the same connection. + echo := make([]byte, 4) + _ = pc.SetReadDeadline(time.Now().Add(30 * time.Second)) + if _, err := io.ReadFull(pc, echo); err != nil { + t.Fatalf("held player never served after the route came back: %v", err) + } + if string(echo) != "held" { + t.Fatalf("echo = %q, want %q", echo, "held") + } +} + +// TestControlOutageGraceDisabledClosesPlayer pins the off switch: with the grace +// at zero the hub must drop the route the instant its session closes, exactly as +// it did before, rather than hanging players for a client that may never return. +func TestControlOutageGraceDisabledClosesPlayer(t *testing.T) { + const psk = "e2e-ctl-nohang" + hubAddr, relay := outageRelay(t, psk, map[string]any{"registrationGraceMs": 0}) + + relay.stop() + relay.dropAll() + time.Sleep(500 * time.Millisecond) + + // No route at all now, so the hub closes the connection during the handshake. + pc, err := net.DialTimeout("tcp", hubAddr, 5*time.Second) + if err != nil { + t.Fatalf("player dial: %v", err) + } + defer pc.Close() + if _, err := pc.Write(playerHandshake("mc.local")); err != nil { + t.Fatalf("player handshake: %v", err) + } + _ = pc.SetReadDeadline(time.Now().Add(10 * time.Second)) + if _, err := pc.Read(make([]byte, 1)); err == nil || isTimeout(err) { + t.Fatalf("player was held with the registration grace disabled (err=%v)", err) + } +} + +// TestControlOutageHangExpiresClosesPlayer covers the other end: a route that is +// never reclaimed must not hold its players forever. +func TestControlOutageHangExpiresClosesPlayer(t *testing.T) { + const psk = "e2e-ctl-expire" + hubAddr, relay := outageRelay(t, psk, map[string]any{"registrationGraceMs": 2000}) + + relay.stop() + relay.dropAll() + time.Sleep(500 * time.Millisecond) + + pc := resumePlayer(t, hubAddr, "mc.local") + start := time.Now() + _ = pc.SetReadDeadline(time.Now().Add(20 * time.Second)) + if _, err := pc.Read(make([]byte, 1)); err == nil || isTimeout(err) { + t.Fatalf("held player was never released after the grace expired (err=%v)", err) + } + if elapsed := time.Since(start); elapsed < 500*time.Millisecond { + t.Fatalf("player closed after %s, so it was refused rather than held", elapsed) + } else { + t.Logf("held player released after %s", elapsed.Round(100*time.Millisecond)) + } +} + +// isTimeout distinguishes "still held, nothing to read yet" from "the hub closed +// us" — which is the whole distinction these tests turn on. +func isTimeout(err error) bool { + var ne net.Error + if errors.As(err, &ne) { + return ne.Timeout() + } + return false +} + +func playerHandshake(host string) []byte { + return wire.BuildHandshake(767, host, 25565, 2) +} diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index f70e91a..d9cdcbe 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -22,13 +22,19 @@ func startClient(t *testing.T, hubAddr, psk string, maxConn int, mappings []clie // tests that need liveness detection to trigger quickly. func startClientWithPing(t *testing.T, hubAddr, psk string, maxConn, pingMs int, mappings []client.Mapping) *client.Client { t.Helper() - cfg := &client.Config{ + return startClientCfg(t, &client.Config{ Server: hubAddr, PSK: psk, MaxConn: maxConn, PingIntervalMs: pingMs, Mappings: mappings, - } + }) +} + +// startClientCfg runs an in-process client from a fully-specified config, for +// tests that need a knob the shorthand helpers do not expose. +func startClientCfg(t *testing.T, cfg *client.Config) *client.Client { + t.Helper() c := client.New(cfg) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(func() { @@ -142,7 +148,8 @@ func TestLargeTransfer(t *testing.T) { } // TestConcurrentStreamsUseMultipleConns confirms the least-loaded allocator -// opens additional worker connections once streams saturate (>8). +// grows the pool breadth-first: concurrent streams spread over several worker +// connections rather than stacking on one, without ever exceeding maxConn. func TestConcurrentStreamsUseMultipleConns(t *testing.T) { const psk = "e2e-concurrent" const n = 20 diff --git a/e2e/harness_test.go b/e2e/harness_test.go index 755f78a..748fcbf 100644 --- a/e2e/harness_test.go +++ b/e2e/harness_test.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "encoding/binary" + "encoding/json" "fmt" "io" "net" @@ -61,14 +62,33 @@ func freePort(t *testing.T) int { // startHub launches the Java hub on the given port and blocks until it accepts // connections. The process is killed on test cleanup. func startHub(t *testing.T, port int, psk string) { + t.Helper() + startHubCfg(t, port, psk, nil) +} + +// startHubCfg is startHub with extra config keys merged over the defaults, for +// tests that need to tune a hub-side knob. +func startHubCfg(t *testing.T, port int, psk string, extra map[string]any) { t.Helper() install := filepath.Join(repoRoot(), "server", "build", "install", "redapricot-server") if _, err := os.Stat(install); err != nil { t.Fatalf("hub not built at %s (run scripts/build.sh first): %v", install, err) } - cfg := fmt.Sprintf(`{"listen":"127.0.0.1:%d","psk":%q,"timestampWindowMs":30000,"pendingTimeoutMs":5000}`, port, psk) + settings := map[string]any{ + "listen": fmt.Sprintf("127.0.0.1:%d", port), + "psk": psk, + "timestampWindowMs": 30000, + "pendingTimeoutMs": 5000, + } + for k, v := range extra { + settings[k] = v + } + cfg, err := json.Marshal(settings) + if err != nil { + t.Fatal(err) + } cfgPath := filepath.Join(t.TempDir(), "hub.json") - if err := os.WriteFile(cfgPath, []byte(cfg), 0o644); err != nil { + if err := os.WriteFile(cfgPath, cfg, 0o644); err != nil { t.Fatal(err) } diff --git a/e2e/resilience_test.go b/e2e/resilience_test.go index 98ead77..ca9978a 100644 --- a/e2e/resilience_test.go +++ b/e2e/resilience_test.go @@ -26,6 +26,7 @@ type blackholeRelay struct { addr string backend string gen atomic.Uint64 + down atomic.Bool mu sync.Mutex held []net.Conn } @@ -59,6 +60,10 @@ func newBlackholeRelay(t *testing.T, backend string) *blackholeRelay { func (r *blackholeRelay) handle(cli net.Conn) { born := r.gen.Load() + if r.down.Load() { + _ = cli.Close() + return + } up, err := net.Dial("tcp", r.backend) if err != nil { _ = cli.Close() @@ -89,6 +94,34 @@ func (r *blackholeRelay) handle(cli net.Conn) { // unaffected. func (r *blackholeRelay) blackhole() { r.gen.Add(1) } +// dropAll hard-resets every currently-established pair: a real FIN/RST reaches +// both ends immediately, as when a middlebox is restarted or a route flaps, +// rather than the silent stranding blackhole models. Later connections are +// carried normally. +// +// stop takes the relay out of service: new connections are refused rather than +// carried, so the tunnel stays down until restore is called. Models an outage +// the client cannot immediately reconnect through. +func (r *blackholeRelay) stop() { r.down.Store(true) } + +// restore puts the relay back in service. Flows stranded before it are still +// dead — only fresh connections are carried, which is what a client gets after +// a middlebox or upstream link comes back. +func (r *blackholeRelay) restore() { r.down.Store(false) } + +// This is the fast path into stream resumption: the client learns the conn is +// gone at once instead of waiting out a heartbeat timeout. +func (r *blackholeRelay) dropAll() { + r.gen.Add(1) + r.mu.Lock() + held := r.held + r.held = nil + r.mu.Unlock() + for _, c := range held { + _ = c.Close() + } +} + // TestBlackholedPathRecovers is the end-to-end regression guard for the // stability bug this hardening was written for: with the tunnel's path silently // dropped, the client used to notice nothing at all. Its read loops parked diff --git a/e2e/resume_test.go b/e2e/resume_test.go new file mode 100644 index 0000000..1a32e2d --- /dev/null +++ b/e2e/resume_test.go @@ -0,0 +1,270 @@ +package e2e + +import ( + "bytes" + "fmt" + "io" + "math/rand" + "net" + "sync" + "testing" + "time" + + "github.com/iceBear67/redapricot/client" + "github.com/iceBear67/redapricot/client/wire" +) + +// resumePlayer opens a player connection and returns it, having sent only the +// handshake. The caller keeps it open across the outage — which is the whole +// point: before stream resumption this socket was closed by the hub the instant +// its worker conn died. +func resumePlayer(t *testing.T, hubAddr, host string) net.Conn { + t.Helper() + pc, err := net.DialTimeout("tcp", hubAddr, 5*time.Second) + if err != nil { + t.Fatalf("player dial: %v", err) + } + t.Cleanup(func() { _ = pc.Close() }) + if _, err := pc.Write(wire.BuildHandshake(767, host, 25565, 2)); err != nil { + t.Fatalf("player handshake: %v", err) + } + return pc +} + +// echoExchange streams payload through an echoing destination and verifies that +// what comes back is byte-for-byte identical, calling disrupt once `at` bytes +// have made the round trip. +// +// Comparing the whole stream rather than sampling is deliberate: a resumption +// bug does not corrupt bytes, it duplicates or skips a range, and only an exact +// comparison of the full sequence catches an off-by-one in the offsets. +func echoExchange(t *testing.T, pc net.Conn, payload []byte, at int, disrupt func()) { + t.Helper() + const chunk = 16 << 10 + + var wg sync.WaitGroup + wg.Add(1) + writeErr := make(chan error, 1) + go func() { + defer wg.Done() + for off := 0; off < len(payload); off += chunk { + end := min(off+chunk, len(payload)) + _ = pc.SetWriteDeadline(time.Now().Add(60 * time.Second)) + if _, err := pc.Write(payload[off:end]); err != nil { + writeErr <- fmt.Errorf("write at %d: %w", off, err) + return + } + } + writeErr <- nil + }() + + got := make([]byte, len(payload)) + read, fired := 0, false + for read < len(got) { + _ = pc.SetReadDeadline(time.Now().Add(60 * time.Second)) + n, err := pc.Read(got[read:]) + read += n + if !fired && read >= at { + fired = true + disrupt() + } + if err != nil { + t.Fatalf("player read failed after %d/%d bytes: %v", read, len(got), err) + } + } + wg.Wait() + if err := <-writeErr; err != nil { + t.Fatalf("player write: %v", err) + } + + if !bytes.Equal(got, payload) { + // Report the first divergence: its offset says whether the stream gained + // or lost bytes, which is the difference between a retransmit that + // replayed too much and one that replayed too little. + for i := range got { + if got[i] != payload[i] { + t.Fatalf("echo diverges at byte %d of %d (sent %#x, got %#x)", + i, len(payload), payload[i], got[i]) + } + } + } +} + +func randomPayload(seed int64, n int) []byte { + p := make([]byte, n) + rand.New(rand.NewSource(seed)).Read(p) + return p +} + +// TestResumePreservesByteStream is the correctness bar for stream resumption: +// a worker conn is hard-reset mid-transfer and the *same* player connection must +// keep working, with a byte stream that neither gains nor loses a single byte. +// +// Byte-exactness is the whole difficulty. Frames handed to a dying socket are +// lost with no notification and the cipher cannot be resynchronized, so each +// side has to replay from the offset the other reports it accepted. Getting that +// offset wrong by any amount splices the stream mid-Minecraft-packet, which a +// round-trip test that only checked "traffic flows again" would happily pass. +func TestResumePreservesByteStream(t *testing.T) { + const psk = "e2e-resume" + hubPort := freePort(t) + hubAddr := fmt.Sprintf("127.0.0.1:%d", hubPort) + startHub(t, hubPort, psk) + dest := newMockDest(t, modeEcho) + + // Only the tunnel runs through the relay; the player talks to the hub + // directly, as it would from the internet. So the drop hits the middle leg + // while both terminal sockets stay healthy — exactly the case resumption is + // for. + relay := newBlackholeRelay(t, hubAddr) + c := startClientWithPing(t, relay.addr, psk, 1, 400, []client.Mapping{ + {Pattern: "mc.local", Destination: dest.addr}, + }) + + pc := resumePlayer(t, hubAddr, "mc.local") + payload := randomPayload(1, 3<<20) + echoExchange(t, pc, payload, 512<<10, func() { + t.Log("hard-resetting the tunnel mid-transfer") + relay.dropAll() + }) + t.Logf("stream survived the reset intact; worker conns now %d", c.WorkerConnCount()) +} + +// TestResumeWithConcurrentStreams covers the failure the single-stream test +// cannot reach: stream ids restart at 1 on every conn, so after a reattach two +// players can hold the same id on different conns. A binding that updates the +// conn and the id separately will credit or reset the wrong player's stream, and +// that only shows up when a second stream is there to be corrupted. +func TestResumeWithConcurrentStreams(t *testing.T) { + const psk = "e2e-resume-multi" + hubPort := freePort(t) + hubAddr := fmt.Sprintf("127.0.0.1:%d", hubPort) + startHub(t, hubPort, psk) + dest := newMockDest(t, modeEcho) + + relay := newBlackholeRelay(t, hubAddr) + startClientWithPing(t, relay.addr, psk, 2, 400, []client.Mapping{ + {Pattern: "mc.local", Destination: dest.addr}, + }) + + const players = 3 + conns := make([]net.Conn, players) + for i := range conns { + conns[i] = resumePlayer(t, hubAddr, "mc.local") + // Distinct payloads: if a reattach crosses two streams the bytes land on + // the wrong player, which an identical payload would hide. + if _, err := conns[i].Write([]byte(fmt.Sprintf("hello-%d", i))); err != nil { + t.Fatalf("player %d warmup write: %v", i, err) + } + echo := make([]byte, len("hello-0")) + _ = conns[i].SetReadDeadline(time.Now().Add(15 * time.Second)) + if _, err := io.ReadFull(conns[i], echo); err != nil { + t.Fatalf("player %d warmup echo: %v", i, err) + } + } + + var wg sync.WaitGroup + for i := range conns { + wg.Add(1) + go func(i int) { + defer wg.Done() + payload := randomPayload(int64(100+i), 768<<10) + // Only the first player triggers the reset; the others are mid-flight + // when it lands. + disrupt := func() {} + if i == 0 { + disrupt = relay.dropAll + } + echoExchange(t, conns[i], payload, 128<<10, disrupt) + }(i) + } + wg.Wait() +} + +// TestResumeDisabledClosesImmediately pins the off switch. With resumption +// declined the hub must not hang the player waiting for a reattach that is never +// coming: the socket has to close as it did before the feature existed, so +// disabling it is a true revert rather than a slower failure. +func TestResumeDisabledClosesImmediately(t *testing.T) { + const psk = "e2e-resume-off" + hubPort := freePort(t) + hubAddr := fmt.Sprintf("127.0.0.1:%d", hubPort) + startHub(t, hubPort, psk) + dest := newMockDest(t, modeEcho) + + relay := newBlackholeRelay(t, hubAddr) + off := false + startClientCfg(t, &client.Config{ + Server: relay.addr, + PSK: psk, + MaxConn: 1, + PingIntervalMs: 400, + StreamResume: &off, + Mappings: []client.Mapping{{Pattern: "mc.local", Destination: dest.addr}}, + }) + + pc := resumePlayer(t, hubAddr, "mc.local") + if _, err := pc.Write([]byte("ping")); err != nil { + t.Fatalf("warmup write: %v", err) + } + echo := make([]byte, 4) + _ = pc.SetReadDeadline(time.Now().Add(15 * time.Second)) + if _, err := io.ReadFull(pc, echo); err != nil { + t.Fatalf("warmup echo: %v", err) + } + + relay.dropAll() + + // Well inside the hub's 20s grace: if the player is still open here, the hub + // parked a stream for a client that never opted in. + _ = pc.SetReadDeadline(time.Now().Add(10 * time.Second)) + if _, err := pc.Read(make([]byte, 1)); err == nil { + t.Fatal("player socket stayed open after the tunnel dropped with resume disabled") + } +} + +// TestResumeGraceExpiryClosesPlayer covers the other end of the lifetime: when +// the tunnel never comes back, a hung player must not hang forever. The hub +// drops it once its grace expires, without leaking the stream or its buffers. +func TestResumeGraceExpiryClosesPlayer(t *testing.T) { + const psk = "e2e-resume-grace" + hubPort := freePort(t) + hubAddr := fmt.Sprintf("127.0.0.1:%d", hubPort) + startHubCfg(t, hubPort, psk, map[string]any{"resumeGraceMs": 3000}) + dest := newMockDest(t, modeEcho) + + relay := newBlackholeRelay(t, hubAddr) + startClientCfg(t, &client.Config{ + Server: relay.addr, + PSK: psk, + MaxConn: 1, + PingIntervalMs: 400, + ResumeGraceMs: 2000, + Mappings: []client.Mapping{{Pattern: "mc.local", Destination: dest.addr}}, + }) + + pc := resumePlayer(t, hubAddr, "mc.local") + if _, err := pc.Write([]byte("ping")); err != nil { + t.Fatalf("warmup write: %v", err) + } + echo := make([]byte, 4) + _ = pc.SetReadDeadline(time.Now().Add(15 * time.Second)) + if _, err := io.ReadFull(pc, echo); err != nil { + t.Fatalf("warmup echo: %v", err) + } + + // Stop carrying the tunnel entirely, so every reattach attempt fails. + relay.stop() + relay.dropAll() + + start := time.Now() + _ = pc.SetReadDeadline(time.Now().Add(30 * time.Second)) + if _, err := pc.Read(make([]byte, 1)); err == nil { + t.Fatal("player socket never closed after the resume grace expired") + } + if elapsed := time.Since(start); elapsed < time.Second { + t.Fatalf("player closed after %s, before any reattach could be attempted", elapsed) + } else { + t.Logf("hung player released after %s", elapsed.Round(100*time.Millisecond)) + } +} diff --git a/server/config.example.json b/server/config.example.json index 62fe45a..2993073 100644 --- a/server/config.example.json +++ b/server/config.example.json @@ -4,5 +4,10 @@ "timestampWindowMs": 30000, "pendingTimeoutMs": 10000, "streamWindowBytes": 262144, - "sessionIdleTimeoutMs": 90000 + "sessionIdleTimeoutMs": 90000, + "streamResume": true, + "resumeGraceMs": 20000, + "maxParkedStreams": 256, + "statsIntervalMs": 0, + "registrationGraceMs": 15000 } diff --git a/server/src/main/java/io/icybear/redapricot/Config.java b/server/src/main/java/io/icybear/redapricot/Config.java index 93a67ae..741c51d 100644 --- a/server/src/main/java/io/icybear/redapricot/Config.java +++ b/server/src/main/java/io/icybear/redapricot/Config.java @@ -13,7 +13,13 @@ public record Config( long timestampWindowMs, long pendingTimeoutMs, int streamWindowBytes, - long sessionIdleTimeoutMs + long sessionIdleTimeoutMs, + boolean streamResume, + long resumeGraceMs, + int maxParkedStreams, + long maxParkedBytes, + long statsIntervalMs, + long registrationGraceMs ) { public static Config load(Path file) throws Exception { JsonObject json = new JsonObject(Files.readString(file)); @@ -30,6 +36,15 @@ public record Config( int window = json.getInteger("streamWindowBytes", Protocol.DEFAULT_STREAM_WINDOW); window = Math.max(Protocol.MIN_STREAM_WINDOW, Math.min(Protocol.MAX_STREAM_WINDOW, window)); + boolean resume = json.getBoolean("streamResume", Boolean.TRUE); + // A parked stream can hold up to one window of unacked bytes plus one of + // parked player bytes, for the whole grace period, and nothing else + // bounds how many streams park at once — so anyone able to kill worker + // conns is otherwise a cheap memory amplifier. The default admits ~256 + // hanging players at the default window. + int maxParked = json.getInteger("maxParkedStreams", 256); + long maxParkedBytes = json.getLong("maxParkedBytes", (long) maxParked * 2 * window); + return new Config( host, port, @@ -39,6 +54,20 @@ public record Config( window, // Comfortably above the client's default 20s ping interval; // 0 disables the watchdog. - json.getLong("sessionIdleTimeoutMs", 90_000L)); + json.getLong("sessionIdleTimeoutMs", 90_000L), + resume, + // Must exceed the client's own grace by at least one dial, or the + // hub drops a player while its client is still mid-reattach. The + // value is advertised in SessionReady precisely so the client can + // clamp itself under it rather than rely on matching config. + json.getLong("resumeGraceMs", 20_000L), + maxParked, + maxParkedBytes, + json.getLong("statsIntervalMs", 0L), + // Long enough to cover a client's control-session reconnect + // (its backoff caps at 10s) without holding a player so long + // that they give up anyway; 0 disables and restores the old + // behaviour of dropping routes the moment a session closes. + json.getLong("registrationGraceMs", 15_000L)); } } diff --git a/server/src/main/java/io/icybear/redapricot/Hub.java b/server/src/main/java/io/icybear/redapricot/Hub.java index d19199a..fbc9d93 100644 --- a/server/src/main/java/io/icybear/redapricot/Hub.java +++ b/server/src/main/java/io/icybear/redapricot/Hub.java @@ -7,10 +7,16 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ThreadLocalRandom; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; @@ -21,6 +27,7 @@ import java.util.regex.PatternSyntaxException; */ public final class Hub { private static final Logger LOG = LogManager.getLogger("redapricot.hub"); + private static final SecureRandom RNG = new SecureRandom(); public final Vertx vertx; public final Config config; @@ -30,11 +37,40 @@ public final class Hub { private final Map patterns = new ConcurrentHashMap<>(); private final Map pending = new ConcurrentHashMap<>(); - /** A compiled routing pattern and the control session that registered it. */ - private record Registration(Pattern regex, ControlSession session) {} + /** + * Every tunneled player, keyed by its current CID, whether live or parked. + * Keeping parked streams here rather than on the worker conn is the whole + * point: a stream's identity is the player, and state that dies with the + * connection cannot survive that connection dying. Insertion-ordered so the + * parked cap can evict the oldest first. + */ + private final Map streams = new LinkedHashMap<>(); + private int parkedCount; + private long parkedBytes; - /** A successful match: the registered pattern that matched and its owning session. */ - public record Match(String pattern, ControlSession session) {} + /** + * A compiled routing pattern and the control session that registered it. + * + *

{@code session} is null while the registration is orphaned — its + * client's control session has closed but the route is held open until + * {@code orphanDeadline} in case the client reconnects. Players matching an + * orphaned route are hung rather than refused. + */ + private record Registration(Pattern regex, ControlSession session, long orphanDeadline) { + Registration(Pattern regex, ControlSession session) { + this(regex, session, 0); + } + + boolean orphaned() { + return session == null; + } + } + + /** + * A successful match: the registered pattern that matched and its owning + * session, which is null when the route is orphaned (§ control-outage hang). + */ + public record Match(String pattern, ControlSession session, long orphanDeadline) {} public Hub(Vertx vertx, Config config) { this.vertx = vertx; @@ -64,9 +100,29 @@ public final class Hub { } patterns.put(pattern, new Registration(regex, session)); LOG.info("registered pattern '{}' -> {}", pattern, session.id()); + replayAwaiting(pattern, session); return Protocol.REGISTER_OK; } + /** + * Deliver the control requests held while this pattern had no live session. + * + *

These players connected during the client's reconnect and were hung + * instead of refused; the request was never sent, so it is sent now. Each + * moves from "waiting for a route" to the ordinary "waiting for a worker", + * which means swapping its deadline over to {@code pendingTimeoutMs}. + */ + private void replayAwaiting(String pattern, ControlSession session) { + for (PendingPlayer p : pending.values()) { + if (!p.isAwaitingSession() || !p.getPattern().equals(pattern)) continue; + p.setAwaitingSession(false); + p.setOwner(session); + rearm(p, config.pendingTimeoutMs()); + session.sendControlRequest(p.getCid(), p.getPattern(), p.getPlayerIp(), p.getPlayerPort()); + LOG.info("replayed control request for hung player {} on session {}", p.getCidHex(), session.id()); + } + } + public void unregister(String pattern, ControlSession session) { // Remove only if this session still owns the pattern (a newer session may have taken it). patterns.computeIfPresent(pattern, (k, reg) -> reg.session() == session ? null : reg); @@ -80,36 +136,140 @@ public final class Hub { public Match match(String address) { String host = normalizeAddress(address); for (Map.Entry e : patterns.entrySet()) { - if (e.getValue().regex().matcher(host).matches()) { - return new Match(e.getKey(), e.getValue().session()); + Registration reg = e.getValue(); + if (reg.regex().matcher(host).matches()) { + return new Match(e.getKey(), reg.session(), reg.orphanDeadline()); } } return null; } - /** Drop every pattern owned by a (closing) session, plus any players still pending for it. */ + /** + * A control session closed. Its routes are kept as orphaned for + * {@code registrationGraceMs}, and the players waiting on them are hung + * rather than dropped. + * + *

Without this, a client's reconnect — half a second at best, ten at worst + * once its backoff has grown — is a window in which every arriving player is + * told there is no such server, even though the tunnel is seconds from being + * back. The players already tunneled are unaffected either way; they ride + * worker conns, which a control-session close never touches. + * + *

A grace of 0 restores the old behaviour exactly. + */ public void removeSession(ControlSession session) { - patterns.entrySet().removeIf(e -> e.getValue().session() == session); + long grace = config.registrationGraceMs(); + if (grace <= 0) { + patterns.entrySet().removeIf(e -> e.getValue().session() == session); + pending.values().removeIf(p -> { + if (p.getOwner() != session) return false; + cancelTimer(p); + p.getSocket().close(); + LOG.info("dropping pending player {} (control session {} closed)", p.getCidHex(), session.id()); + return true; + }); + return; + } + + long deadline = System.currentTimeMillis() + grace; + int orphaned = 0; + for (Map.Entry e : patterns.entrySet()) { + Registration reg = e.getValue(); + if (reg.session() != session) continue; + e.setValue(new Registration(reg.regex(), null, deadline)); + orphaned++; + } + + // A player that was already matched is in the same position: its request + // went to a session that will never answer, so it waits for the route to + // come back and is then replayed like any other. + int hung = 0; + for (PendingPlayer p : pending.values()) { + if (p.getOwner() != session) continue; + p.setOwner(null); + p.setAwaitingSession(true); + rearm(p, grace); + hung++; + } + if (orphaned > 0 || hung > 0) { + LOG.info("control session {} closed; holding {} route(s) and {} player(s) for {}ms", + session.id(), orphaned, hung, grace); + vertx.setTimer(grace, id -> expireOrphans()); + } + } + + /** Drop routes whose grace ran out, and the players still hung on them. */ + private void expireOrphans() { + long now = System.currentTimeMillis(); + Set gone = new HashSet<>(); + patterns.entrySet().removeIf(e -> { + Registration reg = e.getValue(); + if (!reg.orphaned() || reg.orphanDeadline() > now) return false; + gone.add(e.getKey()); + return true; + }); + if (gone.isEmpty()) return; + LOG.info("dropping {} orphaned route(s) not reclaimed within the grace period", gone.size()); pending.values().removeIf(p -> { - if (p.getOwner() != session) return false; - if (p.getTimerId() >= 0) vertx.cancelTimer(p.getTimerId()); + if (!p.isAwaitingSession() || !gone.contains(p.getPattern())) return false; + cancelTimer(p); p.getSocket().close(); - LOG.info("dropping pending player {} (control session {} closed)", p.getCidHex(), session.id()); + LOG.info("dropping hung player {} (route '{}' never came back)", p.getCidHex(), p.getPattern()); return true; }); } // ---- pending players ---- + /** + * Mint a takeover capability. The CID is the only thing authorizing a client + * to claim a player (and, with resumption, to reclaim one), so it comes from + * a cryptographic source rather than ThreadLocalRandom — a predictable value + * would be a session-hijacking primitive. + */ public byte[] newCid() { byte[] cid = new byte[Protocol.CID_LEN]; - ThreadLocalRandom.current().nextBytes(cid); + RNG.nextBytes(cid); return cid; } public void addPending(PendingPlayer p) { pending.put(p.getCidHex(), p); - p.setTimerId(vertx.setTimer(config.pendingTimeoutMs(), id -> { + rearm(p, config.pendingTimeoutMs()); + } + + /** + * Hang a player whose route is orphaned: hold it until a client re-registers + * the pattern, at which point {@link #replayAwaiting} delivers the control + * request that was never sent. + * + * @param deadline wall-clock millis at which the route's grace runs out + */ + public void addAwaiting(PendingPlayer p, long deadline) { + p.setOwner(null); + p.setAwaitingSession(true); + pending.put(p.getCidHex(), p); + rearm(p, Math.max(1, deadline - System.currentTimeMillis())); + LOG.info("holding player {} for '{}': route is orphaned, waiting for its client", + p.getCidHex(), p.getPattern()); + } + + public PendingPlayer takePending(byte[] cid) { + String hex = Hex.encode(cid); + PendingPlayer p = pending.remove(hex); + if (p != null) cancelTimer(p); + return p; + } + + public void removePending(String cidHex) { + PendingPlayer p = pending.remove(cidHex); + if (p != null) cancelTimer(p); + } + + /** Replace a pending player's deadline, cancelling whatever it had. */ + private void rearm(PendingPlayer p, long delayMs) { + cancelTimer(p); + p.setTimerId(vertx.setTimer(delayMs, id -> { PendingPlayer removed = pending.remove(p.getCidHex()); if (removed != null) { LOG.warn("pending player {} timed out", p.getCidHex()); @@ -118,16 +278,167 @@ public final class Hub { })); } - public PendingPlayer takePending(byte[] cid) { - String hex = Hex.encode(cid); - PendingPlayer p = pending.remove(hex); - if (p != null && p.getTimerId() >= 0) vertx.cancelTimer(p.getTimerId()); - return p; + private void cancelTimer(PendingPlayer p) { + if (p.getTimerId() >= 0) { + vertx.cancelTimer(p.getTimerId()); + p.setTimerId(-1); + } } - public void removePending(String cidHex) { - PendingPlayer p = pending.remove(cidHex); - if (p != null && p.getTimerId() >= 0) vertx.cancelTimer(p.getTimerId()); + // ---- tunneled streams & resumption (§7.5) ---- + + /** Register a stream that has just been bound to a worker conn. */ + public void addStream(PlayerStream st) { + streams.put(st.cidHex, st); + } + + /** Forget a stream for good; its player socket is gone or going. */ + public void removeStream(PlayerStream st) { + if (streams.remove(st.cidHex) == null) return; + if (st.parked) unpark(st); + st.unacked.clear(); + } + + /** Look up a stream by the CID a client presented, live or parked. */ + public PlayerStream streamByCid(byte[] cid) { + return streams.get(Hex.encode(cid)); + } + + /** + * Player bytes arrived: hand them to whichever conn currently carries the + * stream. Routing here rather than from the conn that installed the socket + * handler is what lets a stream change conns without rebinding handlers. + */ + public void onPlayerData(PlayerStream st, io.vertx.core.buffer.Buffer buf) { + WorkerConn w = st.worker; + if (w != null) { + w.playerData(st, buf); + return; + } + // Parked, so there is nowhere to send: hold the bytes in order and make + // sure the socket really is stopped. The player is paused on park, but a + // batch already in flight can still land here. + if (st.pendingUp == null) st.pendingUp = io.vertx.core.buffer.Buffer.buffer(); + st.pendingUp.appendBuffer(buf); + st.player.pause(); + } + + /** The player hung up. Drop the stream everywhere and tell the client if it is still bound. */ + public void onPlayerGone(PlayerStream st) { + WorkerConn w = st.worker; + removeStream(st); + if (w != null) w.playerGone(st); + } + + /** + * Hang a player whose worker conn died, instead of closing it. + * + *

Only the tunnel leg failed — the player socket is still perfectly good — + * so it is paused and held until the client reattaches the stream over a + * fresh conn. The deadline is absolute and fixed at the first park: re-arming + * it on each park would let a flapping client hold a player forever. + * + * @return false if the stream cannot be parked and must be closed instead + */ + public boolean park(PlayerStream st) { + if (!st.resumable || st.parked) return false; + long now = System.currentTimeMillis(); + if (st.graceDeadline == 0) st.graceDeadline = now + config.resumeGraceMs(); + long remaining = st.graceDeadline - now; + if (remaining <= 0) return false; + + st.parked = true; + st.worker = null; + // Explicitly, not as a side effect of the window filling: an idle stream + // has a wide-open window, so nothing else would stop the next keepalive + // walking into a send on a dead connection. + st.player.pause(); + st.pausedForAggregate = false; // that conn's drain handler will never fire again + + parkedCount++; + parkedBytes += st.parkedBytes(); + st.timerId = vertx.setTimer(remaining, id -> { + LOG.info("parked player {} not reclaimed within grace; closing", st.cidHex); + removeStream(st); + st.player.close(); + }); + enforceParkedCaps(st); + return true; + } + + /** + * Reclaim a parked stream. Returns null when no stream is parked under this + * CID; the caller distinguishes "never heard of it" from "still bound + * elsewhere" via {@link #streamByCid}. + */ + public PlayerStream takeParked(byte[] cid) { + PlayerStream st = streams.get(Hex.encode(cid)); + if (st == null || !st.parked) return null; + unpark(st); + return st; + } + + /** Re-key a stream to the freshly minted CID handed out in RESUME_ACK. */ + public void rekeyStream(PlayerStream st, byte[] cid) { + streams.remove(st.cidHex); + st.cid = cid; + st.cidHex = Hex.encode(cid); + streams.put(st.cidHex, st); + } + + private void unpark(PlayerStream st) { + if (!st.parked) return; + st.parked = false; + parkedCount--; + parkedBytes -= st.parkedBytes(); + if (parkedBytes < 0) parkedBytes = 0; + if (st.timerId >= 0) { + vertx.cancelTimer(st.timerId); + st.timerId = -1; + } + } + + /** + * Bound what hanging players may cost. Each holds up to a window of unsent + * bytes plus a window of parked ones for the whole grace period, and nothing + * else limits how many park at once — so anyone able to kill worker conns is + * otherwise a cheap memory amplifier. Oldest first, since they have the least + * grace left to be reclaimed in. + */ + private void enforceParkedCaps(PlayerStream keep) { + if (parkedCount <= config.maxParkedStreams() && parkedBytes <= config.maxParkedBytes()) return; + List evict = new ArrayList<>(); + Iterator it = streams.values().iterator(); + while (it.hasNext() && (parkedCount - evict.size() > config.maxParkedStreams() + || parkedBytes > config.maxParkedBytes())) { + PlayerStream st = it.next(); + if (!st.parked || st == keep) continue; + evict.add(st); + parkedBytes -= st.parkedBytes(); + } + for (PlayerStream st : evict) { + LOG.warn("parked-stream cap reached; dropping hanging player {}", st.cidHex); + parkedBytes += st.parkedBytes(); // removeStream subtracts it again + removeStream(st); + st.player.close(); + } + } + + /** Live and parked stream counts, for the periodic stats line. */ + public int streamCount() { + return streams.size(); + } + + public int parkedCount() { + return parkedCount; + } + + public long parkedBytes() { + return parkedBytes; + } + + public int patternCount() { + return patterns.size(); } // ---- helpers ---- diff --git a/server/src/main/java/io/icybear/redapricot/HubConnection.java b/server/src/main/java/io/icybear/redapricot/HubConnection.java index d26200c..78de932 100644 --- a/server/src/main/java/io/icybear/redapricot/HubConnection.java +++ b/server/src/main/java/io/icybear/redapricot/HubConnection.java @@ -168,6 +168,10 @@ public final class HubConnection { // Heartbeat is optional: accept it only when the client offered it, so // older clients keep working (they just lose silent-path detection). boolean heartbeat = (flags & Protocol.FLAG_WORKER_HEARTBEAT) != 0; + // Resumption likewise. Parking a stream for a client that will never + // reattach is strictly worse than closing it — the player hangs for the + // whole grace instead of failing fast — so this bit gates the park path. + boolean resume = hub.config.streamResume() && (flags & Protocol.FLAG_STREAM_RESUME) != 0; // REKEY = Rand || Timestamp(I64 big-endian). Magic is excluded. byte[] rekey = new byte[randLen + 8]; @@ -181,13 +185,20 @@ public final class HubConnection { frames.switchCiphers( Crypto.decryptCipher(rekey, Crypto.DIR_C2S), Crypto.encryptCipher(rekey, Crypto.DIR_S2C)); - // Echo the accepted flags plus our own receive window. - int accepted = Protocol.FLAG_STREAM_FC | (heartbeat ? Protocol.FLAG_WORKER_HEARTBEAT : 0); - frames.send(new ProtoWriter() + // Echo the accepted flags plus our own receive window. When resumption is + // accepted, our grace period follows: the client clamps its own retry + // budget under it, which turns a cross-config invariant ("the hub must + // wait longer than the client retries") into a negotiated one that + // operator skew cannot break. + int accepted = Protocol.FLAG_STREAM_FC + | (heartbeat ? Protocol.FLAG_WORKER_HEARTBEAT : 0) + | (resume ? Protocol.FLAG_STREAM_RESUME : 0); + ProtoWriter ready = new ProtoWriter() .u8(Protocol.CTL_SESSION_READY) .varInt(accepted) - .varInt(hub.config.streamWindowBytes()) - .toBytes()); + .varInt(hub.config.streamWindowBytes()); + if (resume) ready.varInt((int) hub.config.resumeGraceMs()); + frames.send(ready.toBytes()); if (magic == Protocol.MAGIC_CONTROL) { ControlSession session = new ControlSession(hub, frames, id); @@ -195,10 +206,11 @@ public final class HubConnection { closeCleanup = session::onClose; LOG.info("{} control session established (heartbeat {})", id, heartbeat); } else if (magic == Protocol.MAGIC_WORKER) { - WorkerConn worker = new WorkerConn(hub, frames, id, peerWindow, hub.config.streamWindowBytes()); + WorkerConn worker = new WorkerConn(hub, frames, id, peerWindow, hub.config.streamWindowBytes(), resume); frames.setHandler(worker::onFrame); closeCleanup = worker::onClose; - LOG.info("{} worker conn established (peer window {}, heartbeat {})", id, peerWindow, heartbeat); + LOG.info("{} worker conn established (peer window {}, heartbeat {}, resume {})", + id, peerWindow, heartbeat, resume); } else { LOG.warn("{} bad magic {}; closing", id, magic); frames.close(); @@ -259,12 +271,21 @@ public final class HubConnection { socket.pause(); Buffer buffered = hs.copy(); // handshake + any pipelined bytes, forwarded verbatim - PendingPlayer p = new PendingPlayer(cid, cidHex, socket, buffered, pattern, ip, port, session); - hub.addPending(p); + PendingPlayer p = new PendingPlayer(cid, cidHex, socket, buffered, pattern, ip, port); closeCleanup = () -> hub.removePending(cidHex); - session.sendControlRequest(cid, pattern, ip, port); - LOG.info("{} player {}:{} host '{}' matched pattern '{}' cid={}", - id, ip, port, host, pattern, cidHex); + if (session == null) { + // The route is orphaned: its client's control session has closed and + // has not come back yet. Hold the player rather than telling it there + // is no such server — the request is replayed the moment a client + // re-registers the pattern. + hub.addAwaiting(p, matched.orphanDeadline()); + } else { + p.setOwner(session); + hub.addPending(p); + session.sendControlRequest(cid, pattern, ip, port); + } + LOG.info("{} player {}:{} host '{}' matched pattern '{}' cid={}{}", + id, ip, port, host, pattern, cidHex, session == null ? " (held: route orphaned)" : ""); } } diff --git a/server/src/main/java/io/icybear/redapricot/HubServer.java b/server/src/main/java/io/icybear/redapricot/HubServer.java index ac6010e..f510d12 100644 --- a/server/src/main/java/io/icybear/redapricot/HubServer.java +++ b/server/src/main/java/io/icybear/redapricot/HubServer.java @@ -35,10 +35,27 @@ public final class HubServer extends AbstractVerticle { if (ar.succeeded()) { LOG.info("redapricot hub listening on {}:{}", config.host(), ar.result().actualPort()); LOG.info("PSK handshake address: {}", hub.pskAddress); + armStats(); startPromise.complete(); } else { startPromise.fail(ar.cause()); } }); } + + /** + * Periodic one-line snapshot of what the hub is holding. Off unless + * statsIntervalMs is set, so it costs nothing by default. + * + *

Parked streams and the bytes they retain are the numbers worth watching: + * they are the memory stream resumption trades for keeping players connected, + * and the first place a resumption problem shows up as a trend. + */ + private void armStats() { + long interval = config.statsIntervalMs(); + if (interval <= 0) return; + vertx.setPeriodic(interval, id -> LOG.info( + "stats streams={} parked={} parkedBytes={} patterns={}", + hub.streamCount(), hub.parkedCount(), hub.parkedBytes(), hub.patternCount())); + } } diff --git a/server/src/main/java/io/icybear/redapricot/PendingPlayer.java b/server/src/main/java/io/icybear/redapricot/PendingPlayer.java index 00c1b7d..c37dbd5 100644 --- a/server/src/main/java/io/icybear/redapricot/PendingPlayer.java +++ b/server/src/main/java/io/icybear/redapricot/PendingPlayer.java @@ -17,7 +17,22 @@ public final class PendingPlayer { private final String pattern; private final String playerIp; private final int playerPort; - private final ControlSession owner; // control session this player was routed to + + /** + * Control session this player was routed to, or null while the route is + * orphaned. Mutable because a hung player is rebound to whichever session + * re-registers its pattern. + */ + @Setter + private ControlSession owner; + + /** + * Hung waiting for a control session to come back, rather than waiting for a + * worker to claim it. No ControlRequest has been delivered yet, so this + * player is the hub's to replay once a route reappears. + */ + @Setter + private boolean awaitingSession; @Setter private long timerId = -1; diff --git a/server/src/main/java/io/icybear/redapricot/PlayerStream.java b/server/src/main/java/io/icybear/redapricot/PlayerStream.java new file mode 100644 index 0000000..26f0187 --- /dev/null +++ b/server/src/main/java/io/icybear/redapricot/PlayerStream.java @@ -0,0 +1,88 @@ +package io.icybear.redapricot; + +import io.vertx.core.buffer.Buffer; +import io.vertx.core.net.NetSocket; + +/** + * One tunneled player: the player socket plus the flow-control state of the mux + * stream carrying it (PROTOCOL.md §7.3). + * + *

This is deliberately not owned by {@link WorkerConn}. A stream's + * identity is the player, not the connection it happens to ride: the worker conn + * is a replaceable transport, and state that dies with it cannot be recovered + * when it drops. + * + *

Confined to the hub's single event loop, so the mutable fields need no + * synchronization. + */ +public final class PlayerStream { + /** Capability that authorized the takeover; also the resume key. Re-minted on each reattach. */ + byte[] cid; + String cidHex; + final NetSocket player; + /** Registered pattern that matched, echoed to the client. */ + final String pattern; + final String playerIp; + final int playerPort; + + /** The conn currently carrying this stream, and its id there. */ + WorkerConn worker; + int sid; + + /** Budget for player -> client DATA. */ + int sendWnd; + /** Player bytes awaiting send window; the player is paused while non-null. */ + Buffer pendingUp; + /** client -> player bytes flushed but not yet granted back. */ + int credited; + + // Pause reasons. Vert.x pause() is a flag rather than a counter, so a socket + // can be paused for several reasons at once and must only be resumed once + // none of them hold — see WorkerConn#maybeResumePlayer. + boolean pausedForWindow; // this stream's send window is exhausted + boolean pausedForAggregate; // the shared worker socket's write queue is full + boolean parked; // the worker conn died; hanging until a reattach + + /** Whether the conn carrying this stream negotiated resumption (§7.5). */ + boolean resumable; + + // Resumption bookkeeping (§7.5). Three distinct offsets, and conflating them + // is the classic mistake: what to retransmit is measured from what the peer + // *accepted*, while the flow-control window is measured from what it + // *credited*. The gap between the two is credit still owed. + long sentOffset; // bytes handed to the wire + long ackedOffset; // running sum of WND deltas received + long acceptedOffset; // client -> player bytes taken off the wire + long deliveredOffset; // client -> player bytes actually written to the socket + final UnackedBytes unacked = new UnackedBytes(); + + /** + * Absolute wall-clock deadline for reattaching, fixed at the first park. Not + * re-armed on a later park: a flapping hub would otherwise keep extending it + * and hang the player indefinitely. + */ + long graceDeadline; + long timerId = -1; + + PlayerStream(PendingPlayer p, WorkerConn worker, int sid, int sendWnd) { + this.cid = p.getCid(); + this.cidHex = p.getCidHex(); + this.player = p.getSocket(); + this.pattern = p.getPattern(); + this.playerIp = p.getPlayerIp(); + this.playerPort = p.getPlayerPort(); + this.worker = worker; + this.sid = sid; + this.sendWnd = sendWnd; + } + + /** Whether the player socket should be flowing right now. */ + boolean shouldFlow() { + return !pausedForWindow && !pausedForAggregate && !parked; + } + + /** Roughly how much this stream holds while parked, for the hub-wide cap. */ + int parkedBytes() { + return unacked.length() + (pendingUp != null ? pendingUp.length() : 0); + } +} diff --git a/server/src/main/java/io/icybear/redapricot/Protocol.java b/server/src/main/java/io/icybear/redapricot/Protocol.java index 77519f8..9855941 100644 --- a/server/src/main/java/io/icybear/redapricot/Protocol.java +++ b/server/src/main/java/io/icybear/redapricot/Protocol.java @@ -34,15 +34,37 @@ public final class Protocol { public static final int MUX_WND = 0x04; // per-stream flow-control credit grant public static final int MUX_PING = 0x05; // liveness probe, StreamID 0 public static final int MUX_PONG = 0x06; // liveness reply, echoes the nonce + /** Reattach a parked stream to this conn: CID + the client's accepted offset (§7.5). */ + public static final int MUX_RESUME = 0x07; + /** Hub's answer to RESUME: its accepted offset plus a freshly minted CID. */ + public static final int MUX_RESUME_ACK = 0x08; /** Reserved stream id for connection-scoped mux frames (PING/PONG). Streams start at 1. */ public static final int MUX_CTL_SID = 0; + // RST reason codes (optional trailing byte; absence means "unspecified"). + // Distinguishing them matters for resume: "unknown stream" is terminal, + // "already bound" means a racing attempt won and this one must not tear down. + public static final int RST_UNSPECIFIED = 0x00; + public static final int RST_UNKNOWN_STREAM = 0x01; // CID unknown, expired, or hub restarted + public static final int RST_ALREADY_BOUND = 0x02; // another RESUME won the race + public static final int RST_RESUME_ABANDONED = 0x03; + public static final int RST_FLOW_CONTROL = 0x04; + public static final int RST_DIAL_FAILED = 0x05; + // Session-establishment feature flags (trailing VarInt on the Rekey message, // echoed after the SessionReady type byte when accepted). public static final int FLAG_STREAM_FC = 0x01; /** Mux-level PING/PONG on worker conns, so a silently dropped path is detected. */ public static final int FLAG_WORKER_HEARTBEAT = 0x02; + /** + * Stream resumption (§7.5): on a worker-conn drop the hub hangs the player + * socket instead of closing it, and the client reattaches the stream + * byte-exactly over a fresh conn. When accepted, the hub appends its resume + * grace period to SessionReady so the client can bound its own retry budget + * against it. + */ + public static final int FLAG_STREAM_RESUME = 0x04; // Per-stream flow-control window bounds (bytes). public static final int DEFAULT_STREAM_WINDOW = 256 * 1024; diff --git a/server/src/main/java/io/icybear/redapricot/UnackedBytes.java b/server/src/main/java/io/icybear/redapricot/UnackedBytes.java new file mode 100644 index 0000000..0e21113 --- /dev/null +++ b/server/src/main/java/io/icybear/redapricot/UnackedBytes.java @@ -0,0 +1,95 @@ +package io.icybear.redapricot; + +import io.vertx.core.buffer.Buffer; + +import java.util.ArrayDeque; +import java.util.Deque; + +/** + * The bytes a stream has sent but the client has not yet credited — exactly the + * region a reattach may have to retransmit (PROTOCOL.md §7.5). + * + *

It needs no cap of its own: credit is only granted as bytes reach the + * client's destination socket, so flow control already bounds the outstanding + * region to one window. That is what makes byte-exact resumption affordable. + * + *

A deque of the chunks already materialized by the send path, rather than one + * growing {@link Buffer}: appending to a Buffer reallocates and recopies as it + * grows, which would add a second per-byte copy to the whole upstream path. Here + * retention is free — the chunk was allocated to be sent anyway. + */ +final class UnackedBytes { + private final Deque chunks = new ArrayDeque<>(); + private int head; // bytes of the first chunk already credited + private long base; // stream offset of the first live byte + private int length; // live bytes across all chunks + + int length() { + return length; + } + + long base() { + return base; + } + + /** Offset one past the last byte handed to the wire. */ + long end() { + return base + length; + } + + void append(byte[] chunk) { + if (chunk.length == 0) return; + chunks.addLast(chunk); + length += chunk.length; + } + + /** Drop everything the client has credited up to {@code off}. */ + void advance(long off) { + long drop = off - base; + if (drop <= 0) return; + if (drop > length) drop = length; // only from a peer crediting bytes never sent + while (drop > 0) { + byte[] first = chunks.peekFirst(); + int avail = first.length - head; + int take = (int) Math.min(drop, avail); + head += take; + base += take; + length -= take; + drop -= take; + if (head == first.length) { + chunks.removeFirst(); + head = 0; + } + } + } + + /** + * The outstanding bytes at and after {@code off}, or {@code null} when + * {@code off} falls outside what is still held — which means the peer named + * an offset we can no longer satisfy and the stream cannot be resumed. + */ + Buffer from(long off) { + long skip = off - base; + if (skip < 0 || skip > length) return null; + Buffer out = Buffer.buffer((int) (length - skip)); + int start = head; + for (byte[] chunk : chunks) { + int avail = chunk.length - start; + if (skip >= avail) { + skip -= avail; + start = 0; + continue; + } + out.appendBytes(chunk, start + (int) skip, avail - (int) skip); + skip = 0; + start = 0; + } + return out; + } + + void clear() { + chunks.clear(); + head = 0; + length = 0; + } +} diff --git a/server/src/main/java/io/icybear/redapricot/WorkerConn.java b/server/src/main/java/io/icybear/redapricot/WorkerConn.java index 8010b13..0c30ee3 100644 --- a/server/src/main/java/io/icybear/redapricot/WorkerConn.java +++ b/server/src/main/java/io/icybear/redapricot/WorkerConn.java @@ -4,15 +4,12 @@ import io.icybear.redapricot.net.EncryptedFrames; import io.icybear.redapricot.util.ProtoReader; import io.icybear.redapricot.util.ProtoWriter; import io.vertx.core.buffer.Buffer; -import io.vertx.core.net.NetSocket; import lombok.RequiredArgsConstructor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; /** * An authenticated worker connection (Magic 0x02). Multiplexes many player @@ -34,28 +31,17 @@ public final class WorkerConn { private final String id; private final int sendWndInit; // client's advertised per-stream receive window (our send budget) private final int recvWndInit; // our advertised per-stream receive window (basis for credit grants) + /** + * Whether this conn negotiated stream resumption. Gates the park path: a + * client that will never reattach is better served by an immediate close than + * by a player left hanging for the whole grace period. + */ + private final boolean resume; - private final Map streams = new HashMap<>(); + private final Map streams = new HashMap<>(); - // Aggregate backpressure for the single shared worker socket: players parked - // until its write queue drains. Per-stream fairness is the credit windows' - // job; this only reacts to the whole pipe being congested. - private final Set upstreamPaused = new HashSet<>(); private boolean workerDrainArmed = false; // whether the worker socket's single drainHandler is set - /** Per-stream flow-control bookkeeping. */ - private final class StreamState { - final NetSocket player; - int sendWnd = sendWndInit; // budget for player -> client DATA - Buffer pendingUp; // player bytes awaiting send window (player is paused meanwhile) - boolean pausedForWindow; - int credited; // client -> player bytes flushed but not yet granted back - - StreamState(NetSocket player) { - this.player = player; - } - } - public void onFrame(byte[] payload) { ProtoReader r = new ProtoReader(payload); int type = r.readUByte(); @@ -64,6 +50,8 @@ public final class WorkerConn { case Protocol.MUX_SYN -> handleSyn(sid, r.readBytes(Protocol.CID_LEN)); case Protocol.MUX_DATA -> handleData(sid, r.readBuffer(r.remaining())); case Protocol.MUX_WND -> handleWnd(sid, r.readVarInt()); + case Protocol.MUX_RESUME -> + handleResume(sid, r.readBytes(Protocol.CID_LEN), r.readI64(), r.readI64()); case Protocol.MUX_FIN, Protocol.MUX_RST -> closeStream(sid); case Protocol.MUX_PING -> sendPong(r.readI64()); case Protocol.MUX_PONG -> { /* liveness only; arrival is what matters */ } @@ -76,26 +64,40 @@ public final class WorkerConn { PendingPlayer p = hub.takePending(cid); if (p == null) { LOG.warn("worker {} SYN for unknown CID", id); - sendRst(sid); + sendRst(sid, Protocol.RST_UNKNOWN_STREAM); return; } - NetSocket player = p.getSocket(); - StreamState st = new StreamState(player); + PlayerStream st = new PlayerStream(p, this, sid, sendWndInit); + st.resumable = resume; streams.put(sid, st); + hub.addStream(st); - // From now on the player socket belongs to this stream. - player.handler(buf -> { - sendUpstream(sid, st, buf); - checkAggregate(st); - }); - player.closeHandler(v -> onPlayerGone(sid, st)); - player.exceptionHandler(t -> onPlayerGone(sid, st)); + // From now on the player socket belongs to this stream. The handlers are + // installed once and route through the hub, which dispatches to whichever + // conn currently carries the stream. + // + // They must not call this conn's methods directly: a lambda defined here + // captures `this`, so after the stream moves to another conn it would keep + // writing into the dead one's transport, where sends are silently dropped + // and the player goes mute with nothing logged. Re-installing handlers on + // every reattach would be the other option, but a Vert.x socket resumed + // while a stale handler is still attached loses bytes, so routing beats + // rebinding. + st.player.handler(buf -> hub.onPlayerData(st, buf)); + st.player.closeHandler(v -> hub.onPlayerGone(st)); + st.player.exceptionHandler(t -> hub.onPlayerGone(st)); // Forward the buffered handshake (and any pipelined bytes), then resume. - sendUpstream(sid, st, p.getBuffered()); - if (!st.pausedForWindow) player.resume(); + sendUpstream(st, p.getBuffered()); + maybeResumePlayer(st); + checkAggregate(st); + LOG.info("worker {} stream {} bound to {}", id, sid, st.pattern); + } + + /** Player bytes arrived on a stream this conn currently carries. */ + void playerData(PlayerStream st, Buffer buf) { + sendUpstream(st, buf); checkAggregate(st); - LOG.info("worker {} stream {} bound to {}", id, sid, p.getPattern()); } /** @@ -103,12 +105,12 @@ public final class WorkerConn { * the overflow is parked in {@code pendingUp} and the player socket paused * until the client grants more credit. */ - private void sendUpstream(int sid, StreamState st, Buffer buf) { + private void sendUpstream(PlayerStream st, Buffer buf) { if (st.pendingUp != null) { // still waiting for window; keep ordering st.pendingUp.appendBuffer(buf); return; } - int off = drainUpstream(sid, st, buf, 0); + int off = drainUpstream(st, buf, 0); if (off < buf.length()) { st.pendingUp = buf.getBuffer(off, buf.length()); if (!st.pausedForWindow) { @@ -119,10 +121,20 @@ public final class WorkerConn { } /** Send from {@code buf[off..]} within the stream window, chunked; returns the new offset. */ - private int drainUpstream(int sid, StreamState st, Buffer buf, int off) { + private int drainUpstream(PlayerStream st, Buffer buf, int off) { while (off < buf.length() && st.sendWnd > 0) { int n = Math.min(Math.min(CHUNK, st.sendWnd), buf.length() - off); - sendData(sid, buf.getBytes(off, off + n)); + byte[] chunk = buf.getBytes(off, off + n); + if (st.resumable) { + // Retain before sending. A frame written to a dying socket is + // lost with no notification, so the only trustworthy record of + // what the client still owes us is taken before the attempt. + // Retention is free here: the chunk was materialized to be sent. + st.unacked.advance(st.ackedOffset); + st.unacked.append(chunk); + } + st.sentOffset += n; + sendData(st.sid, chunk); st.sendWnd -= n; off += n; } @@ -131,51 +143,155 @@ public final class WorkerConn { /** The client granted {@code delta} more bytes of credit on a stream. */ private void handleWnd(int sid, int delta) { - StreamState st = streams.get(sid); + PlayerStream st = streams.get(sid); if (st == null || delta <= 0) return; + // The running sum doubles as the acked offset: the client grants credit + // exactly as bytes reach the destination socket, so a credited byte can + // never need retransmitting. + st.ackedOffset += delta; st.sendWnd += delta; if (st.pendingUp != null) { Buffer pending = st.pendingUp; - int off = drainUpstream(sid, st, pending, 0); + int off = drainUpstream(st, pending, 0); st.pendingUp = off >= pending.length() ? null : pending.getBuffer(off, pending.length()); } - if (st.pendingUp == null && st.pausedForWindow) { - st.pausedForWindow = false; - if (!upstreamPaused.contains(st)) st.player.resume(); - } + if (st.pendingUp == null) st.pausedForWindow = false; + maybeResumePlayer(st); checkAggregate(st); } private void handleData(int sid, Buffer data) { - StreamState st = streams.get(sid); + PlayerStream st = streams.get(sid); if (st == null) return; // Never pause the shared socket: the client bounds what it sends per // stream to our advertised window, so a slow player only piles up a // bounded amount in its own write queue; credit is granted back as the // write completes (i.e. the bytes reached the player socket). int len = data.length(); + // Accepted the moment the bytes are taken off the wire, not when the write + // completes. Completion is asynchronous and suppressed once the connection + // closes — precisely when a reattach needs this number to be right — so + // reporting delivery would under-count and make the client replay bytes + // the player already has. + st.acceptedOffset += len; st.player.write(data).onComplete(ar -> { - if (ar.failed() || frames.isClosed() || streams.get(sid) != st) return; + if (ar.failed()) return; + // Both counters advance even if this conn has since died or the stream + // has moved on. Discarding them would destroy up to half a window of + // credit per outage, and — worse — leave the delivered offset that a + // reattach restates the window from permanently short. + st.deliveredOffset += len; st.credited += len; + if (st.worker != this || frames.isClosed()) return; if (st.credited * 2 >= recvWndInit) { int delta = st.credited; st.credited = 0; - sendWnd(sid, delta); + sendWnd(st.sid, delta); } }); } + /** + * Reattach a parked stream to this connection (§7.5). + * + *

Runs to completion in one event-loop turn — rebind, acknowledge, replay — + * so the hub's single-threaded model makes the ordering race-free by + * construction, with no interleaving of live and replayed bytes to reason + * about. + */ + private void handleResume(int sid, byte[] cid, long clientAccepted, long clientDelivered) { + PlayerStream st = hub.takeParked(cid); + if (st == null) { + // Tell a stream we have never heard of apart from one that is still + // bound: the first is terminal for the client, the second means a + // racing attempt won and this one must leave the player alone. + boolean bound = hub.streamByCid(cid) != null; + LOG.warn("worker {} RESUME for {} CID", id, bound ? "still-bound" : "unknown"); + sendRst(sid, bound ? Protocol.RST_ALREADY_BOUND : Protocol.RST_UNKNOWN_STREAM); + return; + } + + // Delivery is a strictly stronger fact than credit — the client only + // credits what it has delivered — so the reported offset can be adopted + // wholesale. Doing so also repairs the ledger: the grants destroyed by the + // outage are exactly the gap between the two, and without this the + // retained region would carry that dead prefix for the stream's whole life. + st.ackedOffset = Math.max(st.ackedOffset, clientDelivered); + st.unacked.advance(st.ackedOffset); + Buffer replay = st.unacked.from(clientAccepted); + if (replay == null) { + LOG.warn("worker {} RESUME at offset {} outside the retained region [{}, {}]; closing player", + id, clientAccepted, st.unacked.base(), st.unacked.end()); + sendRst(sid, Protocol.RST_UNKNOWN_STREAM); + hub.removeStream(st); + st.player.close(); + return; + } + + st.worker = this; + st.sid = sid; + st.resumable = resume; + streams.put(sid, st); + + // Restate the window rather than patching it. Three offsets, three jobs: + // the replay above is measured from what the client *accepted*, the window + // from what it *delivered* (the window being a promise about undelivered + // bytes), and never from what it *credited* — credit travels as deltas, and + // the grants in flight when the connection died are gone for good, so a + // window derived from them stays permanently short. When a full window was + // outstanding at the drop that means permanently zero, which deadlocks: + // nothing can be sent, so no credit can ever come back. + int outstanding = st.unacked.length(); + st.sendWnd = Math.max(0, sendWndInit - outstanding); + // Symmetrically, drop our own pending credit instead of flushing it: the + // delivered offset in the ack already carries everything those deltas + // would have, and sending both would grant the same bytes twice. + st.credited = 0; + + // A fresh capability per reattach keeps a CID single-use, so a leaked one + // never grants more than the outage it was observed in. + byte[] newCid = hub.newCid(); + hub.rekeyStream(st, newCid); + frames.send(new ProtoWriter() + .u8(Protocol.MUX_RESUME_ACK).varInt(sid) + .i64(st.acceptedOffset) + .i64(st.deliveredOffset) + .bytes(newCid) + .toBytes()); + + // Replayed straight to the wire: it must not be re-charged against the + // window or re-appended to the retained region, both of which + // drainUpstream would do. + for (int off = 0; off < replay.length(); off += CHUNK) { + int end = Math.min(off + CHUNK, replay.length()); + sendData(sid, replay.getBytes(off, end)); + } + + if (st.pendingUp != null) { + Buffer pending = st.pendingUp; + int off = drainUpstream(st, pending, 0); + st.pendingUp = off >= pending.length() ? null : pending.getBuffer(off, pending.length()); + } + if (st.pendingUp == null) st.pausedForWindow = false; + maybeResumePlayer(st); + checkAggregate(st); + LOG.info("worker {} stream {} resumed ({} bytes replayed, {} outstanding)", + id, sid, replay.length(), outstanding); + } + private void closeStream(int sid) { - StreamState st = streams.remove(sid); + PlayerStream st = streams.remove(sid); if (st != null) { - upstreamPaused.remove(st); + st.worker = null; + hub.removeStream(st); st.player.close(); } } /** Park the player if the shared worker socket's write queue is congested. */ - private void checkAggregate(StreamState st) { - if (frames.writeQueueFull() && upstreamPaused.add(st)) { + private void checkAggregate(PlayerStream st) { + if (!st.pausedForAggregate && frames.writeQueueFull()) { + st.pausedForAggregate = true; st.player.pause(); armWorkerDrain(); } @@ -187,20 +303,30 @@ public final class WorkerConn { workerDrainArmed = true; frames.socket().drainHandler(v -> { workerDrainArmed = false; - if (upstreamPaused.isEmpty()) return; - StreamState[] parked = upstreamPaused.toArray(new StreamState[0]); - upstreamPaused.clear(); - for (StreamState st : parked) { - if (!st.pausedForWindow) st.player.resume(); + for (PlayerStream st : streams.values()) { + if (!st.pausedForAggregate) continue; + st.pausedForAggregate = false; + maybeResumePlayer(st); } }); } - /** The player side of a stream vanished: drop it from every table, release any backpressure it held, and FIN the peer if still live. */ - private void onPlayerGone(int sid, StreamState st) { - boolean wasLive = streams.remove(sid) == st; - upstreamPaused.remove(st); - if (wasLive) sendFin(sid); + /** + * Resume the player socket if no reason to hold it applies any more. + * + *

The single arbitration point for every pause reason. Vert.x + * {@code pause()} is a flag rather than a counter, so resuming while another + * reason still holds would let bytes through that we have nowhere to put. + */ + private void maybeResumePlayer(PlayerStream st) { + if (st.shouldFlow()) st.player.resume(); + } + + /** The player side of a stream this conn carries vanished: unbind it and FIN the client. */ + void playerGone(PlayerStream st) { + boolean wasLive = streams.remove(st.sid) == st; + st.worker = null; + if (wasLive) sendFin(st.sid); } private void sendData(int sid, byte[] data) { @@ -211,8 +337,9 @@ public final class WorkerConn { frames.send(new ProtoWriter().u8(Protocol.MUX_FIN).varInt(sid).toBytes()); } - private void sendRst(int sid) { - frames.send(new ProtoWriter().u8(Protocol.MUX_RST).varInt(sid).toBytes()); + /** The reason is a trailing byte, optional on the wire; peers that predate it send none. */ + private void sendRst(int sid, int reason) { + frames.send(new ProtoWriter().u8(Protocol.MUX_RST).varInt(sid).u8(reason).toBytes()); } private void sendWnd(int sid, int delta) { @@ -224,10 +351,24 @@ public final class WorkerConn { frames.send(new ProtoWriter().u8(Protocol.MUX_PONG).varInt(Protocol.MUX_CTL_SID).i64(nonce).toBytes()); } + /** + * Only the tunnel leg died. Where the session negotiated resumption the + * player sockets are hung rather than closed, and wait for the client to + * reattach their streams over a fresh conn (§7.5); otherwise this is the old, + * unconditional close. + */ public void onClose() { - for (StreamState st : streams.values()) st.player.close(); + int parked = 0; + for (PlayerStream st : streams.values()) { + st.worker = null; + if (hub.park(st)) { + parked++; + } else { + hub.removeStream(st); + st.player.close(); + } + } + LOG.info("worker {} closed ({} of {} player(s) hung for reattach)", id, parked, streams.size()); streams.clear(); - upstreamPaused.clear(); - LOG.info("worker {} closed", id); } } diff --git a/server/src/test/java/io/icybear/redapricot/CryptoCodecTest.java b/server/src/test/java/io/icybear/redapricot/CryptoCodecTest.java index 11c5632..ebbf100 100644 --- a/server/src/test/java/io/icybear/redapricot/CryptoCodecTest.java +++ b/server/src/test/java/io/icybear/redapricot/CryptoCodecTest.java @@ -78,7 +78,8 @@ class CryptoCodecTest { /** A Hub whose event loop is never touched (register/match/normalize use no Vert.x state). */ private static Hub testHub() { return new Hub(null, new Config("0.0.0.0", 25565, "test-psk", 30_000L, 10_000L, - Protocol.DEFAULT_STREAM_WINDOW, 90_000L)); + Protocol.DEFAULT_STREAM_WINDOW, 90_000L, + true, 20_000L, 256, 256L * 2 * Protocol.DEFAULT_STREAM_WINDOW, 0L, 15_000L)); } private static ControlSession testSession(Hub hub, String id) {