fix(analytics): stop the dashboard from starving the execution path - #2315
Open
joelorzet wants to merge 9 commits into
Open
fix(analytics): stop the dashboard from starving the execution path#2315joelorzet wants to merge 9 commits into
joelorzet wants to merge 9 commits into
Conversation
The stream is recycled server-side on a fixed lifetime, so the browser sees onerror on a healthy connection every few minutes. The handler treated that close as fatal: it called EventSource.close(), which also cancels the native reconnect, and started the 10s poll. Nothing ever returns to the stream, so every viewer ends up polling within minutes of opening the page. The poll costs far more than the stream. Each tick fires five requests and the next tick aborts whatever is still in flight, so a viewer on a wide range produces 30 requests a minute and discards most of the answers. onerror now reopens the stream with an exponential backoff (1s, 2s, 4s) and falls back to polling only once those attempts are spent. A delivered message resets the counter, so a routine recycle costs one reconnect rather than a permanent demotion. An auth abort clears a pending backoff so a 401 cannot reopen the stream. The decision is a pure function so it can be tested without a DOM.
The poll ran on setInterval, so a tick fired whether or not the previous pass had finished. Every pass starts by aborting the pass before it, so a pass that outlives the 10s interval is cancelled before it can deliver: the runs table and the facet counts never updated, loading never cleared, and the queries behind the abandoned requests kept running to their statement timeout. At a 120s pass that is twelve generations in flight against a single viewer. The poll now rearms on settle rather than on the wall clock, so one pass is in flight at a time and a slow pass simply polls less often. A fast pass is unchanged apart from the milliseconds it takes to return. The scheduler is a standalone helper so the stacking can be tested with fake timers rather than through the hook.
The stream emits an event per run, and each one called fetchData directly. Because every pass aborts the pass before it, an organization that starts runs faster than a refresh completes cancelled every refresh but the last, while still paying on the server for the queries behind all of them. The refresh is now leading-edge debounced over a 2s window: the first event refreshes at once, further events inside the window join one trailing refresh. An idle dashboard is unchanged, which is what a viewer watching for a single run expects, and only a burst is grouped. Grouping cannot lose a run. The refresh reads the whole window rather than a delta, so one pass reflects every event that arrived while it was booked. What a viewer can miss is an intermediate state of a run that changes twice inside the window, never its final state.
The /analytics read path shared lib/db's max:10 pool with everything else these pods do, including the execution writes that record a run's progress. A dashboard fans out five requests per pass and its heaviest are unbounded in the range they scan, so a couple of viewers can hold every connection a pod has. The executor then cannot write progress, the reaper files those runs as system errors at the stale threshold, and a slow page becomes dropped runs. lib/analytics/queries.ts now reads through a dedicated pool at max:3, the same shape the metrics collector already uses for the same reason. Whatever the dashboard does it cannot reach the app pool, so the write path never queues behind a chart. Analytics requests queue instead, which is the intended trade: the page degrades, the platform does not. statement_timeout is 15s rather than the app pool's 30s, overridable with ANALYTICS_STATEMENT_TIMEOUT_MS. The client abandons a pass well before either bound, and an abandoned request keeps its connection until Postgres cancels the statement, so the tighter bound returns it sooner.
Contributor
|
Resolved. Exempt: pull request carries the 'no-issue-required' label. |
Tying the refresh to a stream failure froze the page. The stream emits only summary and heartbeat frames; nothing in the repository emits new-run or run-updated. formatSSE writes a bare data: line with no event name, so a heartbeat reaches onmessage and reset the reconnect counter every 30s, while the lifetime recycle is 5 minutes. The counter never reached its limit, so the fallback never started and fetchData ran once at mount and never again. The runs table, the chart, the network panel and the status counts held their mount-time values for the life of the page, while the header kept showing a recent update time from the summary event. The refresh now starts with the stream and runs whatever the stream is doing, which is what the fallback used to provide once it demoted. The stream carries the summary and nothing else, so a stream that will not reopen costs summary freshness alone. The retry decision says reconnect or stop rather than reconnect or poll, because polling is no longer its business. The reconnect keeps its own value: a routine recycle no longer ends the summary events for the rest of the session.
createLeadingDebounce grouped new-run and run-updated events. Nothing emits those: the stream sends summary and heartbeat only, so the debounce and its call site were both unreachable. It was also wrong for the job it claimed. The window was measured from the start of a pass, not from its settle, so a pass slower than the window still started a second on top of it. The second aborts the first, pendingCount never reaches zero and loading never clears, which is the stacking the poll scheduler was written to avoid. All five of its tests used a task that settled at once, so none of them showed it. Removing it leaves the stream independent of the row filters, so the stream and the refresh now hold separate effects and changing a filter no longer recycles the connection. If per-run events are added later, the grouping has to gate on settle and carry a test with a task slower than its window.
stop() followed by start() while a pass was in flight left two live chains. The pending finally read the boolean as false and armed a timer that overwrote the handle start() had just stored, so the earlier chain kept ticking with nothing holding its handle and stop() could never reach it. Every stop and start now opens a generation, and a pass only rearms while its own generation is current. startPolling builds a fresh scheduler each time so the hook did not reach this, but the function is exported and had no test for it. The test is mutation checked: three calls against the previous implementation, two against this one.
getAnalyticsChecksum ran its three arms through Promise.all, and the stream calls it per open connection every poll interval. On a pool of three that meant one tick of one stream held every connection, so page requests queued behind the streams that were watching them for changes. A queue wait is invisible to statement_timeout, which only starts once a connection is held, so the wait surfaced as an edge timeout with no server-side signal. The arms now run in sequence. Each is an index-only lookup, so serialising costs a few ms of wall clock and turns a tick from three connections into one. The pool goes from three to five, which is the fan-out of a single refresh pass, so one viewer no longer queues against itself. Five per pod still leaves the app pool's ten untouched, and it is well under the forty analytics connections that saturated the database. The test helper now fails when more than one pool matches its size, rather than silently picking whichever came first.
The import aliased analyticsDb to db for the whole module, which moved every function in it onto the analytics pool, not only the dashboard reads. getSpendCapData is not one: it serves the organization limits panel and the MCP spending-limits tool, and it started queuing behind chart traffic on a pool sized for charts. Both pools are now imported under their own names and each read says which one it uses. The dashboard reads take analyticsDb, getSpendCapData keeps db.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
A single analytics dashboard could take every database connection a pod has, and the runs it starved were then filed as platform errors.
The chain, measured on production:
MAX_LIFETIME_MS). The browser seesonerroron a healthy connection, and the handler calledEventSource.close(), which also cancels the native reconnect, then started the 10s poll. Nothing ever returns to the stream, so every viewer ends up polling within minutes of opening the page.setInterval, so a tick fired whether or not the previous pass had finished. Each pass begins by aborting the pass before it, so a pass slower than 10s was cancelled before it could deliver: the runs table never updated andloadingnever cleared.lib/analytics/queries.tsread through the sharedmax:10pool. Four pods gives 40 connections. During the incident all 40 were active and 37 of them were analytics queries, the longest at 1m48s.system_error/infrastructurein batches of 128, 129, 128 and 89 across four passes. That batch write is what paged.One managed organization absorbed 179 of the reaped runs, 50 of them inside the alert window, only because it runs the most workflows. The cause was not specific to it.
What changed
Four commits, one fault each.
Reconnect the stream instead of falling back to polling.
onerrornow reopens with an exponential backoff (1s, 2s, 4s) and polls only once those attempts are spent. A delivered message resets the counter, so a routine recycle costs one reconnect rather than a permanent demotion to 30 requests a minute.Run one fetch at a time. The poll rearms on settle rather than on the wall clock, so one pass is in flight at a time and a slow pass simply polls less often. A fast pass is unchanged.
Group the stream-triggered refreshes. Run events are leading-edge debounced over 2s. An idle dashboard still reacts with no added latency. Grouping cannot lose a run, because the refresh reads the whole window rather than a delta. A viewer can miss an intermediate state of a run that changes twice inside the window, never its final state.
Give the analytics reads their own pool.
max:3with a 15sstatement_timeout, the same shape the metrics collector has used since the 2026-05-29 incident. Whatever the dashboard does it cannot reach the app pool, so the write path never queues behind a chart. Analytics requests queue instead. That trade is deliberate: the page degrades, the platform does not.Why the pool cap matters on its own
#2313 made the gas filter cheap and #2303 bounded the app pool at 30s. Both help. Neither stops a slow analytics query from taking the connections the executor needs, and #2313 notes the endpoint is still slow at 30d because the page selection scans
workflow_executionsthree times. The cap is what makes a slow query a slow page rather than dropped runs.Configuration
ANALYTICS_STATEMENT_TIMEOUT_MSoverrides the 15s bound, alongside the existingAPP_STATEMENT_TIMEOUT_MSandMETRICS_STATEMENT_TIMEOUT_MS. No schema change, no migration, no new index.Not in this PR
phantomrow whenPOST /api/internal/executionstimes out. That is what turns a saturated database into dropped customer runs, and it needs its own ticket.Verification
596 test files, 21,998 tests pass,
pnpm checkandpnpm type-checkclean. New unit tests cover the retry decision, the scheduler under a task slower than its interval, the debounce grouping, and the pool options read off the real module rather than a restated literal.