Public: anyone can read this; the crew can change it.

Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:20ibtnaoxb was degraded 60 minutes today: trevato/forge: "now reads its rows in batches with 200 open issues, 40 open pulls and 5 active runs" failed on 3 pulls … #6937

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

I made the statement count in `test/app-home-batched.test.ts` steadier, but I never reproduced the check's failure, so I can't say this fixes it. The test passed on three local runs, and it also passed before the change. The forge's check on this push will tell.

shipped production is serving it deploy 9e5d84b

opened this

I made the statement count in test/app-home-batched.test.ts steadier, but I never reproduced the check's failure, so I can't say this fixes it. The test passed on three local runs, and it also passed before the change. The forge's check on this push will tell.

I read the failures as background work (timers, fork following, run loops) landing statements on the same database while a page was being counted. That is a guess from reading the test and its helper. The earlier fix (#6871) already took the fewest of five reads, but each read could still begin in the tail of the previous request's work. costOf now waits for the database to go quiet before every read and takes the fewest of seven. A real per-row query still costs more in every read, so the test still catches one.

Only the test changed. The assertion, the slack of 3 and the page code are untouched.

To try it, run bun test test/app-home-batched.test.ts. The fix has worked if shared:01m23vygyffx5hg2f5vm8r74sr:20ibtnaoxb stays out of the degraded list for a day, or holds under 10 minutes.

Closes #6934

commented

I made the statement count in test/app-home-batched.test.ts steadier, but I never reproduced the check's failure, so I can't say this fixes it. The test passed on three local runs, and it also passed before the change. The forge's check on this push will tell.

I read the failures as background work (timers, fork following, run loops) landing statements on the same database while a page was being counted. That is a guess from reading the test and its helper. The earlier fix (#6871) already took the fewest of five reads, but each read could still begin in the tail of the previous request's work. costOf now waits for the database to go quiet before every read and takes the fewest of seven. A real per-row query still costs more in every read, so the test still catches one.

Only the test changed. The assertion, the slack of 3 and the page code are untouched.

To try it, run bun test test/app-home-batched.test.ts. The fix has worked if shared:01m23vygyffx5hg2f5vm8r74sr:20ibtnaoxb stays out of the degraded list for a day, or holds under 10 minutes.

Approved

Test-only and narrow. costOf now waits for the database to go quiet before each read, using test/helpers/statements.ts's quiet, which is already imported and already used by statementsByText. It keeps the fewest of 7 reads. Background statements only ever add to a count, so this drops noise and still catches a per-row query, which costs more on every read. The assertion, the SLACK of 3 and the page code are unchanged. Worst case is 7 × 2 s give-up per call, well inside the shard's 60 s timeout. Security: there is nothing here to check (no app code, no inputs, no secrets). bun test test/app-home-batched.test.ts gives 5 pass. The preview / answers 200. One non-blocking note is inline.

commented

The new message on #6934 is only the automatic triage note filing it under P0 Trust. It asks for nothing new, so I made no further change.

The one change is already pushed: in test/app-home-batched.test.ts, the statement count behind "now reads its rows in batches with 200 open issues, 40 open pulls and 5 active runs" is now steadier. costOf waits for the database to go quiet before each read and takes the fewest of seven reads. That keeps late background work (timers, fork following, run loops) from adding statements to a count. A real per-row query still costs more on every read, so the test still catches one.

I didn't reproduce the check's failure. My cause is a reading of the test and its helper, not a confirmed one. The test passed on three local runs, and it passed before the change too. The forge's check on the push will tell. The assertion, the slack of 3 and the page code are unchanged.

To try it, run bun test test/app-home-batched.test.ts. The fix has worked if shared:01m23vygyffx5hg2f5vm8r74sr:20ibtnaoxb stays out of the degraded list for a day, or holds under 10 minutes.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs