Merge pull request #5128 from cardstack/cs-11393-skip-loadlinks-prerender-searches

Skip loadLinks for prerender-issued searches; remove dead per-instance cache

Author: habdelra

.claude/skills/indexing-diagnostics/SKILL.md

@@ -812,35 +812,30 @@ It is **not** persisted in `boxel_index.diagnostics` (that's the client side). S
 **The lines.**
 
 ```
-corr=<id> job=<jobId> handler=Nms parse=… resolveRealms=… sql=… loadLinks=… stringify=… coalescedWait=… | busyMs(parallel-sum) populate=… cacheRead=… cacheWrite=… | results=… cacheHit=… cacheMiss=…    (realm:search-timing)
+corr=<id> job=<jobId> handler=Nms parse=… resolveRealms=… sql=… stringify=… coalescedWait=… | results=…    (realm:search-timing)
 --> QUERY <accept> <url>: 200 [job: <jobId>] corr=<id> dur=Nms                                                                                            (realm:requests)
 eventLoopLagMs(mean/p99/max)=…/…/… inFlightSearch=… heapMB=…                                                                                              (realm:health)
 ```
 
-The first `|`-section is the **sequential wall-clock timeline** (these sum to ≈ `handler`). The `busyMs(parallel-sum)` section is the inner per-result work that runs **concurrently** inside `loadLinks`'s `Promise.all`, so it is summed across N parallel ops and is **NOT wall-clock** — `populate` can read as millions of ms on a large result set. Read it as a **ratio** (which sub-step dominates within `loadLinks`) or divide by `results` for a per-item average; never add it to the timeline.
+The `|`-section is the **sequential wall-clock timeline** (these sum to ≈ `handler`).
+
+**Note — a prerender search skips `loadLinks`.** These lines emit only for prerender `_federated-search` (the correlation id is prerender-gated), and a prerender search skips the relationship-assembly pass: the host re-resolves every result from card+source and reads only `data[].id`, so the realm-server returns each result's pristine row (id + attributes + any static-link relationships) + page meta and does not run `populateQueryFields`. The line therefore carries **no `loadLinks` stage and no `busyMs(parallel-sum)` section** (no `populate` / `cacheRead` / `cacheWrite` / `cacheHit` / `cacheMiss`) — those are the per-result umbrella assembly + per-instance wire-format cache, which do not run on this path. The dominant cost on a prerender search is `sql` (or `stringify` for a fat result set, or queue-wait — see branch 4).
 
 Wall-clock timeline stages:
 
 - `parse` — request body → Query parse.
 - `resolveRealms` — federated realm resolution / lazy-mount.
-- `sql` — the `IndexQueryEngine.searchCards` query (the actual SQL).
-- `loadLinks` — the post-SQL relationship-assembly pass over the whole result set (the wall-clock of the parallel `populate`/cache work below).
-- `stringify` — `JSON.stringify` of the response wire-format.
-- `coalescedWait` — this request coalesced onto an already-in-flight identical search (CS-11121 dedup) and waited for it; the real sql/loadLinks work is on the **leader's** line, not this one.
+- `sql` — the `IndexQueryEngine.searchCards` query (the actual SQL). For a prerender search this is essentially the whole `handler`.
+- `stringify` — `JSON.stringify` of the response wire-format (for a prerender search, the pristine result rows + page meta, with no query-field umbrellas or `included[]`).
+- `coalescedWait` — this request coalesced onto an already-in-flight identical search (in-flight dedup) and waited for it; the real sql work is on the **leader's** line, not this one.
 - `handler=` — handler entry → response assembled (≈ the sum of the wall-clock stages).
 
-Busy-time (parallel-sum, NOT wall-clock):
-
-- `populate` — the per-instance query-field assembly (definition lookup + field-tree walk), summed across all results.
-- `cacheRead` / `cacheWrite` (+ `cacheHit` / `cacheMiss` counters) — the per-instance wire-format cache (`job_scoped_instance_cache`) round-trips, summed across all results.
-
 **Reading it — the decision tree.**
 
-1. **`sql=` is the bulk of `handler`** → a genuinely slow query. Rare here (the premise is fast SQL); confirm with `pg_stat_activity` / `EXPLAIN`. Fast SQL but large `sql=` means lock / connection-pool wait inside the query call.
-2. **`loadLinks` dominates `handler`** → the post-SQL relationship/query-field assembly is the cost (the per-instance wire-format work). Use the `busyMs(parallel-sum)` ratio to see what _inside_ loadLinks dominates: `populate ≫ cacheRead/cacheWrite` means it's the CPU field-walk, not the cache DB round-trips. Check `cacheHit` / `cacheMiss`: a low hit rate means the instance cache isn't helping (e.g. unique-per-card queries) so every result is assembled from scratch.
-3. **`stringify` dominates** → serializing a large federated response. Look at `results=` for a fat result set.
-4. **`dur` (realm:requests) ≫ `handler` (realm:search-timing)** → the time is NOT inside the handler. It was spent **queued before the handler ran** (or sending). This is the saturation fingerprint: cross-reference `realm:health` near the same timestamp — if `eventLoopLagMs` spiked into the hundreds/thousands with `inFlightSearch` high, the single-threaded realm-server's event loop was starved (synchronous post-SQL serialization across many concurrent searches), so the request sat unserviced even though, once it ran, the handler was fast. That is the CS-10820 saturation class seen from the server side.
-5. **`coalescedWait` dominates** → this follower waited on another in-flight identical search. Find the leader (same job + query, overlapping time); its line carries the real breakdown.
+1. **`sql=` is the bulk of `handler`** → a genuinely slow query. Confirm with `pg_stat_activity` / `EXPLAIN`. Fast SQL but large `sql=` means lock / connection-pool wait inside the query call. (For a prerender search this is the typical shape — the post-SQL assembly doesn't run on that path, so `sql` carries most of the handler.)
+2. **`stringify` dominates** → serializing a large federated response. Look at `results=` for a fat result set.
+3. **`dur` (realm:requests) ≫ `handler` (realm:search-timing)** → the time is NOT inside the handler. It was spent **queued before the handler ran** (or sending). This is the saturation fingerprint: cross-reference `realm:health` near the same timestamp — if `eventLoopLagMs` spiked into the hundreds/thousands with `inFlightSearch` high, the single-threaded realm-server's event loop was starved, so the request sat unserviced even though, once it ran, the handler was fast. That is the CS-10820 saturation class seen from the server side.
+4. **`coalescedWait` dominates** → this follower waited on another in-flight identical search. Find the leader (same job + query, overlapping time); its line carries the real breakdown.
 
 **Getting the logs.** These emit from the **realm-server** process (`handle-search` → `searchRealms`). Reach them with the `tail-logs` skill (Loki, realm-server family) or, for staging/prod CloudWatch, the `aws-access` skill:
 

packages/host/config/schema/1780627635271_schema.sql

@@ -0,0 +1,180 @@
+-- This is auto-generated by packages/realm-server/scripts/convert-to-sqlite.ts
+-- Please don't directly modify this file
+
+ CREATE TABLE IF NOT EXISTS bot_commands (
+   id NOT NULL,
+   bot_id NOT NULL,
+   command TEXT NOT NULL,
+   command_filter BLOB NOT NULL,
+   created_at NOT NULL,
+   PRIMARY KEY ( id ) 
+);
+
+ CREATE TABLE IF NOT EXISTS bot_registrations (
+   id NOT NULL,
+   username TEXT NOT NULL,
+   created_at NOT NULL,
+   PRIMARY KEY ( id ) 
+);
+
+ CREATE TABLE IF NOT EXISTS boxel_index (
+   url TEXT NOT NULL,
+   file_alias TEXT NOT NULL,
+   type TEXT NOT NULL,
+   realm_version INTEGER NOT NULL,
+   realm_url TEXT NOT NULL,
+   pristine_doc BLOB,
+   search_doc BLOB,
+   error_doc BLOB,
+   deps BLOB DEFAULT '[]',
+   types BLOB,
+   isolated_html TEXT,
+   indexed_at,
+   is_deleted BOOLEAN,
+   last_modified,
+   embedded_html BLOB,
+   atom_html TEXT,
+   fitted_html BLOB,
+   display_names BLOB,
+   resource_created_at,
+   icon_html TEXT,
+   head_html TEXT,
+   has_error BOOLEAN DEFAULT false NOT NULL,
+   last_known_good_deps BLOB,
+   markdown TEXT,
+   diagnostics BLOB,
+   PRIMARY KEY ( url, realm_url, type ) 
+);
+
+ CREATE TABLE IF NOT EXISTS boxel_index_working (
+   url TEXT NOT NULL,
+   file_alias TEXT NOT NULL,
+   type TEXT NOT NULL,
+   realm_version INTEGER NOT NULL,
+   realm_url TEXT NOT NULL,
+   pristine_doc BLOB,
+   search_doc BLOB,
+   error_doc BLOB,
+   deps BLOB DEFAULT '[]',
+   types BLOB,
+   icon_html TEXT,
+   isolated_html TEXT,
+   indexed_at,
+   is_deleted BOOLEAN,
+   last_modified,
+   embedded_html BLOB,
+   atom_html TEXT,
+   fitted_html BLOB,
+   display_names BLOB,
+   resource_created_at,
+   head_html TEXT,
+   has_error BOOLEAN DEFAULT false NOT NULL,
+   last_known_good_deps BLOB,
+   markdown TEXT,
+   diagnostics BLOB,
+   job_id INTEGER,
+   PRIMARY KEY ( url, realm_url, type ) 
+);
+
+ CREATE TABLE IF NOT EXISTS incoming_webhooks (
+   id NOT NULL,
+   username TEXT NOT NULL,
+   webhook_path TEXT NOT NULL,
+   verification_type TEXT NOT NULL,
+   verification_config BLOB NOT NULL,
+   signing_secret TEXT NOT NULL,
+   created_at NOT NULL,
+   updated_at NOT NULL,
+   PRIMARY KEY ( id ) 
+);
+
+ CREATE TABLE IF NOT EXISTS module_transpile_cache (
+   realm_url TEXT NOT NULL,
+   canonical_path TEXT NOT NULL,
+   body TEXT,
+   headers BLOB,
+   dependency_keys BLOB,
+   generation DEFAULT 0 NOT NULL,
+   created_at,
+   PRIMARY KEY ( realm_url, canonical_path ) 
+);
+
+ CREATE TABLE IF NOT EXISTS modules (
+   url TEXT NOT NULL,
+   cache_scope TEXT NOT NULL,
+   auth_user_id TEXT NOT NULL,
+   resolved_realm_url TEXT NOT NULL,
+   definitions BLOB,
+   deps BLOB,
+   error_doc BLOB,
+   created_at,
+   file_alias TEXT,
+   url_hash TEXT GENERATED ALWAYS AS (url) STORED NOT NULL,
+   diagnostics BLOB,
+   PRIMARY KEY ( url, cache_scope, auth_user_id ) 
+);
+
+ CREATE TABLE IF NOT EXISTS realm_file_meta (
+   realm_url TEXT NOT NULL,
+   file_path TEXT NOT NULL,
+   created_at INTEGER NOT NULL,
+   content_hash TEXT,
+   content_size INTEGER,
+   PRIMARY KEY ( realm_url, file_path ) 
+);
+
+ CREATE TABLE IF NOT EXISTS realm_meta (
+   realm_url TEXT NOT NULL,
+   realm_version INTEGER NOT NULL,
+   value BLOB NOT NULL,
+   indexed_at,
+   PRIMARY KEY ( realm_url, realm_version ) 
+);
+
+ CREATE TABLE IF NOT EXISTS realm_metadata (
+   url TEXT NOT NULL,
+   show_as_catalog BOOLEAN,
+   publishable BOOLEAN,
+   created_at DEFAULT CURRENT_TIMESTAMP NOT NULL,
+   updated_at DEFAULT CURRENT_TIMESTAMP NOT NULL,
+   PRIMARY KEY ( url ) 
+);
+
+ CREATE TABLE IF NOT EXISTS realm_registry (
+   id DEFAULT (hex(randomblob(16))) NOT NULL,
+   url TEXT NOT NULL,
+   kind TEXT NOT NULL,
+   disk_id TEXT NOT NULL,
+   owner_username TEXT NOT NULL,
+   source_url TEXT,
+   last_published_at,
+   pinned BOOLEAN DEFAULT false NOT NULL,
+   created_at DEFAULT CURRENT_TIMESTAMP NOT NULL,
+   updated_at DEFAULT CURRENT_TIMESTAMP NOT NULL,
+   PRIMARY KEY ( id ) 
+);
+
+ CREATE TABLE IF NOT EXISTS realm_user_permissions (
+   realm_url TEXT NOT NULL,
+   username TEXT NOT NULL,
+   read BOOLEAN NOT NULL,
+   write BOOLEAN NOT NULL,
+   realm_owner BOOLEAN DEFAULT false NOT NULL,
+   PRIMARY KEY ( realm_url, username ) 
+);
+
+ CREATE TABLE IF NOT EXISTS realm_versions (
+   realm_url TEXT NOT NULL,
+   current_version INTEGER NOT NULL,
+   PRIMARY KEY ( realm_url ) 
+);
+
+ CREATE TABLE IF NOT EXISTS webhook_commands (
+   id NOT NULL,
+   incoming_webhook_id NOT NULL,
+   command TEXT NOT NULL,
+   command_filter BLOB,
+   created_at NOT NULL,
+   updated_at NOT NULL,
+   PRIMARY KEY ( id ) 
+);

packages/postgres/migrations/1780627635271_drop-job-scoped-instance-cache.js

@@ -0,0 +1,28 @@
+exports.shorthands = undefined;
+
+// Drop the job-scoped per-instance wire-format cache. Prerender `_federated-
+// search` no longer runs the `loadLinks` relationship-assembly pass at all (it
+// returns the matching result ids and the host re-resolves each card from
+// card+source), so the only code that ever read or wrote this table is gone.
+// Nothing else references it — it would otherwise be a table, a janitor timer,
+// and a NOTIFY-listener participant that no path touches.
+exports.up = (pgm) => {
+  pgm.sql(`DROP TABLE IF EXISTS job_scoped_instance_cache;`);
+};
+
+// Recreate the table as it was (UNLOGGED cache keyed by `<jobId>.<reservationId>`
+// + instance url) so the migration is reversible. node-pg-migrate's createTable
+// can't express UNLOGGED, so build it with raw SQL.
+exports.down = (pgm) => {
+  pgm.sql(`
+    CREATE UNLOGGED TABLE job_scoped_instance_cache (
+      job_id      varchar     NOT NULL,
+      url         varchar     NOT NULL,
+      result      text        NOT NULL,
+      created_at  timestamptz NOT NULL DEFAULT NOW(),
+      PRIMARY KEY (job_id, url)
+    );
+    CREATE INDEX job_scoped_instance_cache_created_at_idx
+      ON job_scoped_instance_cache (created_at);
+  `);
+};

packages/realm-server/handlers/handle-search.ts

@@ -79,19 +79,14 @@ export default function handleSearch(opts: {
     }
 
     let cacheOnlyDefinitions = ctxt.get(DURING_PRERENDER_HEADER).length > 0;
-    // Inside a prerender, leave `relationships.{field}.data` populated
-    // for query-backed `linksTo` / `linksToMany` but skip transitive
-    // expansion into `included[]`. The host's prerender-mode getter
-    // resolves the listed IDs from its seed and skips the live
-    // re-query, so the eager closure is a wasted round-trip in this
-    // path. Same gating as `cacheOnlyDefinitions`.
-    let skipQueryBackedExpansion = cacheOnlyDefinitions;
-    // Inside a prerender the host never reads the response's
-    // `included[]` — it resolves every linked card by URL via
-    // card+source (query fields from the seed umbrella, static links via
-    // a lazy-loading `not-loaded` sentinel). Omit `included[]` entirely:
-    // seed the root result cards and skip the static-link BFS that builds
-    // it. Same gating as `cacheOnlyDefinitions`.
+    // Inside a prerender the search skips the `loadLinks`
+    // relationship-assembly pass entirely: the host re-resolves every
+    // result card from its raw card+source file and reads only
+    // `data[].id`, so the query-field `relationships.{field}.data`
+    // umbrellas and the transitive `included[]` are throwaway work
+    // here. The response still carries each result's pristine row (id +
+    // attributes + static-link relationships) and page meta — just no
+    // umbrellas and no `included[]`. Same gating as `cacheOnlyDefinitions`.
     let omitIncluded = cacheOnlyDefinitions;
     // The host's `_federated-search` fetch wrapper stamps
     // `x-boxel-job-priority` while rendering inside a prerender tab.
@@ -105,25 +100,20 @@ export default function handleSearch(opts: {
       ctxt.get(PRERENDER_JOB_PRIORITY_HEADER),
     );
     // `<jobId>.<reservationId>` identity stamped by indexer-driven prerender
-    // requests. Threaded into searchOpts so the per-instance wire-format cache
-    // (`job_scoped_instance_cache`, consulted inside `loadLinks`) scopes its
-    // entries to one indexing job; also reused below as the query-level
-    // cache's job key. Absent for live / external callers.
+    // requests; used below as the job-scoped search cache's job key (the
+    // whole-doc `_federated-search` response cache). Absent for live /
+    // external callers, which therefore bypass the cache.
     let prerenderJobId = sanitizePrerenderJobId(
       ctxt.get(PRERENDER_JOB_ID_HEADER),
     );
     let searchOpts: {
       cacheOnlyDefinitions?: true;
-      skipQueryBackedExpansion?: true;
       omitIncluded?: true;
       priority?: number;
-      jobIdentity?: string;
     } = {};
     if (cacheOnlyDefinitions) searchOpts.cacheOnlyDefinitions = true;
-    if (skipQueryBackedExpansion) searchOpts.skipQueryBackedExpansion = true;
     if (omitIncluded) searchOpts.omitIncluded = true;
     if (jobPriority !== null) searchOpts.priority = jobPriority;
-    if (prerenderJobId) searchOpts.jobIdentity = prerenderJobId;
     let normalizedSearchOpts =
       Object.keys(searchOpts).length > 0 ? searchOpts : undefined;
     // `loggingCorrelationId` / `timings` are deliberately kept OUT of `searchOpts`: