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

Production's idle `forge-lag` stays under 100 ms, read and fixed from `/healthz`'s `loopWorstWhat` #11197

Merged opened this

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

Merged

torque/forge:torque/11119-production-s-idle-forge-lag-stays-under → main 4 commits fork

  • Merged. torque/11119-production-s-idle-forge-lag-stays-under was merged into main as b10f353.

Review

Sign in to review this change.
3 files changed +159 −32
src/actions/crew_now.ts +32 −25
@@ -6,8 +6,8 @@
66 */
77 import { mayViewId } from "../core/sealed.ts";
88 import { getIssueById, listOpenAgentPullsVisibleTo } from "../core/issues.ts";
9 import { waitsForPerson } from "../crew/review.ts";
10 import { getPull } from "../core/pulls.ts";
9 import { reviewWaitsFor } from "../crew/review.ts";
10 import { joinMany } from "../core/pulls.ts";
1111 import { ownerFactRepoIds } from "../core/repos.ts";
1212 import { reviewSays } from "../crew/review_says.ts";
1313 import { reviewerPaused } from "../crew/reviewer_paused.ts";
@@ -56,29 +56,36 @@ export function crewNow(ctx: Pick<Ctx, "app">, user: User) {
5656 }));
5757 // An agent's green pull that waits for a person's review (#597): no
5858 // reviewer on the crew, or one that lapsed — with its age, never silent.
59 const ownRepos = repos.filter((r) => owned.has(r.id));
60 const reviewWaits = listOpenAgentPullsVisibleTo(ctx.app.db, user, 200)
61 .filter((i) => ownRepos.some((r) => r.id === i.repoId))
62 .flatMap((issue) => {
63 const pull = getPull(ctx.app.db, issue.repoId, issue.number);
64 const repo = byId.get(issue.repoId);
65 const wait = pull && repo ? waitsForPerson(ctx.app.db, repo.id, pull) : undefined;
66 return wait === undefined || repo === undefined
67 ? []
68 : [
69 {
70 run: null,
71 app: `${repo.ownerName}/${repo.name}`,
72 who: "you",
73 on: `#${issue.number} ${issue.title}`,
74 why: "review",
75 state: "waiting for your review",
76 says: reviewSays(wait, reviewerPaused(repo)),
77 since: wait.since,
78 costUsd: 0,
79 },
80 ];
81 });
59 // Read for the whole set at once (#11119): the pull rows in one `IN`, and
60 // `reviewWaitsFor` reads holds, verdicts, hand-overs and reviewers once
61 // for every pull, where one pull at a time was ~11 statements a pull,
62 // 2,200 for the 200 this lists, on every forge-view build.
63 const pulls = joinMany(
64 ctx.app.db,
65 listOpenAgentPullsVisibleTo(ctx.app.db, user, 200).filter((i) => owned.has(i.repoId)),
66 ).flatMap((pull) => {
67 const repo = byId.get(pull.repoId);
68 return repo === undefined ? [] : [{ repo, pull }];
69 });
70 const waits = reviewWaitsFor(ctx.app.db, pulls);
71 const reviewWaits = pulls.flatMap(({ repo, pull }) => {
72 const wait = waits.get(pull.id)?.person;
73 return wait === undefined
74 ? []
75 : [
76 {
77 run: null,
78 app: `${repo.ownerName}/${repo.name}`,
79 who: "you",
80 on: `#${pull.number} ${pull.title}`,
81 why: "review",
82 state: "waiting for your review",
83 says: reviewSays(wait, reviewerPaused(repo)),
84 since: wait.since,
85 costUsd: 0,
86 },
87 ];
88 });
8289 return [
8390 ...waiting,
8491 ...reviewWaits,
test/crew-now-review-waits.test.ts added +116
@@ -0,0 +1,116 @@
1 /**
2 * The crew's "waiting for your review" rows (#597) are read for every open
3 * agent pull at once (#11119): production's `loopWorstWhat` named
4 * GET /api/v1/forge/view in 14 of 20 readings, and its `crewNow` read each
5 * pull one at a time, ~11 statements a pull. The row still appears, and the
6 * statements one build costs do not grow with the pulls it lists.
7 */
8 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
9 import { mkdtemp, rm } from "node:fs/promises";
10 import { tmpdir } from "node:os";
11 import { join } from "node:path";
12 import { crewNow } from "../src/actions/crew_now.ts";
13 import { createAgent } from "../src/actions/crew.ts";
14 import { openPull } from "../src/actions/pulls.ts";
15 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
16 import { createApp, type App } from "../src/app.ts";
17 import { createApp as createAppRecord } from "../src/core/apps.ts";
18 import type { Repo } from "../src/core/repos.ts";
19 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
20 import { getUserByName, type User } from "../src/core/users.ts";
21 import { seedAdmin } from "./helpers/admin.ts";
22 import { quietLogs } from "./helpers/globals.ts";
23
24 quietLogs();
25
26 describe("crewNow's review waits", () => {
27 let dataDir = "";
28 let app: App;
29 let ada: User;
30 let repo: Repo;
31 let agentId = "";
32 let n = 0;
33
34 /** An agent's pull whose check went green two hours ago, with no reviewer on the crew. */
35 const greenAgentPull = async () => {
36 const branch = `b${++n}`;
37 const c = await commitToBranch(app, ada, repo, {
38 branch,
39 base: "main",
40 message: branch,
41 files: [{ path: `${branch}.md`, content: "x\n" }],
42 });
43 if (!c.ok) throw new Error(JSON.stringify(c.error));
44 const o = await openPull(app, ada, repo, { title: `pull ${branch}`, body: "", baseBranch: "main", headBranch: branch });
45 if (!o.ok) throw new Error(String(o.error));
46 app.db.query("UPDATE issue SET author_id = ?2 WHERE id = ?1").run(o.value.id, agentId);
47 const run = createRun(app.db, {
48 repoId: repo.id,
49 kind: "check",
50 issueId: o.value.id,
51 env: `pull-${o.value.number}`,
52 commitOid: o.value.headOid,
53 reason: "pull",
54 });
55 claimRun(app.db, run.id);
56 finishRun(app.db, run.id, { state: "ok", summary: "ok" });
57 const ago = new Date(Date.now() - 2 * 3_600_000).toISOString();
58 app.db.query("UPDATE run SET created_at = ?2, finished_at = ?2 WHERE id = ?1").run(run.id, ago);
59 return o.value;
60 };
61
62 /** The rows of one build, and the statements it prepared. */
63 const build = () => {
64 const prepared: string[] = [];
65 const original = app.db.query.bind(app.db);
66 (app.db as unknown as { query: unknown }).query = (sql: string) => {
67 prepared.push(sql);
68 return original(sql);
69 };
70 try {
71 return { rows: crewNow({ app }, ada), statements: prepared.length };
72 } finally {
73 (app.db as unknown as { query: unknown }).query = original;
74 }
75 };
76
77 beforeAll(async () => {
78 dataDir = await mkdtemp(join(tmpdir(), "crew-now-waits-"));
79 app = createApp({ dataDir, port: 0 });
80 const made = await seedAdmin(app, { username: "ada", email: "[email protected]" });
81 if (!made.ok) throw new Error(made.error);
82 ada = made.value;
83 const r = await createRepo(app, ada, { name: "garden", visibility: "private" });
84 if (!r.ok) throw new Error(String(r.error));
85 repo = r.value;
86 createAppRecord(app.db, { repoId: repo.id, slug: "garden", access: "private" });
87 const seeded = await commitToBranch(app, ada, repo, {
88 message: "seed",
89 files: [{ path: "README.md", content: "# garden\n" }],
90 });
91 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
92 const agent = await createAgent(app, ada, { username: "quill" });
93 if (!agent.ok) throw new Error(String(agent.error));
94 agentId = getUserByName(app.db, "quill")!.id;
95 }, 30_000);
96
97 afterAll(async () => {
98 app.close();
99 await rm(dataDir, { recursive: true, force: true });
100 });
101
102 test("a green agent pull with no reviewer is a row waiting for the owner's review", async () => {
103 const pull = await greenAgentPull();
104 const { rows } = build();
105 const row = rows.find((r) => r.on === `#${pull.number} ${pull.title}`);
106 expect(row).toMatchObject({ run: null, app: "ada/garden", who: "you", why: "review", state: "waiting for your review" });
107 });
108
109 test("one build's statements are the same for 1 pull as for 9", async () => {
110 const one = build();
111 for (let i = 0; i < 8; i++) await greenAgentPull();
112 const nine = build();
113 expect(nine.rows.filter((r) => r.why === "review")).toHaveLength(9);
114 expect(nine.statements).toBe(one.statements);
115 }, 30_000);
116 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs