diff --git a/NOTES.md b/NOTES.md index 99c7f8b..17f1f8e 100644 --- a/NOTES.md +++ b/NOTES.md @@ -71,52 +71,43 @@ and payload masking happen outside the lock. since zat also depends on websocket.zig, a new zat alpha (`v0.3.0-alpha.9`) was cut to resolve the diamond dependency. -## known issue: health probes on port 3000 return 400 +### crash 4: GPF in pingLoop after connection teardown (fixed in `6d6c832`) -**not yet fixed.** this will cause k8s rollout to fail even if the relay itself -is healthy. +**symptom**: GPF in `memcpy → Writer.zig → writeAll → client.zig writeFrame` +every ~60-90s of processing. same stack trace as crash 3 but different root +cause — the write lock from crash 3 was necessary but not sufficient. -### what's happening +**cause**: `pingLoop` runs as an `io.concurrent` task and sleeps in 1s +increments. when `readLoop` returns (connection dies), the defer chain runs: +1. `ping_future.cancel(self.io)` — requests cancellation +2. `client.deinit()` — frees stream/TLS buffers -zlay serves the firehose WebSocket + HTTP API on port 3000 via the websocket -library's `Server`. plain HTTP requests (health probes, XRPC endpoints) are -supposed to reach zlay through an `httpFallback` mechanism: +but `pingLoop` did `self.io.sleep(...) catch {}` — swallowing all errors +including `error.Canceled`. so `cancel()` could not stop the task. `deinit()` +then freed the client while `pingLoop` was still running. next iteration's +`writeFrame(.ping, ...)` hit freed memory → GPF. -``` -main.zig:275 → bc.http_fallback = api.handleHttpRequest - broadcaster.Handler.httpFallback() delegates to it -``` - -but the websocket server never calls `httpFallback`. when a non-upgrade HTTP -request arrives: - -1. `server.zig:1738` — `Handshake.parse()` fails (no websocket headers) -2. `server.zig:1738` — `respondToHandshakeError()` sends `400 missingheaders` -3. connection closed. the app handler never sees the request. +**fix (zlay `6d6c832` + websocket.zig `104608b`)**: +- `io.sleep(...) catch {}` → `catch return` — makes pingLoop + cancellation-cooperative. cancel() can now stop the task before deinit() runs. +- added `Client.isClosed()` check before `writeFrame` — defense-in-depth + against writes to dead connections. -the server code (`server.zig`) has **no code path** that detects "valid HTTP -but not a websocket upgrade" and dispatches to `Handler.httpFallback()`. the -method signature exists on the Handler, the router is fully implemented -(`api/router.zig`), but the wiring inside the websocket library is missing. +### fix 5: httpFallback dispatch for health probes (fixed in websocket.zig `4222f98`) -this worked on zig 0.15 — the old websocket library version had this path. it -was lost during the 0.16 fork migration. +**symptom**: health probes on port 3000 return 400 instead of reaching the +application handler. k8s rollout fails. -### workaround options +**cause**: the websocket server's `_handleHandshake` sent 400 on any +non-upgrade HTTP request. there was no code path to detect "valid HTTP but +not a websocket upgrade" and dispatch to `Handler.httpFallback()`. -1. **switch k8s probes to port 3001** — the MetricsServer (`main.zig:67-142`) - serves `/_healthz` and `/_readyz` directly via `std.http.Server`, no - websocket library involved. the handlers are identical to the router ones. - this is the fastest path to a working deploy. - -2. **fix the websocket server** — add a code path in `server.zig` between - handshake parse failure and error response that checks for valid HTTP, - parses method/url/headers/body, and calls `H.httpFallback()` if it exists - (using `comptime std.meta.hasFn`). this is the correct long-term fix. - -the probes were on port 3000 since initial deploy (commit `e111e47` split them -to `/_healthz` / `/_readyz`). they worked fine on 0.15. the 400 is a 0.16 -regression in the websocket fork. +**fix (websocket.zig `4222f98`)**: intercepts `MissingHeaders`, +`InvalidConnection`, and `InvalidUpgrade` errors from `Handshake.parse()`. +for these (and only these), a standalone `parseHttpRequest()` re-parses the +raw buffer and dispatches to `H.httpFallback()` if it exists (comptime +`hasFn` check). other handshake errors (e.g. `InvalidVersion`) still get 400. +10 tests cover all request patterns. ## where things live @@ -174,11 +165,11 @@ the frame pool workers use `Io.Mutex` / `Io.Condition` for synchronization. under `Io.Threaded`, these use direct kernel futex syscalls — safe from plain threads. under `Io.Evented`, they would segfault (see crash 1). -### dependency versions (current `0f11cfc`) +### dependency versions (current `6d6c832`) ``` -zat v0.3.0-alpha.9 (tangled.org) -websocket.zig 0261b7d (github, master) +zat v0.3.0-alpha.11 (tangled.org) +websocket.zig 104608b (github, master) pg.zig 5ce2355 (github, dev branch) rocksdb-zig cdef67b (github) zig 0.16.0-dev.3059+42e33db9d @@ -201,21 +192,18 @@ zig 0.16.0-dev.3059+42e33db9d ## what needs to happen next -1. **decide on health probes** — either switch k8s probes to port 3001 - (immediate, works today) or fix the websocket server's httpFallback dispatch - (correct but more work). both are valid. check ops repo at - `relay/zlay/deploy/zlay-values.yaml` lines 28-45. - -2. **deploy `0f11cfc`** — once probe issue is addressed. all three crashes are - fixed. native build, linux cross-compile, fmt, tests all pass. +1. **deploy `6d6c832`** — all four crashes are fixed, httpFallback dispatch + is in. health probes on port 3000 should work. native build, linux + cross-compile, fmt all pass. -3. **monitor after deploy** — compare against 0.15 baseline: +2. **monitor after deploy** — compare against 0.15 baseline: - thread count (2,903 on 0.15 — should be similar under Threaded) - memory (24.9 GiB VmSize, 1.44 GiB RSS on 0.15) - throughput, reconnect behavior, ConsumerTooSlow rate - verify no new crashes after extended run (hours, not seconds) + - specifically watch for any remaining GPF — if crash 4 fix is correct, + there should be zero GPFs even after hours of operation -4. **follow-up work** (not blocking deploy): - - fix websocket server httpFallback dispatch for port 3000 HTTP +3. **follow-up work** (not blocking deploy): - investigate Evented backend viability (frame workers → io.concurrent?) - consider upstreaming the client write lock to karlseguin/websocket.zig