mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
ci-audit: lesson 1 names a job held back on its own label; planted repo runs PR jobs on Ubicloud
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
6745c691e8
commit
581de0bed4
3 files changed
+40
-13
No files matched your search
@@ -36,8 +36,14 @@ export function lessonRunners(c: LessonContext): Lesson {
|
||||
const slow = priced.filter(({ g }) => (g.wait.p90 ?? 0) > 60 && (g.wait.p90 ?? 0) > (g.run.p50 ?? 0));
|
||||
if (!slow.length) return mk(1, 'pass', `every job's p90 wait is under a minute or below its own running time`, { evidence: groups.slice(0, 3).map(g => `${g.key}: wait p50 ${fmt(g.wait.p50)}, p90 ${fmt(g.wait.p90)} (n=${g.n})`) });
|
||||
const burst = isBurst(slow.flatMap(({ g }) => g.longWaits));
|
||||
const fastSibling = (g: typeof groups[number]) => groups.find(o => o !== g && o.labels === g.labels && (o.wait.p90 ?? 0) <= 60);
|
||||
const pinned = slow.filter(({ g }) => fastSibling(g));
|
||||
const hosted = slow.filter(({ p }) => p.kind === 'github-hosted');
|
||||
const evidence = slow.slice(0, 5).map(({ g, p }) => `${g.key} on ${p.label}: wait p50 ${fmt(g.wait.p50)}, p90 ${fmt(g.wait.p90)} for ${fmt(g.run.p50)} of work; ${g.longWaits.length} of ${g.n} waited over a minute`);
|
||||
if (pinned.length === slow.length && !burst) {
|
||||
const { g } = pinned[0]!;
|
||||
return mk(1, 'finding', `${g.key} waits for a machine (p90 ${fmt(g.wait.p90)}) while ${fastSibling(g)!.key} on the same runner label starts in seconds`, { evidence, fix: `look for what holds only this job back: a concurrency group, a required or reusable workflow, or a runner-group limit on ${g.labels}; a faster runner would not help` });
|
||||
}
|
||||
if (burst) return mk(1, 'finding', 'long waits cluster in a few hours: the repo starts more jobs at once than its runner pool takes', { evidence, fix: 'cap concurrency (concurrency groups with cancel-in-progress, fewer parallel jobs during bursts) before changing runners' });
|
||||
const weekly = hosted.reduce((a, { g }) => a + (g.jobMinutes / Math.max(1, c.m.windowDays)) * 7, 0);
|
||||
const caution = !c.data.isPrivate && hosted.length ? 'a third-party runner executes fork PR code on its machines: review that before moving pull_request jobs' : undefined;
|
||||
@@ -154,7 +160,7 @@ export function lessonParallel(c: LessonContext): Lesson {
|
||||
const slowest = c.m.groups.filter(g => g.workflow === top.workflow).sort((a, b) => (b.wait.p50 ?? 0) + (b.setup.p50 ?? 0) + (b.run.p50 ?? 0) - ((a.wait.p50 ?? 0) + (a.setup.p50 ?? 0) + (a.run.p50 ?? 0)))[0];
|
||||
const waiting = slowest && (slowest.wait.p50 ?? 0) > (slowest.setup.p50 ?? 0) + (slowest.run.p50 ?? 0);
|
||||
const late = `${top.workflow} finishes last on ${Math.round(top.share * 100)}% of PR heads, a median ${fmt(gap)} after everything else`;
|
||||
return mk(7, 'finding', waiting ? `${late}, mostly waiting for a machine (${slowest!.key}: wait p50 ${fmt(slowest!.wait.p50)} for ${fmt((slowest!.setup.p50 ?? 0) + (slowest!.run.p50 ?? 0))} of work)` : late, { evidence, fix: waiting ? `move ${top.workflow}'s jobs to the same fast runner pool as the rest (lesson 1); splitting a job that mostly waits would not help` : `run everything in parallel and split ${top.workflow}'s slowest job (move unrelated steps into their own job)`, saving: { prMinutes: +((gap / 60) * top.share).toFixed(1), jobMinutesPerWeek: null, usdPerMonth: null, measured: true, note: 'upper bound per PR: the slowest check can finish no earlier than the next one' } });
|
||||
return mk(7, 'finding', waiting ? `${late}, mostly waiting for a machine (${slowest!.key}: wait p50 ${fmt(slowest!.wait.p50)} for ${fmt((slowest!.setup.p50 ?? 0) + (slowest!.run.p50 ?? 0))} of work)` : late, { evidence, fix: waiting ? `fix the wait (lesson 1 says where it comes from); splitting a job that mostly waits would not help` : `run everything in parallel and split ${top.workflow}'s slowest job (move unrelated steps into their own job)`, saving: { prMinutes: +((gap / 60) * top.share).toFixed(1), jobMinutesPerWeek: null, usdPerMonth: null, measured: true, note: 'upper bound per PR: the slowest check can finish no earlier than the next one' } });
|
||||
}
|
||||
|
||||
/** A test-runner concurrency flag in a run line (bun --max-concurrency, jest --maxWorkers, vitest --maxConcurrency, make -j). */
|
||||
|
||||
+22
-2
@@ -4,7 +4,7 @@ import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { EXIT, main } from '../lib/ci-audit/cli';
|
||||
import type { Run } from '../lib/ci-audit/collect';
|
||||
import { concurrencyFlag } from '../lib/ci-audit/lessons';
|
||||
import { concurrencyFlag, lessonRunners } from '../lib/ci-audit/lessons';
|
||||
import { cohort, measure, phases } from '../lib/ci-audit/measure';
|
||||
import { priceRunner } from '../lib/ci-audit/prices';
|
||||
import { ISSUE_MARKER, planIssue } from '../lib/ci-audit/publish';
|
||||
@@ -89,7 +89,8 @@ describe('planted repo (three problems, two decoys)', () => {
|
||||
expect(lesson(6).summary).toContain('no cache is saved from main');
|
||||
expect(lesson(6).summary).toContain('100% of sampled cache restores missed');
|
||||
expect(lesson(7).summary).toMatch(/lint\.yml finishes last on 100% of PR heads, a median 3\.\d min after everything else, mostly waiting for a machine/);
|
||||
expect(lesson(7).fix).toContain('fast runner pool');
|
||||
expect(lesson(7).fix).toContain('fix the wait');
|
||||
expect(lesson(1).fix).toContain('same fast runner pool');
|
||||
expect(lesson(1).evidence.join(' ')).toMatch(/lint on ubuntu-24\.04: wait p50 8\.0 min/);
|
||||
expect(lesson(8).summary).toContain('no test exercises the skip logic');
|
||||
const all = JSON.stringify(r.lessons);
|
||||
@@ -170,6 +171,24 @@ describe('job phases', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('lesson 1', () => {
|
||||
const group = (key: string, labels: string, waitP90: number, runP50: number) => ({ key, workflow: key, job: key, labels, n: 10, wait: { n: 10, p50: waitP90 / 2, p90: waitP90 }, setup: { n: 10, p50: 5, p90: 5 }, run: { n: 10, p50: runP50, p90: runP50 }, longWaits: waitP90 > 60 ? Array.from({ length: 10 }, (_, i) => `2026-10-${10 + i}T0${i % 10}:00:00Z`) : [], jobMinutes: 1, billedMinutes: 10 });
|
||||
const ctx = (groups: any[]) => ({ data: { isPrivate: false }, m: { groups, windowDays: 7, sampledRuns: 10 }, st: null } as any);
|
||||
|
||||
test('a GitHub-hosted job that waits while the rest run on a faster pool: move it', () => {
|
||||
const l = lessonRunners(ctx([group('lint', 'ubuntu-24.04', 500, 5), group('test', 'ubicloud-standard-2', 20, 180)]));
|
||||
expect(l.state).toBe('finding');
|
||||
expect(l.fix).toContain('same fast runner pool');
|
||||
});
|
||||
|
||||
test('a job that waits while another job on the same label starts at once: a faster runner would not help', () => {
|
||||
const l = lessonRunners(ctx([group('lint', 'ubuntu-24.04', 500, 5), group('test', 'ubuntu-24.04', 8, 180)]));
|
||||
expect(l.state).toBe('finding');
|
||||
expect(l.summary).toContain('on the same runner label starts in seconds');
|
||||
expect(l.fix).toContain('a faster runner would not help');
|
||||
});
|
||||
});
|
||||
|
||||
describe('prices', () => {
|
||||
test('GitHub standard runners are free on public repos; Ubicloud bills by VM family', () => {
|
||||
expect(priceRunner('ubuntu-24.04', false).usdPerMinute).toBe(0);
|
||||
@@ -254,6 +273,7 @@ describe('Saturday workflow template', () => {
|
||||
expect(doc.on.schedule).toEqual([{ cron: '0 15 * * 6' }]);
|
||||
expect('workflow_dispatch' in doc.on).toBe(true);
|
||||
expect(doc.permissions).toEqual({ actions: 'read', contents: 'read', issues: 'write' });
|
||||
expect(doc.jobs.audit.if).toBe('github.ref_name == github.event.repository.default_branch');
|
||||
});
|
||||
|
||||
test('audits within the GITHUB_TOKEN hourly limit and always publishes the outcome', () => {
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
/**
|
||||
* A synthetic GitHub repo for /ci-audit with three planted problems and two decoys.
|
||||
* planted: no cache saved from main (every PR restore misses); a GitHub-hosted
|
||||
* 5-second lint job that waits ~8 min for a machine and finishes last;
|
||||
* 5-second lint job that waits ~8 min for a machine and finishes last
|
||||
* while every other PR job runs on Ubicloud and starts in ~15 s;
|
||||
* a path-filtered e2e workflow that no test exercises.
|
||||
* decoys: a full-history checkout that already uses filter: blob:none; a
|
||||
* windows job on main that waits over a minute but runs for ten.
|
||||
@@ -25,7 +26,7 @@ on:
|
||||
pull_request:
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-24.04
|
||||
runs-on: ubicloud-standard-2
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-node@v4
|
||||
@@ -35,12 +36,12 @@ jobs:
|
||||
- run: npm ci
|
||||
- run: npm test
|
||||
typecheck:
|
||||
runs-on: ubuntu-24.04
|
||||
runs-on: ubicloud-standard-2
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- run: npx tsc --noEmit
|
||||
release-notes:
|
||||
runs-on: ubuntu-24.04
|
||||
runs-on: ubicloud-standard-2
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
@@ -64,7 +65,7 @@ on:
|
||||
paths: ['src/**']
|
||||
jobs:
|
||||
e2e:
|
||||
runs-on: ubuntu-24.04
|
||||
runs-on: ubicloud-standard-2
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- run: npm ci
|
||||
@@ -111,17 +112,17 @@ function synth(): Data {
|
||||
const sha = `${(0xa000 + i).toString(16)}${'0'.repeat(36)}`;
|
||||
const pr = 100 + i;
|
||||
const ci = run('ci.yml', 'pull_request', sha, T, pr);
|
||||
const test = job(ci.id, 'test', 'ubuntu-24.04', T + 3000, 5 + (i % 4), [['Set up job', 2], ['Run actions/checkout@v4', 3], ['Run actions/setup-node@v4', 25], ['Run npm ci', 40 + (i % 5)], ['Run npm test', 150 + (i * 7) % 50], ['Complete job', 1]]);
|
||||
const test = job(ci.id, 'test', 'ubicloud-standard-2', T + 3000, 15 + (i % 4), [['Set up job', 2], ['Run actions/checkout@v4', 3], ['Run actions/setup-node@v4', 25], ['Run npm ci', 40 + (i % 5)], ['Run npm test', 150 + (i * 7) % 50], ['Complete job', 1]]);
|
||||
logs[test.j.id] = [`${iso(T)} Cache not found for input keys: node-cache-Linux-x64-npm-${i}`, `${iso(T)} npm ci finished`].join('\n');
|
||||
const tc = job(ci.id, 'typecheck', 'ubuntu-24.04', T + 3000, 4 + (i % 3), [['Set up job', 2], ['Run actions/checkout@v4', 3], ['Run npx tsc --noEmit', 12], ['Complete job', 1]]);
|
||||
const rn = job(ci.id, 'release-notes', 'ubuntu-24.04', T + 3000, 4, [['Set up job', 2], ['Run actions/checkout@v4', 4], ['Run git log --oneline -20', 1], ['Complete job', 1]]);
|
||||
const tc = job(ci.id, 'typecheck', 'ubicloud-standard-2', T + 3000, 14 + (i % 3), [['Set up job', 2], ['Run actions/checkout@v4', 3], ['Run npx tsc --noEmit', 12], ['Complete job', 1]]);
|
||||
const rn = job(ci.id, 'release-notes', 'ubicloud-standard-2', T + 3000, 14, [['Set up job', 2], ['Run actions/checkout@v4', 4], ['Run git log --oneline -20', 1], ['Complete job', 1]]);
|
||||
setOnly(ci, Math.max(test.end, tc.end, rn.end));
|
||||
const lint = run('lint.yml', 'pull_request', sha, T, pr);
|
||||
const lj = job(lint.id, 'lint', 'ubuntu-24.04', T + 2000, 420 + (i * 37) % 120, [['Set up job', 1], ['Run actions/checkout@v4', 2], ['Run npx oxlint', 5], ['Complete job', 1]]);
|
||||
setOnly(lint, lj.end);
|
||||
if (i % 2 === 0) {
|
||||
const e2e = run('e2e.yml', 'pull_request', sha, T, pr);
|
||||
const ej = job(e2e.id, 'e2e', 'ubuntu-24.04', T + 2000, 6, [['Set up job', 2], ['Run actions/checkout@v4', 3], ['Run npm ci', 40], ['Run npm run e2e', 120], ['Complete job', 1]]);
|
||||
const ej = job(e2e.id, 'e2e', 'ubicloud-standard-2', T + 2000, 16, [['Set up job', 2], ['Run actions/checkout@v4', 3], ['Run npm ci', 40], ['Run npm run e2e', 120], ['Complete job', 1]]);
|
||||
setOnly(e2e, ej.end);
|
||||
}
|
||||
if (i % 12 === 5) {
|
||||
@@ -202,7 +203,7 @@ const near = (a: string, b: string, gap = 240) => new RegExp(`(${a})[^\\n]{0,${g
|
||||
export const PLANTED = [
|
||||
{ id: 'unseeded-cache', re: near('cache', 'main\\b|default branch|miss|not found|cold|seed') },
|
||||
{ id: 'lint-waits-last', re: near('lint', '(wait|queue|machine|runner)[^\\n]{0,160}\\d+(\\.\\d+)?\\s*(min|minutes|m\\b|s\\b|sec)') },
|
||||
{ id: 'untested-path-filter', re: near('e2e|path[- ]?filter|paths:|skip logic|skips? (PRs|runs)', 'no test|untested|not tested|nothing tests|without (a |any )?test|no .{0,20}test (covers|exercises)') },
|
||||
{ id: 'untested-path-filter', re: near('e2e|path[- ]?filter|paths:|skip logic|skips? (PRs|runs)', 'no test|untested|not tested|nothing (tests|checks|verifies)|without (a |any )?test|no .{0,20}(test|check) (covers|exercises|checks)') },
|
||||
] as const;
|
||||
|
||||
/** Decoys a reply must not present as something to fix. */
|
||||
|
||||
Reference in new issue
Block a user