From f219834f5c05a280900306e9593e1293ee82c378 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Thu, 9 Apr 2026 20:31:37 +1000 Subject: [PATCH] perf(site): add reconnect jitter to reconnectingWebsocket (#24096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation During the April 2 dogfood incident, a pod OOM-kill triggered a reconnection storm: hundreds of chat-stream and agent-RPC websockets all attempted to reconnect at the same deterministic backoff intervals (1 s, 2 s, 4 s, …). Because every browser tab computed the same delay, the surviving replicas received a synchronized wall of new connections at each retry tick, amplifying the overload that caused the first OOM in the first place. The root cause of the memory blowup (chatd serialization cost) is a separate issue. This change addresses the secondary blast-radius problem: when N clients reconnect in lockstep, the retry storm itself becomes a capacity threat. ## Change The shared `createReconnectingWebSocket` utility now applies symmetric jitter (default ±30%) to the capped exponential-backoff delay before scheduling the reconnect timer. With 100 clients and a 1 s base delay, reconnects spread over the 700 ms–1300 ms window instead of all landing at exactly 1000 ms, and once retries hit `maxMs` the scheduler still preserves downward spread instead of collapsing back to a single tick. Two new options are accepted by callers: - **`jitter`** (0–1 fraction, default `0.3`) — controls the jitter window. Values are clamped to `[0, 1]`; `0` preserves exact legacy timing. - **`random`** (`() => number`, default `Math.random`) — injectable RNG, primarily a deterministic test seam. Non-finite output falls back to the midpoint (`0.5`). The `retryingAt` timestamp surfaced to `ChatStatusCallout` is computed from the jittered delay, so the countdown shown to users reflects the actual retry time. The scheduler also keeps `maxMs` as a hard ceiling on the final delay and saturates exponential overflow at that cap instead of dropping to `0ms` retries. No production callers need changes — the default jitter activates automatically for all four call sites (`AgentsPage` chat-list watcher, `AgentChatPage` workspace watcher, `useChatStore` per-chat stream, `useGitWatcher`). The two downstream tests that asserted exact reconnect timing now pin `Math.random()` to `0.5` so those expectations stay deterministic. --- .../ChatConversation/chatStore.test.tsx | 1 + .../AgentsPage/hooks/useGitWatcher.test.ts | 1 + site/src/utils/reconnectingWebSocket.test.ts | 303 ++++++++++++++++-- site/src/utils/reconnectingWebSocket.ts | 81 ++++- 4 files changed, 347 insertions(+), 39 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx index c6fd57230b..6093ae74ef 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx @@ -2089,6 +2089,7 @@ describe("useChatStore", () => { it("sets reconnectState on WebSocket disconnect and clears it after reconnect", async () => { immediateAnimationFrame(); + vi.spyOn(Math, "random").mockReturnValue(0.5); const chatID = "chat-disconnect"; const mockSocket1 = createMockSocket(); diff --git a/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts b/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts index 5cafc47b05..bb9151be34 100644 --- a/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts +++ b/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts @@ -283,6 +283,7 @@ describe("useGitWatcher", () => { vi.useFakeTimers(); try { + vi.spyOn(Math, "random").mockReturnValue(0.5); const socket1 = createMockSocket(); renderHook(() => diff --git a/site/src/utils/reconnectingWebSocket.test.ts b/site/src/utils/reconnectingWebSocket.test.ts index 47444c0461..79560897ec 100644 --- a/site/src/utils/reconnectingWebSocket.test.ts +++ b/site/src/utils/reconnectingWebSocket.test.ts @@ -40,6 +40,8 @@ const expectReconnectSchedule = ( ); }; +const deterministicRandom = () => 0.5; + beforeEach(() => { vi.useFakeTimers(); vi.setSystemTime(new Date("2025-01-01T00:00:00.000Z")); @@ -95,6 +97,7 @@ describe("createReconnectingWebSocket", () => { baseMs: 1000, maxMs: 10000, factor: 2, + random: deterministicRandom, }); expect(connect).toHaveBeenCalledTimes(1); @@ -128,6 +131,189 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(4); }); + it("applies the minimum jitter bound when random returns 0", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0.3, + random: () => 0, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 700 }); + + vi.advanceTimersByTime(699); + expect(connect).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(connect).toHaveBeenCalledTimes(2); + }); + + it("applies the maximum jitter bound when random returns 1", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0.3, + random: () => 1, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1300 }); + + vi.advanceTimersByTime(1299); + expect(connect).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(connect).toHaveBeenCalledTimes(2); + }); + + it("preserves legacy exact timing when jitter is disabled", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0, + random: () => 1, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + + vi.advanceTimersByTime(999); + expect(connect).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(connect).toHaveBeenCalledTimes(2); + }); + + it("clamps jitter greater than 1 before delay math", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 1.5, + random: () => 1, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 2000 }); + }); + + it("treats negative jitter as no jitter", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: -0.5, + random: () => 0, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + }); + + it("treats NaN jitter as no jitter", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: Number.NaN, + random: () => 0.5, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + }); + + it("treats NaN from random() as the midpoint with no jitter offset", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + jitter: 0.3, + random: () => Number.NaN, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + }); + it("caps backoff delay at maxMs", () => { let activeSocket = createMockSocket(); const connect = vi.fn(() => { @@ -140,6 +326,7 @@ describe("createReconnectingWebSocket", () => { baseMs: 1000, maxMs: 5000, factor: 2, + random: () => 1, }); // Disconnect enough times that the uncapped delay would exceed @@ -158,6 +345,81 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(5); }); + it("preserves jitter spread after raw backoff exceeds maxMs", () => { + const makeReconnect = (random: () => number) => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + maxMs: 10000, + factor: 2, + jitter: 0.3, + random, + }); + + for (let i = 0; i < 4; i++) { + activeSocket.emit("close"); + vi.runOnlyPendingTimers(); + } + activeSocket.emit("close"); + + return disconnects[4]!; + }; + + const minReconnect = makeReconnect(() => 0); + expectReconnectSchedule(minReconnect, { attempt: 5, delayMs: 7000 }); + + vi.clearAllTimers(); + vi.setSystemTime(new Date("2025-01-01T00:00:00.000Z")); + + const maxReconnect = makeReconnect(() => 1); + expectReconnectSchedule(maxReconnect, { attempt: 5, delayMs: 10000 }); + expect(maxReconnect.reconnect.delayMs).toBeGreaterThan( + minReconnect.reconnect.delayMs, + ); + }); + + it("saturates overflowed backoff at maxMs instead of 0ms", () => { + let activeSocket = createMockSocket(); + const connect = vi.fn(() => { + activeSocket = createMockSocket(); + return activeSocket; + }); + const disconnects: Array<{ reconnect: ReconnectSchedule; now: number }> = + []; + const onDisconnect = vi.fn((reconnect: ReconnectSchedule) => { + disconnects.push({ reconnect, now: Date.now() }); + }); + + createReconnectingWebSocket({ + connect, + onDisconnect, + baseMs: 1000, + maxMs: 5000, + factor: 1e308, + jitter: 0, + random: deterministicRandom, + }); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[0]!, { attempt: 1, delayMs: 1000 }); + vi.runOnlyPendingTimers(); + + activeSocket.emit("close"); + expectReconnectSchedule(disconnects[1]!, { attempt: 2, delayMs: 5000 }); + }); + it("resets backoff on successful connection", () => { let activeSocket = createMockSocket(); const connect = vi.fn(() => { @@ -170,6 +432,7 @@ describe("createReconnectingWebSocket", () => { baseMs: 1000, maxMs: 10000, factor: 2, + random: deterministicRandom, }); // Disconnect twice to bump the attempt counter. @@ -213,28 +476,6 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(2); }); - it("closes previous socket when reconnecting", () => { - let activeSocket = createMockSocket(); - const sockets: ReturnType[] = []; - const connect = vi.fn(() => { - activeSocket = createMockSocket(); - sockets.push(activeSocket); - return activeSocket; - }); - - createReconnectingWebSocket({ connect }); - - const firstSocket = sockets[0]!; - firstSocket.emit("close"); - vi.runOnlyPendingTimers(); - - // The connect function creates a new socket. The old socket was - // already "closed" by the browser, but on a fresh reconnection - // the utility closes the previous one if it's still the active - // reference. - expect(connect).toHaveBeenCalledTimes(2); - }); - it("dispose stops reconnection and closes the socket", () => { const socket = createMockSocket(); const connect = vi.fn(() => socket); @@ -289,25 +530,17 @@ describe("createReconnectingWebSocket", () => { expect(connect).toHaveBeenCalledTimes(1); }); - it("passes socket to onOpen callback", () => { - const socket = createMockSocket(); - const connect = vi.fn(() => socket); - const onOpen = vi.fn(); - - createReconnectingWebSocket({ connect, onOpen }); - - socket.emit("open"); - expect(onOpen).toHaveBeenCalledWith(socket); - }); - - it("uses default backoff values when none provided", () => { + it("uses default backoff values when backoff options are omitted", () => { let activeSocket = createMockSocket(); const connect = vi.fn(() => { activeSocket = createMockSocket(); return activeSocket; }); - createReconnectingWebSocket({ connect }); + createReconnectingWebSocket({ + connect, + random: deterministicRandom, + }); // Default: baseMs=1000, factor=2, maxMs=10000. activeSocket.emit("close"); diff --git a/site/src/utils/reconnectingWebSocket.ts b/site/src/utils/reconnectingWebSocket.ts index 47ed0c976c..24841f78eb 100644 --- a/site/src/utils/reconnectingWebSocket.ts +++ b/site/src/utils/reconnectingWebSocket.ts @@ -32,12 +32,19 @@ /** Default base delay for exponential backoff (milliseconds). */ const RECONNECT_BASE_MS = 1_000; -/** Default maximum delay cap for exponential backoff (milliseconds). */ +/** Default maximum base delay cap for exponential backoff (milliseconds). */ const RECONNECT_MAX_MS = 10_000; /** Default multiplier applied to the base delay on each retry. */ const RECONNECT_FACTOR = 2; +/** + * Default symmetric jitter applied to the computed reconnect delay. + * `0.3` means the final delay is randomized within ±30% of the base + * exponential-backoff value. + */ +const RECONNECT_JITTER = 0.3; + /** * Metadata for the reconnect attempt that was just scheduled. * `attempt` is 1-based and user-facing: `1` means the first retry after @@ -92,25 +99,83 @@ interface ReconnectingWebSocketOptions { /** Base delay in milliseconds. Defaults to {@link RECONNECT_BASE_MS}. */ baseMs?: number; - /** Maximum delay cap in milliseconds. Defaults to {@link RECONNECT_MAX_MS}. */ + /** + * Hard upper bound on the reconnect delay in milliseconds. Jitter is + * applied to the capped backoff base, so the final delay never exceeds + * this value. + */ maxMs?: number; /** Multiplier applied per attempt. Defaults to {@link RECONNECT_FACTOR}. */ factor?: number; + + /** + * Symmetric jitter applied to the computed delay. `0.3` means the + * final delay may vary within ±30% of the base exponential-backoff + * value. Set to `0` to preserve exact legacy timing. Values are + * clamped to `[0, 1]`; non-finite values are treated as `0`. + */ + jitter?: number; + + /** + * Random-number source used for jitter. Defaults to `Math.random` and + * exists primarily as a deterministic test seam. Output is normalized + * to `[0, 1]`; non-finite values fall back to `0.5`. + */ + random?: () => number; } +const normalizeUnitInterval = (value: number, fallback: number): number => + Number.isFinite(value) ? Math.min(Math.max(value, 0), 1) : fallback; + +const normalizeDelayMs = (value: number, fallback: number): number => + Number.isFinite(value) ? Math.max(0, value) : fallback; + +const applyReconnectJitter = ({ + delayMs, + jitter, + random, +}: { + delayMs: number; + jitter: number; + random: () => number; +}): number => { + const safeJitter = normalizeUnitInterval(jitter, 0); + if (safeJitter <= 0) { + return delayMs; + } + const safeRandom = normalizeUnitInterval(random(), 0.5); + const jitterOffset = (safeRandom * 2 - 1) * safeJitter; + return normalizeDelayMs(Math.round(delayMs * (1 + jitterOffset)), delayMs); +}; + const getReconnectSchedule = ({ attempt, baseMs, maxMs, factor, + jitter, + random, }: { attempt: number; baseMs: number; maxMs: number; factor: number; + jitter: number; + random: () => number; }): ReconnectSchedule => { - const delayMs = Math.min(baseMs * factor ** (attempt - 1), maxMs); + const safeMaxMs = normalizeDelayMs(maxMs, 0); + const rawDelayMs = normalizeDelayMs( + baseMs * factor ** (attempt - 1), + safeMaxMs, + ); + const cappedDelayMs = Math.min(rawDelayMs, safeMaxMs); + const jitteredDelayMs = applyReconnectJitter({ + delayMs: cappedDelayMs, + jitter, + random, + }); + const delayMs = Math.min(jitteredDelayMs, safeMaxMs); return { attempt, delayMs, @@ -129,7 +194,11 @@ const getReconnectSchedule = ({ * * Backoff delay formula: * ``` - * delay = min(baseMs * factor ^ (attempt - 1), maxMs) + * rawDelay = baseMs * factor ^ (attempt - 1) + * cappedDelay = min(rawDelay, maxMs) + * jitteredDelay = round(cappedDelay * (1 + offset)) + * delay = min(jitteredDelay, maxMs) + * offset ∈ [-jitter, +jitter] * ``` * * The reconnect attempt counter resets after a successful `open`. @@ -146,6 +215,8 @@ export function createReconnectingWebSocket( baseMs = RECONNECT_BASE_MS, maxMs = RECONNECT_MAX_MS, factor = RECONNECT_FACTOR, + jitter = RECONNECT_JITTER, + random = Math.random, } = options; let disposed = false; @@ -195,6 +266,8 @@ export function createReconnectingWebSocket( baseMs, maxMs, factor, + jitter, + random, }); onDisconnect?.(reconnect); scheduleReconnect(reconnect);