From 8c5de75eedfb2324eebcbe038f48f85ecea38730 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Fri, 21 Aug 2026 15:48:12 -0400 Subject: [PATCH] [rooms] improve performance of rooms query --- apps/rooms/migrations/0014_room_listable.sql | 34 ++++++ apps/rooms/src/test/integration/api.test.ts | 48 ++++++++ packages/domain/src/rooms-db.ts | 111 +++++++++++++++---- 3 files changed, 169 insertions(+), 24 deletions(-) create mode 100644 apps/rooms/migrations/0014_room_listable.sql diff --git a/apps/rooms/migrations/0014_room_listable.sql b/apps/rooms/migrations/0014_room_listable.sql new file mode 100644 index 0000000..d46be78 --- /dev/null +++ b/apps/rooms/migrations/0014_room_listable.sql @@ -0,0 +1,34 @@ +-- The room flags every public feed filters on, as indexed columns. +-- +-- Generated from packages/domain/src/rooms-db.ts (ROOM_SCHEMA_DDL) — keep in sync. +-- +-- `is_dorm` was already extracted back in 0001, but `Accessibility` and `ExcludeFromLists` +-- were not, so "the rooms a feed may show" could only be asked in JS. Every feed — hot, recommended, +-- featured, new, recently-updated, similar, search, autocomplete — therefore SELECTed the +-- whole `room` table and threw most of it away after parsing it. On this database that is +-- 1173 rows and ~1.7MB of blob read to rank the 109 rooms that are actually listable: the +-- rest are DORMS (one per account, private by construction) and other private rooms. It is +-- the read D1 reports as slow, and it gets worse with every account that signs up. +-- +-- VIRTUAL like the other generated columns, so the blob stays the only copy of the value +-- and no backfill is needed — they are computed on read from the JSON already stored. +ALTER TABLE room ADD COLUMN accessibility INTEGER GENERATED ALWAYS AS (json_extract(data, '$.Accessibility')) VIRTUAL; +ALTER TABLE room ADD COLUMN exclude_from_lists INTEGER GENERATED ALWAYS AS (json_extract(data, '$.ExcludeFromLists')) VIRTUAL; + +-- A PARTIAL index: it holds only the public, non-dorm rooms, which is the small minority +-- the feeds serve from. Scanning it visits those rows alone, so the feeds stop reading the +-- dorms at all rather than reading and discarding them. EXPLAIN QUERY PLAN over the feed +-- query goes from +-- +-- SCAN room +-- +-- to +-- +-- SCAN room USING INDEX idx_room_public +-- +-- `ExcludeFromLists` is deliberately NOT in the WHERE: search reads the public rooms +-- WITHOUT that term (a room can opt out of the browse feeds and still be findable by +-- name), so indexing the wider set lets both reads use this one index — the extra term is +-- then checked against the handful of rows it already fetched. +CREATE INDEX IF NOT EXISTS idx_room_public ON room (room_id) + WHERE is_dorm IS NOT 1 AND accessibility = 1; diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index d76320b..f011773 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -1053,6 +1053,54 @@ describe('rooms endpoints', () => { expect(body.length).toBeLessThanOrEqual(3) }) + // The feeds narrow to the listable rooms in SQL (LISTABLE_WHERE, off idx_room_public) + // rather than by parsing every room in the database. That predicate has to answer + // exactly what the in-memory `isListable` answered, and the interesting case is the + // MISSING key: `ExcludeFromLists !== true` accepts a blob that never carried the field, + // so the SQL has to accept the NULL its generated column extracts. + it('the feeds’ SQL filter treats a missing ExcludeFromLists as not-excluded', async () => { + const seed = (data: Record) => + env.DB.prepare('INSERT INTO room (data) VALUES (?1)').bind(JSON.stringify(data)).run() + + // No `ExcludeFromLists` key at all — the shape of every room seeded before the flag + // existed. + await seed({ + RoomId: 30801, + Name: 'NoExcludeKey', + CreatorAccountId: 830, + Accessibility: 1, + IsDorm: false, + SubRooms: [], + }) + // Same room, opted out. + await seed({ + RoomId: 30802, + Name: 'OptedOut', + CreatorAccountId: 830, + Accessibility: 1, + IsDorm: false, + ExcludeFromLists: true, + SubRooms: [], + }) + + const namesIn = async (path: string) => { + const body = (await (await SELF.fetch(`${ORIGIN}${path}`)).json()) as + { Results: Array<{ Name: string }> } | Array<{ Name: string }> + return (Array.isArray(body) ? body : body.Results).map((r) => r.Name) + } + + for (const feed of ['/rooms/hot?take=200', '/rooms/recommendations?take=200']) { + expect(await namesIn(feed)).toContain('NoExcludeKey') + expect(await namesIn(feed)).not.toContain('OptedOut') + } + + // SEARCH is the wider filter (PUBLIC_WHERE): a room can opt out of the browse feeds + // and still be findable by name, so this must not have narrowed with the feeds. + expect(await namesIn('/rooms/search?query=optedout')).toContain('OptedOut') + + await env.DB.prepare('DELETE FROM room WHERE room_id IN (30801, 30802)').run() + }) + // Served only to the client builds that render it — the 2023 client's other room // listings start failing with NREs when it gets this payload, which is why the route // was parked entirely for a while (see FEATURED_ROOMS_VERSIONS in rooms.app.ts). diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index 5c2564b..c68b624 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -34,11 +34,26 @@ export const ROOM_SCHEMA_DDL: string[] = [ -- and served as the room's \`Stats.VisitCount\`. A real column rather than a field -- in the blob so a visit is one atomic UPDATE that can't lose a concurrent -- read-modify-write of the whole room. - visits INTEGER NOT NULL DEFAULT 0 + visits INTEGER NOT NULL DEFAULT 0, + -- The two flags every public feed filters on, alongside \`is_dorm\` + -- (migrations/0014_room_listable.sql, which appends them here). Generated like the + -- rest so the blob stays the only copy; they exist to be INDEXED — see + -- {@link LISTABLE_WHERE}. + accessibility INTEGER GENERATED ALWAYS AS (json_extract(data, '$.Accessibility')) VIRTUAL, + exclude_from_lists INTEGER GENERATED ALWAYS AS (json_extract(data, '$.ExcludeFromLists')) VIRTUAL )`, `CREATE UNIQUE INDEX IF NOT EXISTS idx_rooms_room_id ON room (room_id)`, `CREATE INDEX IF NOT EXISTS idx_rooms_name_lower ON room (name_lower)`, `CREATE INDEX IF NOT EXISTS idx_rooms_creator ON room (creator_account_id)`, + // PARTIAL index over the public, non-dorm rooms — the only rooms any feed can serve, + // and a small minority of the table (most rooms are dorms, one per account). Scanning + // it visits those rooms alone instead of every room in the database; see + // {@link LISTABLE_WHERE} for why the feeds select on it. + // + // Indexed on `room_id` because a partial index needs some column to key on and the + // feeds all order by it eventually; the WHERE clause is the point, not the key. + `CREATE INDEX IF NOT EXISTS idx_room_public ON room (room_id) + WHERE is_dorm IS NOT 1 AND accessibility = 1`, // A room's tags, one row per tag (migrations/0013_room_tag.sql). Modelled on the // `api` worker's `event_tag`, and the table is AUTHORITATIVE: `serializeRoom` strips // `Tags` from the blob and the reads re-attach it, the same arrangement `subroom` and @@ -890,6 +905,32 @@ interface RoomRow { */ const ROOM_COLUMNS = 'data, visits' +/** + * The rooms a public feed may consider, as a SQL predicate: public, not a dorm, and not + * opted out of lists — the same test {@link isListable} makes in memory, pushed down so + * the blobs of the rooms that fail it never cross the wire. `IS NOT 1` rather than `= 0` + * because a blob missing the key extracts as NULL, which the JS `!== true` accepts. + * + * The feeds all scanned the whole table and threw most of it away: a server's rooms are + * mostly DORMS (one per account, private by construction), so a scan read megabytes of + * blob to rank a hundred rooms. `idx_room_public` covers the first two terms, so this + * visits only the rooms that can actually be served. + * + * The in-memory filter STAYS wherever this is used. It costs nothing once the set is + * small, and it — not the SQL — remains the definition of listable: a blob with a + * surprising type in one of these fields (`"1"` for `Accessibility`, say) would satisfy + * the column's integer affinity while failing `=== 1` in JS, and the feeds must agree + * with {@link isListable} rather than with SQLite. + */ +const LISTABLE_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1 AND exclude_from_lists IS NOT 1' + +/** + * The wider half of {@link LISTABLE_WHERE}: public and not a dorm, without the + * `ExcludeFromLists` term. What SEARCH considers — a room can opt out of the browse feeds + * and still be findable by name — so the two searching reads select on this instead. + */ +const PUBLIC_WHERE = 'is_dorm IS NOT 1 AND accessibility = 1' + /** * Keys on the client's room DTO that nothing here stores, defaulted on every read so the * key is PRESENT rather than absent — the seed blobs and every room written since predate @@ -1210,8 +1251,12 @@ export async function setRoomTags(db: D1Database, roomId: number, tags: RoomTag[ * what the in-memory filter it replaced cost. The join searches the tag index FIRST and * then looks up only the rooms that matched, which is what makes a category row cheap. */ -function roomsByTagsQuery(tagSets: string[][]): { sql: string; binds: string[] } { - if (tagSets.length === 0) return { sql: `SELECT ${ROOM_COLUMNS} FROM room`, binds: [] } +function roomsByTagsQuery(tagSets: string[][], where = ''): { sql: string; binds: string[] } { + // `where` is the caller's row filter ({@link LISTABLE_WHERE} or {@link PUBLIC_WHERE}) — + // unqualified, which is unambiguous under either shape below. It matters most when + // `tagSets` is EMPTY: that branch is the full scan every pseudo-tag feed still runs. + const filter = where === '' ? '' : ` WHERE ${where}` + if (tagSets.length === 0) return { sql: `SELECT ${ROOM_COLUMNS} FROM room${filter}`, binds: [] } const binds: string[] = [] const joins = tagSets.map((tags, i) => { @@ -1222,7 +1267,7 @@ function roomsByTagsQuery(tagSets: string[][]): { sql: string; binds: string[] } }) // `data`/`visits` are unqualified but unambiguous: the joined subqueries expose only // `room_id`. - return { sql: `SELECT ${ROOM_COLUMNS} FROM room r ${joins.join(' ')}`, binds } + return { sql: `SELECT ${ROOM_COLUMNS} FROM room r ${joins.join(' ')}${filter}`, binds } } /** Parse subroom rows and resolve their `CurrentSave` in one batched query. */ @@ -2020,7 +2065,8 @@ function roomHasAnyTag(room: Room, tags: Set): boolean { * Search public, non-dorm rooms. The query is split into terms (space/`+`): * `#tag` terms match the room's Tags; plain terms match the room name * (substring). All terms must match. Returns a paginated `{ Results, TotalResults }`. - * The dataset is small, so this filters in memory rather than in SQL. + * The rows narrow in SQL — public, non-dorm ({@link PUBLIC_WHERE}) — and the name terms + * match in memory over what comes back. * * `#community` is the one tag term that isn't a tag lookup — see {@link COMMUNITY_TAG}. */ @@ -2048,7 +2094,7 @@ export async function searchRooms( const tagSets = tagTerms .filter((tag) => tag !== COMMUNITY_TAG) .map((tag) => [tag, ...(TAG_ALIASES[tag] ?? [])]) - const { sql, binds } = roomsByTagsQuery(tagSets) + const { sql, binds } = roomsByTagsQuery(tagSets, PUBLIC_WHERE) const { results } = await db .prepare(sql) .bind(...binds) @@ -2091,7 +2137,8 @@ export async function searchRooms( * * Matching is case-insensitive and suggestions are de-duplicated case-insensitively, but * each is returned in its stored casing — search doesn't care, and the player reads these. - * The dataset is small, so this filters in memory like {@link searchRooms}. + * Narrowed in SQL to the same rooms {@link searchRooms} considers; the matching itself is + * in memory, like search's. */ export async function autocompleteRoomSearch( db: D1Database, @@ -2101,9 +2148,11 @@ export async function autocompleteRoomSearch( const q = query.trim().toLowerCase() if (q === '' || take <= 0) return [] - const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() - // Tags attached up front: suggestions are drawn from them, and this reads every room for - // its NAME regardless, so the tags cost one extra query rather than a second scan. + const { results } = await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${PUBLIC_WHERE}`) + .all() + // Tags attached up front: suggestions are drawn from them, and this reads every candidate + // room for its NAME regardless, so the tags cost one extra query rather than a second scan. const rooms = (await parseAllWithTags(db, results)).filter( (r) => r.IsDorm !== true && r.Accessibility === 1 ) @@ -2216,8 +2265,8 @@ function createdAt(room: Room): number { * aliases as search). "Hot" is a live-population feed, so current players lead; * rooms nobody is in — and the all-zero seed data — fall back to the stored * engagement score, then to RoomId order so paging stays stable. Paginated via - * skip/take; returns `{ Results, TotalResults }` like search. The dataset is - * small, so this filters/sorts in memory rather than in SQL. + * skip/take; returns `{ Results, TotalResults }` like search. The listable filter runs in + * SQL ({@link LISTABLE_WHERE}); the ranking is in memory. * * `tag=new` and `tag=community` are the filters that aren't tag lookups — see * {@link NEW_TAG} and {@link COMMUNITY_TAG}. @@ -2234,7 +2283,10 @@ export async function getHotRooms( // discovery category row runs: only the rooms carrying the tag have their blobs read. // The two pseudo-tags below name no tag at all, so they still scan. const isPseudo = t === '' || t === NEW_TAG || t === COMMUNITY_TAG - const { sql, binds } = roomsByTagsQuery(isPseudo ? [] : [[t, ...(TAG_ALIASES[t] ?? [])]]) + const { sql, binds } = roomsByTagsQuery( + isPseudo ? [] : [[t, ...(TAG_ALIASES[t] ?? [])]], + LISTABLE_WHERE + ) const { results } = await db .prepare(sql) .bind(...binds) @@ -2319,15 +2371,17 @@ async function lastPublishedAtByRoom(db: D1Database): Promise { - const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() + const { results } = await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${LISTABLE_WHERE}`) + .all() const rooms = parseAll(results).filter((r) => isListable(r) && isPlayerMade(r)) const published = await lastPublishedAtByRoom(db) @@ -2357,7 +2411,9 @@ export async function getNewRooms( skip: number, take: number ): Promise<{ Results: Room[]; TotalResults: number }> { - const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() + const { results } = await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${LISTABLE_WHERE}`) + .all() const rooms = parseAll(results) .filter((r) => isListable(r) && isPlayerMade(r)) .sort((a, b) => createdAt(b) - createdAt(a) || roomIdOf(b) - roomIdOf(a)) @@ -2373,15 +2429,17 @@ export async function getNewRooms( * by engagement (same score as the hot feed). Unlike the hot feed this returns a * bare array — the client's recommendation room-source loader expects a plain * list, like the other `*by/me`/base sources. The `splitTest*` A/B params the - * client passes don't change the result. Paginated via skip/take; the dataset is - * small, so this filters/sorts in memory rather than in SQL. + * client passes don't change the result. Paginated via skip/take; the listable filter runs + * in SQL ({@link LISTABLE_WHERE}); the ranking is in memory. */ export async function getRecommendedRooms( db: D1Database, skip: number, take: number ): Promise { - const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() + const { results } = await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${LISTABLE_WHERE}`) + .all() const stats = await getRoomStats(db) return hydrateRooms( db, @@ -2416,10 +2474,12 @@ export interface FeaturedRoomGroup { * Featured rooms group: public, non-dorm rooms not excluded from lists, in random * order. There's no editorial curation behind this yet, so "featured" is just a * random shuffle of the eligible rooms wrapped in a single always-active group. - * Small dataset, so done in memory. + * Eligibility is filtered in SQL ({@link LISTABLE_WHERE}); the rest is in memory. */ export async function getFeaturedRooms(db: D1Database): Promise { - const { results } = await db.prepare(`SELECT ${ROOM_COLUMNS} FROM room`).all() + const { results } = await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${LISTABLE_WHERE}`) + .all() const rooms = parseAll(results).filter(isListable) // Fisher–Yates shuffle so the feed varies between requests. for (let i = rooms.length - 1; i > 0; i--) { @@ -2450,7 +2510,8 @@ export async function getFeaturedRooms(db: D1Database): Promise() + const { results } = await db + .prepare(`SELECT ${ROOM_COLUMNS} FROM room WHERE ${LISTABLE_WHERE}`) + .all() const sharedCount = (r: Room): number => roomTags(r).filter((t) => targetTags.has(t)).length const stats = await getRoomStats(db)