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

Sentinel: release was degraded 190 minutes today: Release 0.7.509 is unproven: https://forge.open-platform.sh/search?q=a as desktop /search?q=a took 2511 ms, over 1000 m… #10742

Merged opened this

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

Merged

scout/forge:scout/10730-sentinel-release-was-degraded-190-minute → main 3 commits fork

  • Merged. scout/10730-sentinel-release-was-degraded-190-minute was merged into main as 3171d47.

Review

Sign in to review this change.
5 files changed +74 −16
src/actions/search.ts +12 −11
@@ -184,17 +184,18 @@ export async function search(
184184 if (wants("issues")) phase("issues", performance.now() - issuesAt);
185185
186186 const codeAt = performance.now();
187 const code = wants("code") && input.withCode !== false
188 ? await grepRepos(
189 app.reposRoot,
190 visible,
191 q,
192 Math.min(limit * 5, MAX_CODE_HITS),
193 input.codeBudgetMs ?? Infinity,
194 // The slot is keyed on the actor the session resolved, never on input.
195 { memberId: actor?.id },
196 )
197 : { hits: [], truncated: false, more: false, skipped: false };
187 const code =
188 wants("code") && input.withCode !== false
189 ? await grepRepos(
190 app.reposRoot,
191 visible,
192 q,
193 Math.min(limit * 5, MAX_CODE_HITS),
194 input.codeBudgetMs ?? Infinity,
195 // The slot is keyed on the actor the session resolved, never on input.
196 { memberId: actor?.id },
197 )
198 : { hits: [], truncated: false, more: false, skipped: false };
198199 if (wants("code")) phase("code", performance.now() - codeAt);
199200
200201 return {
src/core/search.ts +12 −5
@@ -444,6 +444,7 @@ async function grepRun(
444444 query: string,
445445 maxCount: number,
446446 timeoutMs = GREP_TIMEOUT_MS,
447 signal?: AbortSignal,
447448 ): Promise<RawHit[] | null | "timeout"> {
448449 const pattern = cleanQuery(query);
449450 if (pattern === "" || !isSafeRef(ref)) return [];
@@ -467,7 +468,7 @@ async function grepRun(
467468 // `--max-count` is per file: `a` in a repository of thousands of files
468469 // is megabytes of matches. Only the first `maxCount` are kept, so git is
469470 // stopped once they are read rather than left to write the rest (#10395).
470 { cwd: dir, timeoutMs, stopAfterLines: maxCount },
471 { cwd: dir, timeoutMs, stopAfterLines: maxCount, ...(signal === undefined ? {} : { signal }) },
471472 );
472473 // Exit 1 is "no match" — data, not failure. Anything else (a repo with no
473474 // commits, a killed process) is simply no results from this repo.
@@ -541,13 +542,14 @@ async function grepCached(
541542 query: string,
542543 who: GrepWho,
543544 timeoutMs = GREP_TIMEOUT_MS,
545 signal?: AbortSignal,
544546 ): Promise<RawHit[] | "skipped" | "timeout"> {
545547 const ref = repo.defaultBranch;
546548 const oid = await branchOid(dir, ref);
547549 const slotted = async (at: string): Promise<RawHit[] | null | "skipped" | "timeout"> => {
548550 const got = await (who.slots ?? grepSlots).run(who.memberId, () => {
549551 grepsRun++;
550 return (who.grep ?? grepRun)(dir, at, query, MAX_HITS_PER_REPO, timeoutMs);
552 return (who.grep ?? grepRun)(dir, at, query, MAX_HITS_PER_REPO, timeoutMs, signal);
551553 });
552554 return got.ran ? got.value : "skipped";
553555 };
@@ -612,9 +614,8 @@ function budgetTimer(ms: number): BudgetTimer {
612614 * a busy forge holding the page; what answered by then is kept, the rest is
613615 * `more` (#9402). Each later batch races the time
614616 * left: when the budget runs out first the answer goes back with `more`, and
615 * the batch's greps are left to finish on their own. They keep the slot
616 * `search/grep_slots.ts` gave them until git exits — that is the only cap on
617 * abandoned greps, and it is charged to the caller who started them.
617 * the batch's greps are killed, so no abandoned git outlives the answer
618 * (#10730); the slot `search/grep_slots.ts` gave them is freed as git exits.
618619 */
619620 export async function grepRepos(
620621 reposRoot: string,
@@ -639,8 +640,12 @@ export async function grepRepos(
639640 // Set when this search answers at its budget: the batch it left behind
640641 // starts no further pair. Local to this call, never shared between callers.
641642 let answered = false;
643 // Fired when this search answers at its budget: the greps it leaves behind
644 // are killed (and not cached), so they stop holding git's gate and a slot.
645 const abandon = new AbortController();
642646 const more = () => {
643647 answered = true;
648 abandon.abort();
644649 return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: true, skipped };
645650 };
646651 // Each grep writes its answer into its own place as it settles, so a batch
@@ -655,6 +660,7 @@ export async function grepRepos(
655660 query,
656661 who,
657662 timeoutMs,
663 abandon.signal,
658664 ).then((got) => {
659665 found[k + j] = got;
660666 }),
@@ -685,6 +691,7 @@ export async function grepRepos(
685691 timer.clear();
686692 if (raced === "cut") {
687693 answered = true;
694 abandon.abort();
688695 cut = true;
689696 }
690697 }
src/git/env.ts +9
@@ -322,6 +322,8 @@ export interface CaptureOptions extends RunOptions {
322322 * answers of a command whose whole output can be megabytes (`git grep a`).
323323 */
324324 stopAfterLines?: number;
325 /** Kill the process when this fires (reported as `timedOut`); one already fired never starts it. */
326 signal?: AbortSignal;
325327 }
326328
327329 /**
@@ -375,6 +377,7 @@ export async function runGitCapture(
375377 ): Promise<GitCapture> {
376378 const start = performance.now();
377379 return gitGate.run(async () => {
380 if (options.signal?.aborted === true) return { code: 1, stdout: "", stderr: "", timedOut: true };
378381 const proc = Bun.spawn(["git", ...args], {
379382 ...(options.cwd === undefined ? {} : { cwd: options.cwd }),
380383 env: gitEnv(options.env),
@@ -390,6 +393,11 @@ export async function runGitCapture(
390393 },
391394 options.timeoutMs ?? 30_000,
392395 );
396 const abort = () => {
397 timedOut = true;
398 proc.kill("SIGKILL");
399 };
400 options.signal?.addEventListener("abort", abort, { once: true });
393401 try {
394402 const [out, stderr, code] = await Promise.all([
395403 readLines(proc.stdout, options.stopAfterLines, () => proc.kill("SIGKILL")),
@@ -405,6 +413,7 @@ export async function runGitCapture(
405413 };
406414 } finally {
407415 clearTimeout(timer);
416 options.signal?.removeEventListener("abort", abort);
408417 noteGit(gitRecord, args, start);
409418 }
410419 });
test/git-capture-signal.test.ts added +19
@@ -0,0 +1,19 @@
1 import { expect, test } from "bun:test";
2 import { runGitCapture } from "../src/git/env.ts";
3
4 test("an aborted signal kills a running git and reports it as timed out (#10730)", async () => {
5 const stop = new AbortController();
6 const started = performance.now();
7 const run = runGitCapture(["-c", "alias.nap=!sleep 3 >/dev/null 2>&1 #", "nap"], { timeoutMs: 30_000, signal: stop.signal });
8 setTimeout(() => stop.abort(), 100);
9 const out = await run;
10 expect(out.timedOut).toBe(true);
11 expect(performance.now() - started).toBeLessThan(5_000);
12 });
13
14 test("a signal already aborted starts no git", async () => {
15 const stop = new AbortController();
16 stop.abort();
17 const out = await runGitCapture(["-c", "alias.nap=!sleep 3 >/dev/null 2>&1 #", "nap"], { signal: stop.signal });
18 expect(out.timedOut).toBe(true);
19 });
test/search-batch-race.test.ts +22
@@ -237,3 +237,25 @@ test("a search with no budget never takes the first batch's ceiling", async () =
237237 expect((await search).hits.length).toBe(2);
238238 expect(first.asked()).toBe(0);
239239 });
240
241 test("a grep left behind at the budget is told to stop (#10730)", async () => {
242 const signals: AbortSignal[] = [];
243 const first = handTimer();
244 // Hangs until told to stop, as a git that the answer no longer needs.
245 const grep = ((_d: string, _a: string, _q: string, _n: number, _t: number, signal?: AbortSignal) => {
246 signals.push(signal!);
247 return new Promise<Raw>((done) => signal!.addEventListener("abort", () => done(null)));
248 }) as never;
249 const search = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 10, {
250 slots: createGrepSlots(),
251 grep,
252 now: () => 0,
253 firstTimer: first.timer,
254 });
255 while (signals.length < 4) await new Promise((r) => setTimeout(r, 1));
256 expect(signals.some((s) => s.aborted)).toBe(false);
257 first.fire();
258 const out = await search;
259 expect(out.more).toBe(true);
260 expect(signals.every((s) => s.aborted)).toBe(true);
261 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs