Let coalesced live searches hand back their admission slot - #6066
Let coalesced live searches hand back their admission slot#6066backspace wants to merge 4 commits into
Conversation
The search admission gate bounds concurrent requests, but the quantity that holds heap is concurrent computations: a live search the LiveSearchCache resolves by joining an in-flight compute or serving a cached body builds no result document of its own. Counting those requests against the ceiling for their whole lifetime made the gate shed exactly the traffic the cache had made cheap, and left the ceiling tracking something other than heap. The cache now announces its decision through an onOutcome hook the moment it makes it, and the search handler uses that to release the request's admission slot on a join or hit, so only the computing request keeps its slot until its response ends. The middleware leaves the release on the request state for the handler to find. The ceiling's comments now describe it as a bound on computations, sized so a full gate of distinct ones fits a 2 GB heap. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 385c10ebd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Host Test Results 1 files ± 0 1 suites ±0 2h 40m 57s ⏱️ + 10m 27s Results for commit 4866dd5. ± Comparison against earlier commit ba2b5c2. Realm Server Test Results 1 files ± 0 210 suites +2 1h 14m 35s ⏱️ - 3m 54s Results for commit 4866dd5. ± Comparison against earlier commit ba2b5c2. |
…ot-tracks-computes
There was a problem hiding this comment.
🟡 Changes recommended
Indexing requests can be undercounted, and the production route lacks integration coverage for hit/join release.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR lets coalesced live searches release admission slots immediately on cache hits or joins.
Changes:
- Added synchronous cache outcome callbacks.
- Added idempotent early slot release.
- Updated admission documentation and tests.
File summaries
| File | Reviewed changes |
|---|---|
packages/runtime-common/search-bounds.ts |
Clarifies admission ceiling semantics. |
packages/realm-server/tests/search-admission-test.ts |
Tests early slot release behavior. |
packages/realm-server/tests/live-search-cache-test.ts |
Tests synchronous outcome callbacks. |
packages/realm-server/search-inflight.ts |
Documents computation-based admission semantics. |
packages/realm-server/middleware/index.ts |
Exposes and stores early slot release. |
packages/realm-server/live-search-cache.ts |
Implements outcome notifications. |
packages/realm-server/handlers/handle-search.ts |
Releases slots for live cache hits and joins. |
Review details
Suppressed comments (1)
packages/realm-server/middleware/index.ts:271
- [Claude Code 🤖] This stores the early-release hook for requests that
searchAdmissionclassified as indexing. IfDURING_PRERENDER_HEADERis present without a complete job-scoped cache tuple,handle-searchtakes the live-cache branch; a hit/join then calls this hook and decrements an unconditional admission, soinFlightundercounts and distinct searches can exceed the limit. Keep the hook unset forisIndexingrequests (or otherwise make the handler distinguish that lane).
ctxt.state[SEARCH_ADMISSION_RELEASE] = releaseSlot;
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The live search path wrapped the cache's body string in a Response, which encodes it into a stream that setContextResponse then decodes back into a per-request string, so every response to a shared body made two more copies of it before Koa wrote a third. With joiners and hits no longer holding an admission slot, that per-response cost is what bounds a burst of identical searches on the way out, so the handler now hands the shared string to Koa directly and each response costs its write alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The unit tests pin the cache's onOutcome timing and the middleware's release helper separately; this drives the production wiring end to end. A cache whose compute waits on the test holds the first request mid-compute, a second identical request joins it, and the gate's count stays at one while both are in flight. Also notes on the release helper that it applies to indexing-lane admissions for the same reason. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
What this does
A live
_federated-searchthat theLiveSearchCacheresolves by joining an in-flight computation or serving a cached body now hands its search-admission slot back the moment the cache decides so. Only the request that computes keeps its slot until its response ends.LiveSearchCache.getOrPopulatetakes an optionalonOutcomehook, called synchronously when the outcome is decided: as a miss starts its populate, before a joiner begins waiting, and before a hit returns.searchAdmissionleaves the slot's release on the request state, andreleaseSearchAdmission(ctxt)lets a handler hand it back early; it is idempotent and a no-op for requests the gate never saw.joinandhit.SERVER_MAX_IN_FLIGHT_SEARCHESand the gate describe the ceiling as a bound on concurrent computations, sized so a full gate of distinct ones fits a 2 GB heap, rather than as a bound on requests.Why this shape
The admission gate runs before the body is parsed, so it cannot tell a request that will compute from one that will share another's result, and it counted every request for its whole lifetime. With the live-search cache in place, identical concurrent requests cost roughly one result document, so counting each of them made the gate shed exactly the traffic the cache had made cheap, and left the ceiling tracking request concurrency while heap is held by computations.
Releasing the slot at the cache's decision keeps the gate's cheap pre-parse shed and keeps the default ceiling where the heap analysis put it, while letting a burst of identical searches flow through: each joiner occupies a slot only for the milliseconds between admission and the cache lookup, so its release admits the next arrival. In steady state the gate's count is the number of searches assembling their own result document plus the requests briefly in that window, which is the quantity the ceiling was sized for.
Out of scope
Test plan
packages/realm-server/tests/live-search-cache-test.ts:onOutcomereportsmissas the populate starts,joinbefore the joiner has anything to wait on, andhitsynchronously on the call.packages/realm-server/tests/search-admission-test.ts: a handler that releases early drops the in-flight count to zero while its response is still open, a full gate then admits a waiter on the strength of the freed slot, and the release is not repeated when the response ends.🤖 Generated with Claude Code