Skip to content

Commit 93acd52

Browse files
grrowlclaude
andcommitted
perf(server): batch hydrateRows; per-table catch-up reads use the (tbl,seq) index
Catch-up hydration was N+1 (one query per key); now chunked IN (≤64) so a 500-key reconnect issues ⌈500/64⌉=8 queries, not 500. The per-table changelog read uses the _sync_changes_tbl_seq composite index via the new readChangesSinceFor() rather than reading all changes and filtering by tbl in JS. Merge note: resolved against plan-006 (already on main) — kept the new readChangesSinceFor and honored 006's deletion of the dead snapshotAll. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f4c4eaf commit 93acd52

3 files changed

Lines changed: 105 additions & 7 deletions

File tree

src/server/changes.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -234,20 +234,45 @@ export function readChangesSince(sql: SqlStorage, cursor: number): Array<ChangeR
234234
)
235235
}
236236

237+
/** One table's change rows with seq > cursor, in seq order. Uses the
238+
* `_sync_changes_tbl_seq` composite index — this is the per-table catch-up
239+
* read that index exists for. */
240+
export function readChangesSinceFor(
241+
sql: SqlStorage,
242+
tbl: string,
243+
cursor: number,
244+
): Array<ChangeRow> {
245+
return Array.from(
246+
sql.exec<ChangeRow>(
247+
"SELECT seq, tbl, key, op, ts FROM _sync_changes WHERE tbl = ? AND seq > ? ORDER BY seq",
248+
tbl,
249+
cursor,
250+
),
251+
)
252+
}
253+
237254
/** Current rows for a set of keys, for hydrating deltas. `tbl`/`pk` are
238-
* validated identifiers (the SyncRegistry enforces this). */
255+
* validated identifiers (the SyncRegistry enforces this). Queries in chunks
256+
* of 64 to avoid SQLite bound-parameter limits and eliminate the N+1 pattern:
257+
* a reconnect catch-up over 500 keys now issues ⌈500/64⌉ = 8 queries instead
258+
* of 500. Identifiers are quoted; values are bound parameters. */
239259
export function hydrateRows(
240260
sql: SqlStorage,
241261
tbl: string,
242262
pk: string,
243263
keys: Array<string>,
244264
): Map<string, Record<string, SqlStorageValue>> {
245265
const out = new Map<string, Record<string, SqlStorageValue>>()
246-
for (const k of keys) {
247-
const rows = Array.from(
248-
sql.exec<Record<string, SqlStorageValue>>(`SELECT * FROM ${tbl} WHERE ${pk} = ? LIMIT 1`, k),
249-
)
250-
if (rows.length > 0) out.set(k, rows[0]!)
266+
const CHUNK = 64 // stay far below any SQLite bound-parameter limit
267+
for (let i = 0; i < keys.length; i += CHUNK) {
268+
const chunk = keys.slice(i, i + CHUNK)
269+
const placeholders = chunk.map(() => "?").join(", ")
270+
for (const row of sql.exec<Record<string, SqlStorageValue>>(
271+
`SELECT * FROM "${tbl}" WHERE "${pk}" IN (${placeholders})`,
272+
...chunk,
273+
)) {
274+
out.set(String(row[pk]), row)
275+
}
251276
}
252277
return out
253278
}

src/server/sync-do.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
minChangeSeq,
2727
pruneChanges,
2828
readChangesSince,
29+
readChangesSinceFor,
2930
setDrainCursor,
3031
} from "./changes.ts"
3132
import { Broadcaster } from "./broadcast.ts"
@@ -527,7 +528,7 @@ export abstract class SyncDurableObject<Env = unknown, TUser = unknown> extends
527528
since: number,
528529
seq: string,
529530
): void {
530-
const changes = readChangesSince(this.sql, since).filter((c) => c.tbl === coll.table)
531+
const changes = readChangesSinceFor(this.sql, coll.table, since)
531532
const latest = new Map<string, (typeof changes)[number]>()
532533
for (const c of changes) latest.set(c.key, c)
533534
const liveKeys = [...latest.values()].filter((c) => c.op !== "delete").map((c) => c.key)

tests/cdc.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import { describe, expect, it } from "vitest"
44
import {
55
currentSeq,
66
getDrainCursor,
7+
hydrateRows,
78
initSchema,
89
installTriggers,
910
readChangesSince,
11+
readChangesSinceFor,
1012
setDrainCursor,
1113
} from "../src/server/changes.ts"
1214

@@ -113,4 +115,74 @@ describe("CDC: AFTER triggers -> _sync_changes (D12)", () => {
113115
expect(new Set(seqs).size).toBe(10)
114116
})
115117
})
118+
119+
// WHY: chunking must not drop or duplicate keys — the batched IN query must
120+
// return exactly the rows that exist, regardless of how many chunks are needed
121+
// (ADR-0007 D9: TEXT pk, String(row[pk]) keying must match the changelog key).
122+
it("hydrateRows returns exactly the existing keys across multiple chunks (no drops, no dups)", async () => {
123+
await runInDurableObject(freshStub(), (_i, state) => {
124+
const sql = state.storage.sql
125+
setup(sql)
126+
// Seed 150 rows — spans 3 chunks of 64 (64+64+22).
127+
const ids: string[] = []
128+
for (let i = 0; i < 150; i++) {
129+
const id = `row-${String(i).padStart(3, "0")}`
130+
ids.push(id)
131+
sql.exec("INSERT INTO items(id,name,n) VALUES(?,?,?)", id, `name-${i}`, i)
132+
}
133+
// Request all existing keys plus a handful of nonexistent ones.
134+
const nonexistent = ["missing-1", "missing-2", "missing-3"]
135+
const result = hydrateRows(sql, "items", "id", [...ids, ...nonexistent])
136+
// Exactly the 150 seeded rows — nonexistent keys absent, no dups.
137+
expect(result.size).toBe(150)
138+
for (const id of ids) {
139+
const row = result.get(id)
140+
expect(row).toBeDefined()
141+
expect(row!["id"]).toBe(id)
142+
}
143+
for (const id of nonexistent) {
144+
expect(result.has(id)).toBe(false)
145+
}
146+
})
147+
})
148+
149+
// WHY: readChangesSinceFor must use the (tbl,seq) index path and return only
150+
// the requested table's rows — interleaved writes to other tables must not
151+
// appear. A mid-stream cursor must window correctly, matching the per-table
152+
// contract that emitCatchUp relies on for exactly-once catch-up delivery.
153+
it("readChangesSinceFor isolates by table and windows by cursor", async () => {
154+
await runInDurableObject(freshStub(), (_i, state) => {
155+
const sql = state.storage.sql
156+
initSchema(sql)
157+
// Two tables with interleaved writes.
158+
sql.exec(`CREATE TABLE IF NOT EXISTS messages (id TEXT PRIMARY KEY, body TEXT)`)
159+
sql.exec(`CREATE TABLE IF NOT EXISTS files (id TEXT PRIMARY KEY, name TEXT)`)
160+
installTriggers(sql, "messages", "id")
161+
installTriggers(sql, "files", "id")
162+
163+
sql.exec("INSERT INTO messages(id,body) VALUES('m1','hello')")
164+
sql.exec("INSERT INTO files(id,name) VALUES('f1','doc.pdf')")
165+
sql.exec("INSERT INTO messages(id,body) VALUES('m2','world')")
166+
sql.exec("INSERT INTO files(id,name) VALUES('f2','img.png')")
167+
sql.exec("INSERT INTO messages(id,body) VALUES('m3','!')")
168+
169+
// readChangesSinceFor("messages", 0) returns only messages rows, ascending.
170+
const msgRows = readChangesSinceFor(sql, "messages", 0)
171+
expect(msgRows.map((r) => r.key)).toEqual(["m1", "m2", "m3"])
172+
expect(msgRows.every((r) => r.tbl === "messages")).toBe(true)
173+
// seq values are ascending.
174+
const seqs = msgRows.map((r) => r.seq)
175+
expect(seqs).toEqual([...seqs].sort((a, b) => a - b))
176+
177+
// readChangesSinceFor("files", 0) returns only files rows.
178+
const fileRows = readChangesSinceFor(sql, "files", 0)
179+
expect(fileRows.map((r) => r.key)).toEqual(["f1", "f2"])
180+
expect(fileRows.every((r) => r.tbl === "files")).toBe(true)
181+
182+
// Mid-stream cursor: only rows after the seq of m2 (the 3rd change overall).
183+
// seq of m2 is msgRows[1].seq; we want only m3 after that.
184+
const afterM2 = readChangesSinceFor(sql, "messages", msgRows[1]!.seq)
185+
expect(afterM2.map((r) => r.key)).toEqual(["m3"])
186+
})
187+
})
116188
})

0 commit comments

Comments
 (0)