Skip to content
This repository was archived by the owner on Sep 17, 2026. It is now read-only.

Shard itest ingot across three runners, and record what it does not fix - #21

Closed
Peeja wants to merge 4 commits into
mainfrom
claude/shard-itest
Closed

Peeja wants to merge 4 commits into
mainfrom
claude/shard-itest

Conversation

@Peeja

@Peeja Peeja commented Sep 17, 2026 •

Copy link
Copy Markdown

⏭️ Safe to merge without waiting for CI — all of it, at this head

Refreshed on every push. Checks are not required yet, so this is advice, not a gate.

check why this diff cannot reach it
itest ×4 Already proven green on f4c5c21f (run 35240022926, 4/4). git diff f4c5c21f HEAD -- .github/ is empty — every commit since changes only MONOREPO_TODO.md.
ci (guards + 13 unit) Reads neither changed file. No Go package imports itest.yml; no guard script parses it.
images ×8 Builds from Dockerfiles and images.yml; touches neither changed file.
e2e Has its own e2e.yml; does not read itest.yml.

Derived, not asserted. grep -rn 'itest.yml\|MONOREPO_TODO' --include='*.yml' --include='*.sh' --include='*.go' --include=Makefile . returns three hits, all of them prose in comments (e2e.yml:136, check-base-images.sh:12, forge_encryption_test.go:600). Nothing consumes either file.

What this costs if it's wrong: the checks still run; we just don't wait. If an ignored check goes red after merge, main is red. That is the trade, and it is why the reasoning is re-derived per push rather than encoded as a paths: list that goes stale.


itest ingot was ~28 minutes and had become a drag on every PR. Two files, based on main, stacked on nothing.

Does not need rebasing despite main having moved to 586738ed: this touches only itest.yml and MONOREPO_TODO.md, and #17 and #20 touched neither.

✅ It ran, and it did less than I predicted

Run 35240022926 is green on all four jobs. And #19's unsharded run finished four minutes later on the same runner pool, so this is a real A/B rather than a comparison against last night's number:

