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

.github/workflows/itest.yml

@@ -48,18 +48,31 @@ jobs:
       # ones. pkg/stack/cleanup.go says as much ("For single-suite test
       # machines this is safe"). A matrix gives each job its own Docker host;
       # folding these into one job would have them tear each other down.
+      # ingot is split across runners because its cost is stack boots, not
+      # tests: 13 top-level tests, 13 boots, ~1-2 min each (stack_test.go says
+      # so in its own log line), which was 21 of the job's 28 minutes. Splitting
+      # divides that almost linearly. hilt runs whole -- it is ~8 minutes.
+      #
+      # Which tests each shard runs is DERIVED, not listed: see the itest step.
+      # A hand-written -run regex per shard is a list a new test can fall out
+      # of, and it would fall out silently, which is the shape rule 5 exists
+      # to catch.
       matrix:
-        service: [hilt, ingot]
-    name: itest ${{ matrix.service }}
+        include:
+          - {service: hilt,  name: hilt,      shard: 1, shards: 1}
+          - {service: ingot, name: ingot 1/3, shard: 1, shards: 3}
+          - {service: ingot, name: ingot 2/3, shard: 2, shards: 3}
+          - {service: ingot, name: ingot 3/3, shard: 3, shards: 3}
+    name: itest ${{ matrix.name }}
     # Pinned rather than -latest so the Docker version cannot drift underneath
     # the harness, which requires 25+. Same reason as e2e.yml.
     runs-on: ubuntu-24.04
-    # 45, not 30, because this job now builds six images before it tests: the
-    # hilt suite went 1m50s -> 8m12s on that step alone, so ingot's ~21m30s
-    # run lands near 28m and 30 left about two minutes of margin. The Go
-    # timeout below stays 25m and still fires first on a hang, which is the
-    # point of the ordering -- it dumps goroutines, where a runner kill gives
-    # nothing. Upstream runs 60 for a suite that also does the 5 GiB case.
+    # 45 was set when ingot ran whole and landed near 28 minutes, leaving
+    # little margin. Sharding took the longest job to roughly half that, so
+    # this is now generous rather than tight -- left as it is because the Go
+    # timeout below is the one that matters: it fires first on a hang and
+    # dumps every goroutine, where a runner kill at the cap gives nothing.
+    # Lowering the cap is a separate judgement and wants its own measurement.
     timeout-minutes: 45
     defaults:
       run:
@@ -150,7 +163,31 @@ jobs:
       # Never retried, deliberately: a retry here would mask a flaky suite,
       # which is the one thing this job exists to see.
       - name: itest
-        run: go test -count=1 -v -timeout 25m ./...
+        env:
+          SHARD: ${{ matrix.shard }}
+          SHARDS: ${{ matrix.shards }}
+        run: |
+          set -euo pipefail
+          # Derive this shard's tests from the binary rather than listing them
+          # here. Round-robin over the sorted names, so a test added later
+          # lands in a shard by itself; a hand-written -run regex is a list a
+          # new test falls out of, and it falls out silently -- a green job
+          # that ran nothing looks exactly like a green job that ran
+          # everything. `shards: 1` selects all of them, so hilt takes this
+          # same path unsharded (NR % 1 is always 0).
+          #
+          # Cost here is one stack boot per top-level test, so a split by test
+          # count is a split by time; nothing cleverer is needed.
+          all=$(go test -list '^Test' ./... | grep '^Test' | sort)
+          mine=$(printf '%s\n' "$all" | awk -v k="$SHARD" -v s="$SHARDS" 'NR % s == k % s')
+          if [ -z "$mine" ]; then
+            echo "::error::shard $SHARD/$SHARDS matched no tests -- the suite has fewer than $SHARDS, or -list failed"
+            exit 1
+          fi
+          echo "::group::shard $SHARD/$SHARDS runs $(printf '%s\n' "$mine" | wc -l) of $(printf '%s\n' "$all" | wc -l)"
+          printf '%s\n' "$mine"
+          echo "::endgroup::"
+          go test -count=1 -v -timeout 25m -run "^($(printf '%s\n' "$mine" | paste -sd'|' -))$" ./...
 
       - name: dump container state on failure
         if: failure()

