Repository navigation
Conversation
The reconnect log said "replaying from FSN N", where N was fsnAtZero: the next unacked FSN minus any symbol-dictionary catch-up frames. On an idle connection dropped by a load balancer it read like a replay even though nothing was re-sent, and N pointed at an already-acked frame. Log the first unacked FSN and the number of frames actually replayed.
Load balancers, proxies, firewalls, NAT gateways and the server's own idle timeout (http.net.connection.timeout, 5 min) close quiet QWP connections, so idle senders reconnected over and over. Before this, the I/O loop only pinged in durable-ack mode while acks were pending. Add keepalive_interval_millis (default 30s, 0 disables): the I/O loop pings once nothing has gone out for that long. It shares the throttle and send path with the durable-ack keepalive.
…bilityTest Only Sender and its builder are public API. The test pinned internal signatures (QwpWebSocketSender factories, CursorWebSocketSendLoop constructors) whose only callers live in this jar, so they can never link against a different version. Widen the existing factory in place instead of adding an overload.
|
Verdict: approve. Level-3 review of head
Azure coverage build 279451 failed in No matching OSS/Enterprise tandem exists; the new scheduler was not exercised against a real server during this review. Existing server PING handling and tests were inspected. No submodule changes; the primary working tree was unchanged. |
|
The CI failure in |
pendingDurable is emptied on reconnect and refills only after a frame is sent on the new connection, which stamps lastFrameOrPingNanos, so the reset could never make the durable-ack PING fire sooner.
A PING timeout reconnects and replays unacked frames. With 1 s, a GC pause or a slow-draining server could tear down a live connection.
[PR Coverage check]😍 pass : 29 / 31 (93.55%) file detail
|
|
Verdict: approve. Level-3 review of head No Critical, Moderate, or Minor findings. Correctness and test gates pass; zero admitted coverage gaps.
API scope follows the PR's explicit policy that the changed low-level classes are internal. Limitations: no matching OSS/Enterprise tandem or real-server/HA execution. At posting time, Azure's Windows job remains pending; no failed checks are reported. No submodule changes. Primary checkouts unchanged. |
|
Verdict: approve. Level-3 review of head MinorPR description leaves out two behaviour changes
Trade-off (not a finding)The PING timeout is 600 s, not 30 s: the WebSocket client is built with
What was checked
Coverage and scope
|
This branch has not been deployed
2 participants
Idle QWP connections get closed by load balancers, proxies, firewalls, NATs and the server's own idle timeout. The sender reconnects without data loss, but it's noisy and the log wrongly suggests data is being re-sent.
keepalive_interval_millis: after this long with nothing sent, the sender pings. Default 30 s (on by default),0disables.durable_ack_keepalive_interval_millis=0now disables only the durable-ack PING. To stop all PINGs, also setkeepalive_interval_millis=0.replaying from FSN 2→resuming at FSN 3, 0 unacked frames to replay.Server side: kept-alive idle connections now hold their WAL writers indefinitely instead of releasing them when the 5-minute idle reaper closes the connection. Tracked in questdb/questdb#7931.