unsharded (#19, run 35239716032) sharded (this, run 35240022926)
itest workflow, end to end 30m43s 23m07s
slowest job 29m29s itest ingot 17m07s itest ingot 1/3
test binary ok …/ingot/itest 1267.853s 628.841s + 462.731s + 204.654s
runner-minutes, ingot side 29m29s 42m04s

~25% off the wall clock, not the "roughly half" I claimed when I opened this. 6fb033cd replaces the estimates in MONOREPO_TODO.md with the measurement; the workflow itself is unchanged from the green run.

Total test work is unchanged: 1296.2s against 1267.9s, +2.2%. That is the point rather than a disappointment — nothing got cheaper, it got spread — but it bounds every remaining option.

Where the other half went

  1. The shards are unbalanced — 629s / 463s / 205s — and the critical path is the slowest, not the mean. The round-robin splits by test name order, which silently assumes uniform cost. Shard 1 drew TestForgeVersity (hundreds of subtests, dozens of them 3-second object-lock retention waits) plus TestForgeMultipartExpiryShred and TestForgeReadAfterEviction, both TTL-bound. Shard 3 drew four cheap ones. Perfect balance would be 432s, so imbalance alone costs 3m17s.
  2. Queue wait of 2m25s–4m47s per shard, 4m47s on the critical one. Four concurrent jobs where there were two, and they did not all get runners at once. Pure loss, and it grows with shard count.
  3. The ~6 minute image build is per job and did not move — sharding triplicated it.

It also falsifies the boot arithmetic I gave you twice

Shard 3 ran four tests, so four boots, in 204.654s. A boot is therefore at most ~51s, not the ~80s I derived from 1257/13 — so the "1040s booting, 217s working" decomposition was wrong, and the shared stack is worth less than the 3–4 minutes I last estimated, not more. Parked: it changes test isolation in the one job that exists to catch flakiness, for less than was claimed.

Two levers nobody had looked at (e0205346)

Both found by reading source after the measurement pointed at them:

  • itest and e2e have no Docker layer cache, and never decided not to. itest.yml:143 and e2e.yml:118 shell out to plain docker build, which cannot use --cache-from type=gha at all; only images.yml uses buildx. The "No caching, deliberately (2026-09-11)" comment is in images.yml and reasons about that workflow — "the point is to provoke build failures, not to avoid them". That does not obviously carry where the build is a means to running tests, and images.yml already proves the cold build on the same commit on every PR. Keep images.yml uncached as the canary, cache the other two: up to ~6 min off each of four jobs, for a handful of lines.
  • lockWaitTime is 3s in our own versitygw fork and is self-imposed. tests/integration/utils.go:2654; cleanupLockedObjects sets RetainUntilDate: now + lockWaitTime and then sleeps that long waiting for the lock it just created. 38 call sites ≈ 114s of pure sleep, inside TestForgeVersity, the test that bounds the job. 3s → 1s saves ~76s; 1s is the floor until someone checks sub-second retention round-trips. Lands in versitygw, arrives here as a pin bump.

Then, in order: build-once-and-load (same ~6 min from the other side, 1–2 GB of unmeasured round-trip — try the cache first); balance the shards by measured duration (~3m17s, derived from prior timings, never hand-grouped).

The shards derive their own tests

all=$(go test -list '^Test' ./... | grep '^Test' | sort)
mine=$(printf '%s\n' "$all" | awk -v k="$SHARD" -v s="$SHARDS" 'NR % s == k % s')

That distinction is the whole design. A hand-written -run regex per shard is a list; a test added later falls out of it, and it falls out silently — a green job that ran nothing looks exactly like a green job that ran everything. Deriving it means a new test lands in a shard by itself. An empty selection fails the job loudly rather than passing.

shards: 1 selects everything (NR % 1 is always 0), so hilt takes the same code path unsharded — no second branch to keep correct.

Separate jobs also give separate Docker hosts, which stack.CleanupLeaked requires: it removes every smeltery- container, so two suites sharing a host tear each other down. That constraint is why this is more jobs rather than more parallelism inside one.

⚠️ This changes check names

itest ingot becomes itest ingot 1/3, 2/3, 3/3. A branch protection rule naming the old one will wait on a check that never reports — the same edge the replaces → guards rename had. itest hilt is unchanged.

The trade, stated plainly

~43% more runner-minutes for ~25% less wall clock. Right while pull requests are the bottleneck; wrong if runner-minutes ever become the constraint. Every service brought in-repo adds another image build to the fixed half.

Verified

Three shards split 13 tests 5/4/4; union equals the full list; no test in two shards; shards: 1 selects all 13; actionlint clean. Checked by running the real go test -list output through the same awk — and now confirmed by a green run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

`itest ingot` was ~28 minutes and had become a drag on every pull request.
Measuring first, from the job log rather than from intuition:

    the Go test binary   1257s = 20m57s   (ok .../ingot/itest 1257.018s)
    everything else      ~7 min           (setup, vet, staticcheck, 8 images)

and inside those 21 minutes, 13 top-level tests booting 13 full stacks.
stack_test.go logs the cost itself -- "booting the smelt Forge stack (~1-2
min...)". The subtests run in hundredths of a second. The suite is not slow;
booting the stack thirteen times is.

So the split is by test, and the shards derive their own tests:

    all=$(go test -list '^Test' ./... | grep '^Test' | sort)
    mine=$(... awk 'NR % s == k % s')

rather than a hand-written -run regex per shard. That distinction is the whole
design: a regex is a list, a test added later falls out of it, and it falls out
*silently* -- a green job that ran nothing looks exactly like a green job that
ran everything. Deriving it means a new test lands in a shard by itself. An
empty selection fails the job loudly instead of passing.

`shards: 1` selects everything (NR % 1 is always 0), so hilt takes the same
code path unsharded and there is no second branch to keep correct.