MONOREPO_TODO.md

@@ -223,6 +223,130 @@ that was fixed separately and is not what this entry is about.
 having a stale filter list, or find a third thing — perhaps splitting the
 slowest suites onto a different trigger.
 
+## Decide how much more to spend making `itest ingot` fast
+
+Sharding is done and **measured**, and it did less than predicted: ~25% off the
+wall clock, not the "roughly half" this entry first claimed. The measurement
+also moved where the remaining time is, so read the numbers before the options.
+
+### The A/B, run within four minutes of each other
+
+Both on `ubuntu-24.04`, same runner pool, same hour, so this is a real
+comparison rather than a comparison against last night:
+
+| | unsharded ([#19](https://github.com/fil-forge/forge-2/pull/19), run 35239716032) | sharded ([#21](https://github.com/fil-forge/forge-2/pull/21), run 35240022926) |
+|---|---|---|
+| `itest` workflow, start to finish | **30m43s** | **23m07s** |
+| slowest job, start to finish | 29m29s (`itest ingot`) | 17m07s (`itest ingot 1/3`) |
+| test binary | `ok …/ingot/itest` **1267.853s** | **628.841s** + **462.731s** + **204.654s** = 1296.226s |
+| runner-minutes, ingot only | 29m29s | 42m04s |
+
+**The total test work did not change** — 1296.2s against 1267.9s, +2.2%, which
+is noise plus three process startups. Nothing got cheaper; it got spread. That
+was the intent, and it is worth stating plainly because it bounds every option
+below.
+
+### Where the predicted half went
+
+Three costs, none of which the "13 uniform boots" model had:
+
+1. **The shards are not balanced: 629s / 463s / 205s.** The critical path is
+   the *slowest* shard, not the mean. A perfectly balanced 3-way split of
+   1296s would be 432s, so imbalance alone costs **~197s (3m17s)**.
+
+   The round-robin splits by test *name order*, which silently assumes every
+   test costs about the same. It does not. Shard 1 drew `TestForgeVersity` —
+   the Versity S3-compatibility suite, hundreds of subtests, dozens of them
+   3-second object-lock retention waits — plus `TestForgeMultipartExpiryShred`
+   and `TestForgeReadAfterEviction`, which both wait on TTLs. Shard 3 drew four
+   cheap ones and finished in 205s.
+
+2. **Queue wait: 2m25s to 4m47s per shard** (4m47s on the critical one). Four
+   concurrent jobs where there were two, and the runners did not all start at
+   once. This is pure loss, and it grows with shard count.
+
+3. **The image build, ~6 minutes, is per job and did not move.** Sharding
+   triplicated it in runner-minutes.
+
+### So the options have reordered again
+
+- **Turn on a Docker layer cache in `itest` and `e2e`** — the cheapest thing on
+  this list and, as far as the tree shows, never actually decided against.
+  `itest.yml:143` and `e2e.yml:118` both shell out to plain
+  `docker build`, which cannot use `--cache-from type=gha` at all; only
+  `images.yml` uses buildx, via `docker/build-push-action@v6`. The
+  "No caching, deliberately (2026-09-11)" comment lives in `images.yml` and
+  gives a reason specific to *that* workflow — "the point is to provoke build
+  failures, not to avoid them". That reason does not obviously carry: in
+  `itest` and `e2e` the build is a means to running the tests, and `images.yml`
+  is already proving the cold build works, on the same commit, on every pull
+  request, in parallel. **Keeping `images.yml` uncached as the canary and
+  caching the other two would preserve the whole point of the decision** and
+  take up to ~6 min off each of four jobs. Needs
+  `docker/setup-buildx-action` plus `buildx build --cache-from/--cache-to
+  type=gha --load`; the risk is a stale cache masking a Dockerfile fault,
+  which is exactly what the canary is for.
+- **Build the images once and load them** — the same ~6 min, attacked from the
+  other side: `images.yml` already builds all 8 and could hand them over as an
+  artifact. Unmeasured, and the uncertainty is real: 8 images is likely 1–2 GB
+  of round-trip. A registry would be faster but `images.yml` deliberately takes
+  no `packages: write` so fork PRs work. **Try the cache first** — it is a
+  handful of lines, needs no cross-job plumbing, and if it works this option
+  stops mattering.
+- **`lockWaitTime` in our own versitygw fork is 3 seconds, and it is
+  self-imposed.** `tests/integration/utils.go:2654` in
+  `github.com/fil-forge/versitygw` (pinned at
+  `v0.0.0-20260914113944-a628e2cc628c`) sets
+
+      lockWaitTime time.Duration = time.Second * 3
+
+  and `cleanupLockedObjects` uses it twice: it sets
+  `RetainUntilDate: time.Now().Add(lockWaitTime)` and then sleeps the same
+  duration waiting for the lock it just created to lapse. Nothing external
+  requires 3 seconds — the teardown picks the retention date itself.
+
+  **38 call sites** across `Access_Control.go`, `CopyObject.go`,
+  `CreateMultipartUpload.go`, `GetObject*.go`, `PutObject*.go`,
+  `WORM_protection.go` and `versioning.go`, so at one firing each that is
+  **~114s of pure `time.Sleep`** — roughly a sixth of shard 1's 629s, and
+  `TestForgeVersity` is the test that bounds the whole job.
+
+  3s → 1s would save ~76s. **1s is the safe floor until someone checks**
+  whether sub-second `RetainUntilDate` round-trips through ingot's object-lock
+  path; the S3 header is a timestamp, and second granularity is the
+  conservative assumption rather than a verified one. versitygw is a fork we
+  control, so this is a one-line change — but it is **not** in this
+  repository, so it lands upstream first and arrives here as a pin bump.
+- **Balance the shards by measured duration**, worth ~3m17s. The obvious
+  implementation is a hand-written grouping, which is the silent-green shape
+  this repository keeps deleting — a new test falls out of the list unnoticed.
+  A derived version works: carry the previous green run's per-test durations,
+  bin-pack against them, and give an unknown test the mean so it still runs and
+  merely unbalances slightly. That degrades safely, which the list does not.
+- **One test bounds everything.** `TestForgeVersity` is most of shard 1's 629s.
+  Until it is split or made internally parallel, no shard count gets the job
+  below roughly its own duration plus the ~6 min build plus queue. More shards
+  now buy very little.
+- **Share one stack across tests** — the estimate this entry carried twice, now
+  known to have been built on a wrong decomposition. The "~80s per boot,
+  1040s booting / 217s working" split assumed 13 uniform boots. It cannot be
+  right: shard 3 ran **four** tests, and so four boots, in 204.654s total, so a
+  boot is at most ~51s and the real split is far more evenly divided between
+  booting and working than assumed. **The shared stack is therefore worth less
+  than the 3–4 minutes previously estimated, not more** — and it still changes
+  test isolation in the one job that exists to catch flakiness. Parking it.
+
+**The honest next step is to measure the artifact round-trip**, which is the
+one number nobody has and the one lever that is clearly largest. Everything
+else here is now measured.
+
+**Also worth keeping in view:** the trade sharding actually made was **~43% more
+runner-minutes for ~25% less wall clock**. That was the right trade while PRs
+are the bottleneck; it is the wrong one if runner-minutes ever become the
+constraint, and every service brought in-repo adds another image build to the
+fixed half.
+
+
 ---
 
 # Findings in the imported code