Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:1g9k06j6q7 is past its budget: trevato/forge: "budgets > /map and / answer under their p95 and statement budgets" failed on 3 pulls in the last hour —… #11324
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/11301-sentinel-shared-01m23vygyffx5hg2f5vm8r74 → main 3 commits fork 3 threads · 3 unresolved
2 files
changed +62
−4
bench/snappy.ts +34 −3
| @@ -150,6 +150,37 @@ export interface Timing { | ||
| 150 | 150 | const percentile = (sorted: readonly number[], p: number): number => |
| 151 | 151 | sorted[Math.min(sorted.length - 1, Math.floor((sorted.length * p) / 100))] ?? 0; |
| 152 | 152 | |
| 153 | /** | |
| 154 | * Telemetry flushes on their own timer (docs/CACHING.md, "Telemetry does not | |
| 155 | * invalidate"). They share the connection, so a flush that lands inside a | |
| 156 | * timed request would be counted against the page, and which request it lands | |
| 157 | * in depends on the machine's load, not on the page. Only the flush's writes | |
| 158 | * are named: INSERT/UPDATE/DELETE on `route_timing` and `usage_count`, and the | |
| 159 | * `PRAGMA busy_timeout` calls around them. A page's own read of those tables | |
| 160 | * still counts. | |
| 161 | */ | |
Too broad: this matches a page's own reads of these tables. Anchor it to the telemetry writes. This function also needs a test. |
||
| 162 | const TELEMETRY_SQL = | |
| 163 | /^(INSERT INTO|UPDATE|DELETE FROM)\s+(route_timing|usage_count)\b|^PRAGMA busy_timeout/i; | |
| 164 | ||
| 165 | /** The statements run since the counter's last reset, less the telemetry flush's. */ | |
| 166 | export function pageStatements(counter: SqlCounter): number { | |
| 167 | let telemetry = 0; | |
| 168 | for (const { sql, times } of counter.top(Number.MAX_SAFE_INTEGER)) if (TELEMETRY_SQL.test(sql)) telemetry += times; | |
| 169 | return counter.count - telemetry; | |
| 170 | } | |
| 171 | ||
| 172 | /** | |
| 173 | * The median of per-request statement counts. Another timer's stray statement | |
| 174 | * lands in one window or another by the machine's load and must not move the | |
| 175 | * figure. The cost: a statement issued by fewer than half the requests (a | |
| 176 | * cache miss every fourth request) is not seen here; the cold rows, which | |
| 177 | * write before every request, are where misses are budgeted. | |
| 178 | */ | |
| 179 | export function medianCount(counts: readonly number[]): number { | |
| 180 | const sorted = [...counts].sort((a, b) => a - b); | |
| 181 | return sorted[Math.floor(sorted.length / 2)] ?? 0; | |
| 182 | } | |
| 183 | ||
| 153 | 184 | /** Time `n` sequential requests after one warm-up; SQL is per request. */ |
| 154 | 185 | export async function timeRoute( |
| 155 | 186 | f: SnappyFixture, |
| @@ -168,7 +199,7 @@ export async function timeRoute( | ||
| 168 | 199 | await hit(); |
| 169 | 200 | const times: number[] = []; |
| 170 | 201 | let status = 0; |
| 171 | let statements = 0; | |
| 202 | const counts: number[] = []; | |
| 172 | 203 | for (let i = 0; i < n; i++) { |
| 173 | 204 | before?.(); |
| 174 | 205 | // Only the page's own statements count: not the write before it. |
| @@ -176,9 +207,9 @@ export async function timeRoute( | ||
| 176 | 207 | const started = performance.now(); |
| 177 | 208 | status = await hit(); |
| 178 | 209 | times.push(performance.now() - started); |
| 179 | statements += f.counter.count; | |
| 210 | counts.push(pageStatements(f.counter)); | |
| 180 | 211 | } |
| 181 | const sql = statements / n; | |
| 212 | const sql = medianCount(counts); | |
| 182 | 213 | times.sort((a, b) => a - b); |
| 183 | 214 | return { name, p50: percentile(times, 50), p95: percentile(times, 95), sql, status }; |
| 184 | 215 | } |
test/snappy.test.ts +28 −1
| @@ -13,7 +13,8 @@ import { linkIssue } from "../src/actions/dependencies.ts"; | ||
| 13 | 13 | import { writeStamp } from "../src/core/write_clock.ts"; |
| 14 | 14 | import { feedCandidateCache, readFeed } from "../src/feed/feed.ts"; |
| 15 | 15 | import { mapCacheStats } from "../src/web/map_cache.ts"; |
| 16 | import { snappyFixture, timeRoute, type SnappyFixture } from "../bench/snappy.ts"; | |
| 16 | import { countStatements } from "../bench/run.ts"; | |
| 17 | import { medianCount, pageStatements, snappyFixture, timeRoute, type SnappyFixture } from "../bench/snappy.ts"; | |
| 17 | 18 | |
| 18 | 19 | /** |
| 19 | 20 | * The budgets, per request on the fixture: twelve apps of 24 planned |
| @@ -63,6 +64,32 @@ describe("budgets", () => { | ||
| 63 | 64 | } |
| 64 | 65 | }, 60_000); |
| 65 | 66 | |
| 67 | test("the statement figure is the page's own: no telemetry flush, one outlier ignored", () => { | |
| 68 | const db = new Database(":memory:"); | |
| 69 | db.run("CREATE TABLE route_timing (hour, pattern, bucket, n)"); | |
| 70 | db.run("CREATE TABLE usage_count (n)"); | |
| 71 | const counter = countStatements(db); | |
| 72 | db.query("INSERT INTO route_timing (hour, pattern, bucket, n) VALUES (1, 'a', 1, 1)").run(); | |
| 73 | db.query("INSERT INTO usage_count (n) VALUES (1)").run(); | |
| 74 | db.query("DELETE FROM route_timing WHERE hour < 0").run(); | |
| 75 | db.query("PRAGMA busy_timeout = 0").all(); | |
| 76 | db.query("SELECT 1").all(); | |
| 77 | const mixed = pageStatements(counter); | |
| 78 | counter.reset(); | |
| 79 | db.query("SELECT 1").all(); | |
| 80 | const alone = pageStatements(counter); | |
| 81 | expect(alone).toBeGreaterThan(0); | |
| 82 | expect(mixed).toBe(alone); | |
| 83 | // A page's own read of a telemetry table is still the page's. | |
| 84 | counter.reset(); | |
| 85 | db.query("SELECT n FROM usage_count").all(); | |
| 86 | expect(pageStatements(counter)).toBe(alone); | |
| 87 | counter.stop(); | |
| 88 | db.close(); | |
| 89 | expect(medianCount([19, 19, 40, 19, 19])).toBe(19); | |
| 90 | expect(medianCount([])).toBe(0); | |
| 91 | }); | |
| 92 | ||
| 66 | 93 | test("a warm /map computes no lane: every plan is a hit", async () => { |
| 67 | 94 | await page("/map", f.ownerCookie); |
| 68 | 95 | const misses = mapCacheStats().lanes.misses; |
new above
offline
The original doc comment now sits above the telemetry block and no longer documents
timeRoute. Reorder.