perf: parallelize startup cache warming to fix slow cold load#170
Open
hongwei1 wants to merge 11 commits into
Open
perf: parallelize startup cache warming to fix slow cold load#170hongwei1 wants to merge 11 commits into
hongwei1 wants to merge 11 commits into
Conversation
- chat.ts: POST /api/opey/stream (same-origin via Vite proxy) instead of calling Opey directly from the browser, which failed on Opey's cross-origin session cookie. Drop the direct browser create-session. - oauth2.ts: store baseUri/version in session.clientConfig so the consent service builds a valid OBP URL (was 'undefined/obp/...'). - OpeyClientService.ts: add createOpeySession() to establish an Opey session with the Consent-JWT and forward its cookie on /stream (Opey rejects /stream without a session). - add @grpc/grpc-js (the Express server fails to start without it).
Previously the worker message that schedules a background docs refresh was posted before reading the cached response. On a cold cache the read throws, the catch block performs the full fetch, and the already-posted message echoes back and runs the same full fetch a second time. Post the message only after the cached response has been read successfully.
Cold-start resource docs fetching awaited one API version at a time, so cold load time scaled linearly with the number of active versions. Add a small bounded-concurrency helper and use it to fetch up to 5 versions in parallel, keeping the same per-version error isolation, response-shape checks, and single cache write at the end.
The serial reduce loops over connectors made message-docs and message-docs-json-schema caching scale linearly with connector count. Replace both with the bounded-concurrency helper (limit 4), keeping the existing error-shape check and grouping logic.
Resource docs, message docs, message docs JSON schema, and the glossary were awaited one after another during app setup even though none of them depend on each other. Fetch all four concurrently with Promise.all, keeping the existing fail-fast behavior where any one failing surfaces the setup error page.
Cover the concurrency ceiling, that every item is processed exactly once, and that one failing task does not prevent the others from running.
hongwei1
force-pushed
the
feature/parallel-startup-cache
branch
from
63cf632 to
25cd823
Compare
runWithConcurrency's own comment promised Promise.allSettled-style isolation, but a rejecting task actually broke its worker's loop and Promise.all(workers) rejected immediately, abandoning any queued items still owned by other workers. Catch each task's error inside the loop so every item is still attempted, and only re-throw the first error once every item has settled.
Unlike resource-docs.ts, message-docs.ts's per-connector fetch had no
try/catch, so one connector returning an error payload (which get()
turns into {error: ...} instead of throwing) tripped getGroupedMessageDocs
on an undefined field and aborted caching for every other connector too.
Wrap each connector's fetch and skip it on failure instead.
Also memoize getConnectors(): cacheDoc and cacheDocJsonSchema each call
it independently, and now that main.ts runs them concurrently the two
calls fire at the same instant instead of staggered sequentially.
…caching cacheOneVersion branched on isDynamicEntity separately for each log line. Compute the 'kind' label once and reuse it, cutting the ternary count without splitting the shared try/catch/concurrency skeleton into two functions.
Two gaps in the Opey session handling added when chat was routed through the backend proxy: - invoke() sent no session cookie at all, even though createOpeySession's own doc comment says both /stream and /invoke need one - only stream() was updated to establish and forward it. - stream() created a brand new Opey session on every single chat message instead of reusing one for the conversation, doubling backend requests and adding a round trip of latency per message. - handleAuthentication() on the client used to prove the session actually worked (by creating it directly) before marking the user authenticated; once that moved server-side, a truthy consent response alone was enough to flip userIsAuthenticated, so a broken session only surfaced later as a generic error on the first chat message. Add getOrCreateSessionCookie(), which reuses a cookie already cached on the config or creates and persists one, and call it from both stream() and invoke(). Call it eagerly from /opey/consent via a new establishSession() so a rejected session fails the request immediately instead of being deferred to the first message.
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.
Summary
Cold loads of API Explorer (reported: 60s+ with "0 endpoints" and a long spinner) block
app.mount()until resource docs for every active API version, all message docs, and all message docs JSON schemas are fetched sequentially — oneawaitper version/connector. With 20-30+ active versions at roughly 0.8-1.2s of doc-generation time each, the cold path scales linearly with version count.On top of that, a pre-existing bug doubled the cost:
resource-docs.ts/message-docs.tscache()posted aworker.postMessage(...)background-refresh message before reading the cached response. On a cold cache the read throws, the catch block runs the full fetch, and the already-posted message echoes back through the worker (a pure echo, seepublic/js/worker/web-worker.js) and triggers the exact same full fetch a second time.Changes
worker.postMessage(...)to after the successful cache read in all threecache()/cacheJsonSchema()functions, so only a cache hit schedules a background re-warm.Promise.allSettled: added a smallrunWithConcurrency(items, limit, task)helper and used it to fetch resource docs (limit 5) and message docs / JSON schemas (limit 4) in parallel. A nakedallSettledover 20-30+ items would fire that many concurrent, CPU-expensive doc-generation requests at the OBP backend with no ceiling anywhere in the chain (Express proxy → undici has no connection cap; the browser multiplexes over h2 unbounded). A bounded pool keeps worst-case concurrency (5 + 4 + 4 = 13) reasonable while still cutting cold-load time from sum-of-all-versions to roughlyceil(N/limit) × avg-latency.setupDatanow fetches all four withPromise.all(kept intentionally, notallSettled— any one failing should still surface the existing setup-error page).Verification
npm testgreen (pre-existing unrelated failures inOBPConsentsService/OpeyClientService/ChatWidgettests are present ondeveloptoo, unaffected by this change).Follow-up fixes (after review)
A round of review turned up a few bugs, some introduced by this PR and some in the
Opey backend-proxy work already sitting on this branch:
runWithConcurrency's own comment promisedPromise.allSettled-style isolation, buta rejecting task actually broke its worker's loop and the whole pool rejected
immediately, abandoning items still queued for other workers. It now keeps draining
the queue and only re-throws the first error after everything has settled.
message-docs.ts's per-connector fetch had no try/catch (unlikeresource-docs.ts),so one connector returning an error payload could abort caching for every other
connector and fail the whole app startup. Wrapped and isolated per connector.
cacheDoc/cacheDocJsonSchemaeach callgetConnectors()independently; now thatthey run concurrently the two calls fired at the same instant. Memoized so they share
one in-flight request.
isDynamicEntityternary branching inresource-docs.ts'scacheOneVersion.OpeyClientService.invoke()never sent a session cookie even though/streamwasupdated to, despite
createOpeySession's own doc comment saying both endpoints needone.
stream()also created a brand-new Opey session on every chat message instead ofreusing one per conversation. Added
getOrCreateSessionCookie(), used by bothstream()andinvoke(), and anestablishSession()called eagerly from/opey/consentso a broken session fails at login time instead of surfacing as ageneric error on the user's first chat message.
Out of scope (noted for follow-up)
cache()'s return semantics and requires a "not ready yet" state in the version-switch UI.superagentrequests with a response timeout — today a hung request stalls the whole cache-warm step (unchanged behavior, pre-existing, just no longer masked by parallelism since a hang would only stall one lane's pool worker rather than the whole page).