Repository navigation
perf(web): cut the round trips behind slow list-page loads - #390
Merged
Merged
Conversation
The services list sat on a loading skeleton for ~10s. DevTools showed the server taking 2.23s and the browser spending 10.05s in Queueing, so the warehouse was never the problem: bucketing maple-api spans by concurrent load gives a flat p50 (272ms alone, 212ms at 13+ concurrent). Three separate causes, all client-facing round trips. 1. No Access-Control-Max-Age. Without it Chrome falls back to a 5s preflight cache, and since CLOUDFLARE_WORKER_PLACEMENT pins the worker to us-east-1, each expiry costs a full extra ~165ms round trip from Europe before the real request is sent. 2. `Access-Control-Allow-Headers: *` does not cover `Authorization` — the Fetch spec deliberately excludes it. Browsers still allow the request, so nothing looks broken, but they refuse to cache its preflight. Every call to this API is authenticated, so maxAge alone was a no-op. Measured in Chrome, 3 identical authenticated requests to one URL: 3 preflights with `*` alone, 1 with `Authorization` listed. A plain custom header cached fine under `*`, which is what makes this easy to misdiagnose. These OPTIONS are invisible in our own traces because the tracer is disabled for OPTIONS, which is why server-side data looked healthy. 3. Nothing was batched. Every query was its own POST /execute, and the services table fired one GET /vcs/commits/:sha per row — each a distinct URL, so each needed its own preflight too. Changes: - api-cors: set maxAge and list Authorization explicitly. - POST /api/query-engine/execute-batch: warmRoute once for the batch, concurrency 4 (matching the bucket-cache fill concurrency tuned for the Cloudflare six-connection limit), and a wall-clock deadline SHARED by every item so total time is bounded regardless of how many waves run — the response has to land inside the browser's 45s abort. Per-item failures ride in the success payload so one bad widget can't take down the others sharing its request. - Client-side window batcher at the single choke point executeQueryEngine, so no call sites change. A timer window, not queueMicrotask: atoms mount across separate React commits and a microtask can't span one. Not Effect.request/RequestResolver, which batch within a single fiber's graph while these callers are on unrelated fibers. - GET /api/integrations/vcs/commits?shas=... plus bulk repo lookups, so a table resolves in two reads instead of one per row. Unresolvable SHAs are omitted rather than failing the batch, and provider probes for unknown SHAs are capped. Verified against a real stack in Chrome: a full /services load now issues zero OPTIONS (6 API calls, all preflights cached), two queries coalesce into one /execute-batch, and one bulk request resolves the whole table.
Contributor
🍁 Maple PR previewNote Preview resources were removed when this pull request closed. Final commit |
This branch was successfully deployed
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
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.
Why
The services list sat on a loading skeleton for ~10s. The DevTools timing was the giveaway — 2.23s waiting for the server, 10.05s in
Queueing— so most of the wall time was spent before the request was even sent.The warehouse is innocent. Bucketing
maple-apiserver spans for/api/query-engine/*by how many requests landed in the same 5s window (Aug 4–11) gives a flat p50: 272ms alone, 403ms at 4–6 concurrent, 212ms at 13+. Client-side RUM (session_events) agrees. Adding concurrency does not slow the server down.It was three separate client-facing round-trip problems.
1. No
Access-Control-Max-AgeWithout it Chrome falls back to a 5-second preflight cache.
CLOUDFLARE_WORKER_PLACEMENTis{ region: "aws:us-east-1" }(deliberate — it pins the worker next to Postgres), so from Europe a round trip measures ~165ms (cf-placement: remote-IADon a-TXLray). Every expiry cost a full extra transatlantic round trip before the real request went out.2.
Access-Control-Allow-Headers: *does not coverAuthorizationThis is the one that made fix #1 worthless on its own. The Fetch spec deliberately excludes
Authorizationfrom the*wildcard. Browsers still allow the request — so nothing looks broken — but they refuse to cache its preflight. Since every browser call to this API is authenticated,maxAgewas a complete no-op.Measured in Chrome, 3 identical authenticated GETs to one URL:
allowedHeaders["*"]["*", "Authorization"]Control in the same session: a plain
x-testcustom header cached fine under["*"]— which is exactly what makes this easy to misdiagnose.These OPTIONS never reach our traces (
HttpMiddleware.withTracerDisabledWhen()skipsOPTIONS), which is why the server-side data looked healthy while the browser burned its time on round trips we never recorded.3. Nothing was batched
There was no batch endpoint — every query was its own
POST /execute. Andservices-table.tsxfired oneGET /vcs/commits/:shaper row, eagerly on mount, contradicting the arm-on-hover design documented incommit-sha-hover-card.tsx. Each SHA is a distinct URL, so each also needed its own preflight.What changed
api-cors.ts— setmaxAge: 86_400and listAuthorizationexplicitly.POST /api/query-engine/execute-batch—warmRouteonce for the whole batch (the documented ordering fix), concurrency 4 to match the bucket-cache fill concurrency already tuned for the Cloudflare six-connection limit, and a wall-clock deadline shared by every item so total time is bounded no matter how many waves run. Per-item failures ride in the success payload, so one bad widget can't take down the others sharing its request.executeQueryEngine, so no call sites changed. A timer window rather thanqueueMicrotask— atoms mount across separate React commits and a microtask can't span one. Deliberately notEffect.request/RequestResolver: those batch within a single fiber's execution graph, and these callers are on unrelated fibers.GET /api/integrations/vcs/commits?shas=…plus bulk repo lookups, so a table resolves in two DB reads instead of one per row. Unresolvable SHAs are omitted rather than failing the batch; provider probes for unknown SHAs are capped.Verification
Ran a real stack (API + web + electric-sync on spare ports) and drove Chrome:
access-control-max-age: 86400on a live worker/execute-batch, bothoutcome: "success"with live warehouse data/servicesload issuing zero OPTIONS — 6 API calls, every preflight served from cacheTests: 151 api + 58 web passing. Includes a regression guard asserting both
access-control-max-ageand thataccess-control-allow-headerscontainsAuthorization, plus tests for the shared batch deadline (bounded so it discriminates a shared deadline from a per-item timeout) and per-item failure isolation.Reviewer notes
services-table.tsxdiff looks like 342 lines but is 67 real insertions — the rest is re-indentation from wrapping the JSX in the new provider.git diff -wshows the truth./api/billing/customer(p50 92ms / p95 11.2s) and/v2/ingest_keys(p50 16ms / p95 12.7s) — shows a bimodal 11–17s tail uncorrelated with what the endpoint does. That's the org-config Postgres read on the hot path plus the documented dial hang. This PR reduces exposure to that tail by cutting request count, but does not fix it.0b2721314; rebased cleanly ontof68f3ad60. The two perf PRs that landed in between touch no file in this change set.Follow-up: batching added head-of-line blocking (second commit)
Worth calling out, because this PR creates the problem and only bounds it.
Batch delivery is all-or-nothing — the caller gets every result together, so a batch resolves at the speed of its slowest member. Against a per-query p50 of ~250–600ms and a p95 of ~5s, a large batch will often be gated by one tail query. On
/services(2 queries) that's noise; on a dashboard grid it means every widget waits for the worst one. Eleven fast widgets blocked on one slow query is worse than the requests we saved.The second commit holds the client cap (
QUERY_ENGINE_BATCH_MAX) equal to the server's fan-out concurrency (QE_BATCH_CONCURRENCY, 4), so a batch is exactly one wave and can't be gated by a tail query sitting in a later wave. Both constants carry a note to keep them in step.That's a bound, not a fix. The fix is to stream the outcomes as an SSE event stream so each query settles the moment it completes — one round trip, zero extra preflights, and progressive paint, which removes the tradeoff instead of tuning around it. It also improves failure behaviour: today a 45s client abort discards the whole batch, whereas anything already streamed stays delivered.
The pieces exist on both ends — Effect's HttpApi has first-class streaming success (
HttpApiSchema.StreamSse, including a reserved mid-stream failure event) andHttpApiClientdecodes it into an EffectStream. The client batcher is already shaped for it: it deliberately sits outside the atom system holding a map of pending promises, so it only needs to settle them one at a time instead of all at once. That lands as its own PR, after which the cap can rise again.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.