Public: anyone can read this; the crew can change it.
Sentinel: release was degraded 528 minutes today: Release 0.7.382 is unproven: https://forge.open-platform.sh/search?q=a as desktop /search?q=a did not answer within 200… #9476
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9402-sentinel-release-was-degraded-528-minute → main 1 commit fork
2 files
changed +99
−33
src/core/search.ts +45 −30
| @@ -578,8 +578,12 @@ export interface GrepWho { | ||
| 578 | 578 | readonly now?: () => number; |
| 579 | 579 | /** Resolves after `ms`; `clear` stops it. A `setTimeout` by default. */ |
| 580 | 580 | readonly timer?: (ms: number) => BudgetTimer; |
| 581 | /** The first batch's ceiling (`BUDGETED_GREP_MS`), kept apart from `timer` so tests fire each on its own. */ | |
| 582 | readonly firstTimer?: (ms: number) => BudgetTimer; | |
| 581 | 583 | } |
| 582 | 584 | |
| 585 | type Got = RawHit[] | "skipped" | "timeout"; | |
| 586 | ||
| 583 | 587 | export interface BudgetTimer { |
| 584 | 588 | readonly done: Promise<void>; |
| 585 | 589 | clear(): void; |
| @@ -599,8 +603,10 @@ function budgetTimer(ms: number): BudgetTimer { | ||
| 599 | 603 | * stops the moment the cap is reached — a common word in a monorepo costs |
| 600 | 604 | * one repo's work, not fifty. |
| 601 | 605 | * |
| 602 | * With a finite budget the first batch is always waited for, so a cold search | |
| 603 | * shows what it finds however slow git is. Each later batch races the time | |
| 606 | * With a finite budget the first batch is waited for up to `BUDGETED_GREP_MS`, | |
| 607 | * queue for git's gate included, so a cold search shows what it finds without | |
| 608 | * a busy forge holding the page; what answered by then is kept, the rest is | |
| 609 | * `more` (#9402). Each later batch races the time | |
| 604 | 610 | * left: when the budget runs out first the answer goes back with `more`, and |
| 605 | 611 | * the batch's greps are left to finish on their own. They keep the slot |
| 606 | 612 | * `search/grep_slots.ts` gave them until git exits — that is the only cap on |
| @@ -633,46 +639,54 @@ export async function grepRepos( | ||
| 633 | 639 | answered = true; |
| 634 | 640 | return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: true, skipped }; |
| 635 | 641 | }; |
| 636 | const grepBatch = async (slice: readonly Repo[]) => { | |
| 637 | const found: (RawHit[] | "skipped" | "timeout")[] = []; | |
| 642 | // Each grep writes its answer into its own place as it settles, so a batch | |
| 643 | // cut at the clock still keeps the repositories that did answer. | |
| 644 | const grepBatch = async (slice: readonly Repo[], found: (Got | undefined)[]) => { | |
| 638 | 645 | for (let k = 0; k < slice.length && !answered; k += width) { |
| 639 | found.push( | |
| 640 | ...(await Promise.all( | |
| 641 | slice | |
| 642 | .slice(k, k + width) | |
| 643 | .map((repo) => | |
| 644 | grepCached( | |
| 645 | repo, | |
| 646 | repoDir(reposRoot, repo.ownerName, repo.name), | |
| 647 | query, | |
| 648 | who, | |
| 649 | timeoutMs, | |
| 650 | ), | |
| 651 | ), | |
| 652 | )), | |
| 646 | await Promise.all( | |
| 647 | slice.slice(k, k + width).map((repo, j) => | |
| 648 | grepCached( | |
| 649 | repo, | |
| 650 | repoDir(reposRoot, repo.ownerName, repo.name), | |
| 651 | query, | |
| 652 | who, | |
| 653 | timeoutMs, | |
| 654 | ).then((got) => { | |
| 655 | found[k + j] = got; | |
| 656 | }), | |
| 657 | ), | |
| 653 | 658 | ); |
| 654 | 659 | } |
| 655 | return found; | |
| 656 | 660 | }; |
| 657 | 661 | for (let i = 0; i < considered.length; i += batch) { |
| 658 | 662 | const slice = considered.slice(i, i + batch); |
| 659 | let found: (RawHit[] | "skipped" | "timeout")[]; | |
| 660 | if (i === 0 || budgetMs === Infinity) { | |
| 661 | found = await grepBatch(slice); | |
| 663 | const found: (Got | undefined)[] = []; | |
| 664 | let cut = false; | |
| 665 | if (budgetMs === Infinity) { | |
| 666 | await grepBatch(slice, found); | |
| 662 | 667 | } else { |
| 663 | const left = budgetMs - (now() - started); | |
| 664 | if (left <= 0) return more(); | |
| 665 | const running = grepBatch(slice); | |
| 668 | // Batch 0 is waited for, but never past BUDGETED_GREP_MS counted from | |
| 669 | // here: a grep's own timeout starts only once git's gate admits it, so a | |
| 670 | // forge busy with git could otherwise hold the page for seconds (#9402). | |
| 671 | const wait = i === 0 ? BUDGETED_GREP_MS : budgetMs - (now() - started); | |
| 672 | if (wait <= 0) return more(); | |
| 673 | const running = grepBatch(slice, found); | |
| 666 | 674 | // An abandoned batch settles on its own; a rejection then is nobody's. |
| 667 | 675 | running.catch(() => {}); |
| 668 | const timer = (who.timer ?? budgetTimer)(left); | |
| 669 | const raced = await Promise.race([running, timer.done.then(() => "cut" as const)]); | |
| 676 | const timer = (i === 0 ? (who.firstTimer ?? budgetTimer) : (who.timer ?? budgetTimer))(wait); | |
| 677 | const raced = await Promise.race([ | |
| 678 | running.then(() => "done" as const), | |
| 679 | timer.done.then(() => "cut" as const), | |
| 680 | ]); | |
| 670 | 681 | timer.clear(); |
| 671 | if (raced === "cut") return more(); | |
| 672 | found = raced; | |
| 682 | if (raced === "cut") { | |
| 683 | answered = true; | |
| 684 | cut = true; | |
| 685 | } | |
| 673 | 686 | } |
| 674 | 687 | for (const [j, repo] of slice.entries()) { |
| 675 | const mine = found[j] ?? []; | |
| 688 | const mine = found[j]; | |
| 689 | if (mine === undefined) continue; | |
| 676 | 690 | if (mine === "skipped") { |
| 677 | 691 | skipped = true; |
| 678 | 692 | continue; |
| @@ -690,6 +704,7 @@ export async function grepRepos( | ||
| 690 | 704 | }); |
| 691 | 705 | } |
| 692 | 706 | } |
| 707 | if (cut) return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: true, skipped }; | |
| 693 | 708 | } |
| 694 | 709 | return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: gaveUp, skipped }; |
| 695 | 710 | } |
test/search-batch-race.test.ts +54 −3
| @@ -6,7 +6,7 @@ | ||
| 6 | 6 | */ |
| 7 | 7 | import { expect, test } from "bun:test"; |
| 8 | 8 | import { createGrepSlots } from "../src/search/grep_slots.ts"; |
| 9 | import { grepRepos, type BudgetTimer } from "../src/core/search.ts"; | |
| 9 | import { BUDGETED_GREP_MS, grepRepos, type BudgetTimer } from "../src/core/search.ts"; | |
| 10 | 10 | import type { Repo } from "../src/core/repos.ts"; |
| 11 | 11 | |
| 12 | 12 | type Raw = { path: string; line: number; text: string }[] | null; |
| @@ -39,12 +39,14 @@ function handTimer() { | ||
| 39 | 39 | let fire!: () => void; |
| 40 | 40 | let asked = 0; |
| 41 | 41 | let cleared = 0; |
| 42 | const timer = (_ms: number): BudgetTimer => { | |
| 42 | let ms = -1; | |
| 43 | const timer = (wait: number): BudgetTimer => { | |
| 43 | 44 | asked++; |
| 45 | ms = wait; | |
| 44 | 46 | const done = new Promise<void>((r) => (fire = r)); |
| 45 | 47 | return { done, clear: () => void cleared++ }; |
| 46 | 48 | }; |
| 47 | return { timer, fire: () => fire(), asked: () => asked, cleared: () => cleared }; | |
| 49 | return { timer, fire: () => fire(), asked: () => asked, cleared: () => cleared, ms: () => ms }; | |
| 48 | 50 | } |
| 49 | 51 | |
| 50 | 52 | test("a spent budget still waits for batch 0 and keeps its hits", async () => { |
| @@ -186,3 +188,52 @@ test("a search with no budget never asks for a timer and waits for every batch", | ||
| 186 | 188 | expect(out.hits.length).toBe(5); |
| 187 | 189 | expect(clock.asked()).toBe(0); |
| 188 | 190 | }); |
| 191 | ||
| 192 | test("batch 0 stuck behind git's gate is cut at BUDGETED_GREP_MS: what answered is kept, the rest is more (#9402)", async () => { | |
| 193 | const g = deferredGreps(); | |
| 194 | const slots = createGrepSlots(); | |
| 195 | const first = handTimer(); | |
| 196 | const later = handTimer(); | |
| 197 | const search = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 30, { | |
| 198 | slots, | |
| 199 | grep: g.grep, | |
| 200 | now: () => 0, | |
| 201 | timer: later.timer, | |
| 202 | firstTimer: first.timer, | |
| 203 | }); | |
| 204 | for (const n of ["r0", "r1", "r2", "r3"]) await g.started(n); | |
| 205 | g.answer("r0"); | |
| 206 | g.answer("r1"); | |
| 207 | // Their answers reach the batch in microtasks; a real timer fires after them. | |
| 208 | await new Promise<void>((r) => setTimeout(r, 0)); | |
| 209 | // r2 and r3 are still waiting on git; the ceiling passes. | |
| 210 | first.fire(); | |
| 211 | const out = await search; | |
| 212 | expect(first.ms()).toBe(BUDGETED_GREP_MS); | |
| 213 | expect(first.cleared()).toBe(1); | |
| 214 | expect(out.more).toBe(true); | |
| 215 | expect(out.hits.map((h) => h.repo.name)).toEqual(["r0", "r1"]); | |
| 216 | // No later batch started, and its budget timer was never asked for. | |
| 217 | expect(later.asked()).toBe(0); | |
| 218 | expect(g.running().sort()).toEqual(["r2", "r3"]); | |
| 219 | // The abandoned greps keep their slots until git exits. | |
| 220 | expect(slots.held()).toEqual({ total: 2, strangers: 2 }); | |
| 221 | g.answer("r2"); | |
| 222 | g.answer("r3"); | |
| 223 | for (let k = 0; k < 10 && slots.held().total > 0; k++) await Promise.resolve(); | |
| 224 | expect(slots.held()).toEqual({ total: 0, strangers: 0 }); | |
| 225 | }); | |
| 226 | ||
| 227 | test("a search with no budget never takes the first batch's ceiling", async () => { | |
| 228 | const g = deferredGreps(); | |
| 229 | const first = handTimer(); | |
| 230 | const search = grepRepos("/nowhere", [0, 1].map(repo), "q", 200, Infinity, { | |
| 231 | slots: createGrepSlots(), | |
| 232 | grep: g.grep, | |
| 233 | firstTimer: first.timer, | |
| 234 | }); | |
| 235 | for (const n of ["r0", "r1"]) await g.started(n); | |
| 236 | for (const n of ["r0", "r1"]) g.answer(n); | |
| 237 | expect((await search).hits.length).toBe(2); | |
| 238 | expect(first.asked()).toBe(0); | |
| 239 | }); |