Cost is one stack boot per top-level test, so splitting by test count splits by
time; nothing cleverer is warranted. Separate jobs also give separate Docker
hosts, which stack.CleanupLeaked requires -- it removes every `smeltery-`
container, so two suites sharing a host tear each other down.

**This changes check names**: `itest ingot` becomes `itest ingot 1/3`, `2/3`,
`3/3`. A branch protection rule naming the old one will wait on a check that
never reports -- the same edge the `replaces` -> `guards` rename had.

What it does not fix is in MONOREPO_TODO.md rather than left implicit: the 8
images are still built three times per pull request, and sharding multiplies
that rather than reducing it; and the largest win available -- nine of the
thirteen tests could share one stack -- is untouched, because it changes test
isolation and a flaky itest is the one thing this job exists to catch.

Verified: the three shards split 13 tests 5/4/4, their union equals the full
list, no test appears twice, shards=1 selects all 13, and actionlint is clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
Recorded in the same turn the PR was opened.

The Known debt entry here had been tracking `itest ingot` as a job whose image
builds were eating its margin. Measuring the job log says otherwise: 21 of the
28 minutes is the test binary, and inside that, 13 top-level tests boot 13 full
stacks at 1-2 minutes each. The image builds are the smaller half. The entry is
replaced with the measurement rather than extended, since the trend it recorded
was reasoning from the wrong quantity.

#21 halves the wall clock by sharding. What it does not fix -- images built
three times per PR, and nine tests that could share one stack -- is a decision
with a runner-minutes-versus-engineering-time tradeoff, so it went to
MONOREPO_TODO.md rather than being taken quietly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
@Peeja

Peeja commented Sep 17, 2026

Copy link
Copy Markdown
Author

LGTM; merging when green.

The entry as first written put the shared stack at 8-16 minutes, which was
measured against the un-sharded baseline in the same breath as proposing the
sharding that removes it. Both changes attack stack boots, so they overlap
almost entirely -- and they partly cancel, because stack sharing works within a
process: nine shared tests spread across three shards boot the shared stack
three times, not once.

Backing a boot out of the one hard number -- 1257s over 13 of them, so ~80s
each -- puts ~1040s of that job in booting and ~217s in test work, which makes
the marginal gain over sharding about 3 to 4 minutes rather than 8 to 16. For a
change that alters test isolation in the job that exists to catch flakiness,
that is a much poorer trade than it first read.

It also moves which lever matters. At ~15 minutes the fixed overhead is about
half the job, and most of it is the eight image builds that sharding multiplied
by three. Building once is now the dominant option and the shared stack the
marginal one -- the reverse of how the entry first ordered them.

Everything but the 28 minutes and the 1257s is labelled as an estimate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
Two pages of state that moved with them: the open list is #19 and #21, both
on main and independent now that #17's merge retargeted #19 and Petra rebased
it; the branch-deletion list is eighteen rather than seven, derived rather
than typed; the root AGENTS.md exists, so the thing that was proposed is now
the thing that either works or does not.

#21 does not need rebasing -- it touches only itest.yml and MONOREPO_TODO.md,
neither of which main has touched since 3c3fe76.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Run 35240022926 is green on all four jobs, and #19's unsharded run finished
four minutes later on the same runner pool, so there is a real A/B rather than
a comparison against last night:

    itest workflow      30m43s unsharded   ->  23m07s sharded
    slowest job         29m29s             ->  17m07s
    test binary         1267.853s          ->  628.841 + 462.731 + 204.654

Total test work is unchanged (+2.2%), which is the point: nothing got cheaper,
it got spread. But the saving is ~25%, not the "roughly half" this entry
claimed, and the three reasons are all things the 13-uniform-boots model did
not have.

The shards are unbalanced 629/463/205 because round-robin splits by test name
order and assumes uniform cost. Shard 1 drew TestForgeVersity, whose subtests
include dozens of three-second retention waits, plus two more TTL-bound tests.
Perfect balance would be 432s, so imbalance alone costs 3m17s. On top of that
the shards waited 2m25s-4m47s for runners, and the ~6 minute image build is
per job and did not move.

That also falsifies the boot decomposition twice-quoted here. Shard 3 ran four
tests -- four boots -- in 204.654s, so a boot is at most ~51s, not ~80s, and
the 1040s-booting/217s-working split was wrong. The shared stack is worth less
than the 3-4 minutes last estimated, not more, so it is parked and building the
images once is now unambiguously the largest lever.

Balancing the shards by measured duration is the one remaining cheap win, and
it has to be derived from a previous run's timings rather than hand-grouped --
a hand-written grouping is a list a new test falls out of silently, which is
the shape this repository keeps deleting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
#21's run is green on all four jobs and #19's unsharded run finished four
minutes later on the same pool, so this is an A/B rather than a comparison
against last night: 30m43s -> 23m07s on the workflow, 1267.853s of test binary
-> 628.841 + 462.731 + 204.654. Total test work unchanged at +2.2%, which is
the point -- nothing got cheaper, it got spread -- and runner time went 29m29s
-> 42m04s.

The missing half is three things: shards unbalanced 629/463/205 because
round-robin splits by name order and TestForgeVersity is enormous; 2m25s-4m47s
of queue wait for four concurrent jobs instead of two; and the ~6 minute image
build, which is per job and did not move.

Shard 3 ran four tests, so four boots, in 204.654s. A boot is therefore at most
~51s, not the ~80s this page twice derived from 1257/13, so the
1040s-booting/217s-working split was wrong and the shared stack is worth less
than last estimated rather than more. Parked. Building the images once is the
largest lever by a clear margin now.

Also: the open-PR row for #21 now names 6fb033c, the docs-only commit carrying
this measurement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Both found by reading source rather than by guessing, and neither is in the
entry's original list.

itest.yml:143 and e2e.yml:118 shell out to plain `docker build`, which cannot
use --cache-from type=gha at all; only images.yml uses buildx. The "No caching,
deliberately" comment is in images.yml and gives a reason specific to it --
provoking build failures. That reason does not carry to the two workflows where
the build is a means to running tests, especially since images.yml already
proves the cold build on the same commit on every PR. Keeping images.yml as the
uncached canary and caching the other two preserves the decision and takes ~6
min off four jobs.

Separately, lockWaitTime in tests/integration/utils.go:2654 of our own versitygw
fork is 3 seconds, and cleanupLockedObjects both sets RetainUntilDate to now+3s
and then sleeps 3s waiting for the lock it just created. 38 call sites, so about
114 seconds of sleep in the test that bounds the whole job. 3s -> 1s saves ~76s;
1s is the floor until someone checks sub-second RetainUntilDate round-trips.
That change lands in versitygw and arrives here as a pin bump.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
Petra's idea: since nothing is required yet, say per PR which checks a reviewer
can merge without waiting for, with reasoning a paths: filter could not express.
It is the interim for the path-filtering question and avoids both of that
question's faults -- a list that goes stale silently, and skipped jobs that
never satisfy a required check.

Two conditions make it real rather than comfortable: derive the claim (the
closure is not obvious, which is why CI is unfiltered at all), and state what it
costs when wrong (the checks still run, so an ignored red leaves main red). The
blocks are kept as the worked examples that design the real filtering later.

#22 puts it in AGENTS.md. Also: #19 is green, #21 is at e020534.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
@Peeja

Peeja commented Sep 17, 2026

Copy link
Copy Markdown
Author

Doesn't seem to be the big win I'd hoped. #22 will suffice for now. We may return to this later, or try a different approach.

@Peeja Peeja closed this Sep 17, 2026
Peeja pushed a commit that referenced this pull request Sep 17, 2026
The page said "itest ingot is sharded" as of the last commit, which stopped
being true at 16:21Z. Fixed everywhere: the open-PR tables are #19 and #22, the
second outstanding check-name change is off the table, and the debt entry now
leads with ~30 minutes unsharded as a measured choice rather than an unexamined
one.

The measurement is kept in full, because every remaining option is judged
against it and because reviving the branch is a reopen rather than a rebuild --
claude/shard-itest survives at e020534, and the branch list now says
explicitly not to delete it.

Flagged: the two levers that need no sharding -- the missing buildx layer cache
in itest/e2e, and the self-imposed 3s lockWaitTime in our versitygw fork -- were
born on the closed branch and exist nowhere on main. They are the live work now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
…deadline

#23 re-homes the CI wall-clock measurement that would otherwise live only on
closed #21, plus two findings from reading main: Phase 1 has no release.yml to
use, and tags are gated on the rename because a submodule tag must sit in the
repository its module path names. #24 removes the last two per-service .github
directories. #25 layer-caches the image builds in itest and e2e while images.yml
stays cold as the canary -- the only one of the five open PRs that can fail.

The hazard is on Needs Human Work because it needs a person: forgectl's
metrics-payments and metrics-faults run against environment: mainnet every 30
minutes and 12 hours, pushing to an OTLP endpoint from repository secrets. They
run in the polyrepo today. Archiving fil-forge/forgectl stops them silently, and
reproducing them here needs an environment and secrets configured on this
repository.

Also records what was deliberately not attempted pre-rename and why: the release
workflow (unverifiable until tags can be cut, and an unrunnable workflow is the
silent-green shape this repo keeps deleting), compat.yml (needs published images
to test against), the tag scheme (long-lived, hard to reverse, a recommendation
rather than an agent's decision), and building the Dockerfile.release files
(goreleaser-shaped, they need the release flow first).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
…st clears

Petra approved the lot on 2026-09-17. Rather than let the reasoning go with the
list -- Recently cleared gets pruned -- the durable parts moved to Current State:

Rule 2 gains its worked example. Pinning an image the tree already pins means
reusing that digest rather than resolving a fresh one, because a fresh resolve
puts two builds of one tag in one repository, which is the agreement failure
rather than the reproducibility one.

Known debt gains two entries. Image pins inside subtree prefixes are a standing
cost every future subtree pull carries, accepted because unlike a lint config an
image pin has no root-level alternative -- worth meeting at the next resync
rather than rediscovering as a conflict. And the four guard scripts are recorded
as the agent's own initiative and approved, named rather than assumed welcome
because an earlier squashed-subtree guard was declined.

The measured case against a Go image guard was already on Current State, so it
needed no migration.

Also: no check-name change is outstanding now. #16's is resolved, #21 is closed,
and #27 adds a step to the existing guards job rather than a new check. That
line is worth re-reading before required checks are turned on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
This page said every subtree was resynced to its upstream head. True when
written, false now -- sprue, libforge and ucantone all committed today.
Measured by comparing each git-subtree-split recorded in main's history against
that repository's live origin/main: ingot 25 behind, hilt 6, smelt 4, sprue 4,
delegator 4, piri 2, and swarf, indexing-service, piri-signing-service and
forgectl at the import point.

Two rows matter beyond the count.

smelt 96fc212 is the same bug #27 just fixed, found upstream two days earlier --
its message names pg_isready going green against the temporary initdb server.
#27 derived it independently from the plc-postgres log, so the account holds,
but it was already known in the polyrepo and nobody here had looked. Recorded as
not-novel rather than left reading as a discovery. The fixes are complementary:
upstream makes the consumer wait and also fixes an openbao raft-leader race we
have not hit; #27 fixes the signal. They do not conflict, line 150 versus
163-183.

ingot #166 is CI itest sharding -- the same work as closed #21 -- and arrives
with the final pull regardless.

Petra's policy, recorded: no regular pulls, one final pull at the end, pull
early where upstream fixes something we hit. smelt is the one row meeting that
bar now, flagged as a recommendation rather than started, since an
agent-initiated subtree pull is a rule 7 operation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants