Skip to content

[design] Artifact-free sharding on a free plan: move the completeness check to the shard instead of moving its data #80

Description

@sotashimozono

REJECTED. Two independent reviews with no shared context, plus my own re-measurement, agree: the premise is wrong and the mechanism is broken. Recording both so the next attempt does not repeat either.

The premise was a misread field

I justified this design on "artifact storage is structurally unusable on a free plan, and deleting does not help because expired artifacts keep counting." Measured directly:

expired records: 4894    with size_in_bytes > 0: 4894    total 21.3 MB

An expired artifact keeps its metadata row, including size_in_bytes, after GitHub deletes the bytes. Every sum I reported — including the 2.72 GB figure and #78's "a one-day artifact from 2026-07-14 was still present five weeks later, still counting" — was summing deleted bytes alongside live ones. Retention is working.

Filtering on expired == false across all 25 private repos of the affected account:

live artifacts 11,411
live bytes 90.7 MB
documented free-plan pool 500 MB (shared with Packages, which I cannot read: 403 needs read:packages)

So the account is at roughly a fifth of its pool, and the CreateArtifact quota error is not explained by live bytes. The most likely reconstruction: storage did exceed the limit, the manual prune fixed the underlying number, and the 6–12 hour recalculation lag kept the flag set — which is exactly why deletion looked like the wrong lever. It was the right lever with a slow feedback loop. Uploads work again as of run 32232523231 (2026-08-19T08:25Z), where Upload this shard's records and timings succeeded with no change to the workflow.

And it targets the one family with no steady-state cost

Live bytes by family, across the four largest repos (86 of the 90.7 MB):

family live files live bytes retention
-out-* 7457 58.73 MB 7 days
-records 750 16.93 MB 30 days
-lcov 908 10.33 MB 7 days
-timings 98 0.11 MB 1 day
-coverage-* 0 0 MB 1 day

This issue's headline number was "104 KB × 8 per run, 97 % of this fleet's artifact bytes." That is a share of cumulative created bytes. By live bytes the coverage family is zero — it expires in a day. The design spends its whole budget on the only family that costs nothing to keep.

The mechanism does not work either

The core claim was that "every unit ran exactly once" needs no transport, because it decomposes into "assign is a partition" (a unit test) plus "each shard ran its assignment" (an in-shard check). Both halves fail, and src/ownership.jl says so in its own docstring:

Units absent from the history are not here; they are assigned on sight, round-robin over the order they are observed in (see _owns)

  • assign is not total over the units. It is total over keys(timings). Everything the history has not seen goes through _owns(::Assigned, …), whose ctx.unknown += 1 is a mutable per-process counter in observation order — not a pure function of (timings, n). On a fresh consumer with no ci-timings branch, assign returns Dict() and 100 % of ownership comes from the path the claim does not describe.
  • The in-shard check is a tautology. ctx.ran is appended to iff _owns returned true, and _owns consults the same ctx.assignment computed once in the same process. Comparing them compares a value against its own definition.
  • Under steal there is no assignment at all (_owns(::Claimed, …)_claim), so the local quantity to compare against does not exist. sharded-tests.yml:797 already states the check is "NOT best-effort, and not conditional on steal".
  • The real cross-shard payload is the history itself, and its divergence is the failure this repo has already measured and written into the file at :290-293: seven shards loaded 23 timing rows, the eighth loaded none after a transient git ls-remote failure, "two units ran twice, two ran nowhere, and only the completeness gate noticed." A per-shard self-check is precisely the check that would not have noticed. Each shard is internally consistent with its history.

Two of the three reasons I gave for rejecting the git-ref transport were also wrong

  • "It would require contents: write on eight jobs running arbitrary test code; today only timings has write." Wrong. timings is the only job that declares permissions:; every other job inherits the caller's grant, and :249-252 requires callers to grant contents: write. Every consumer does. Worse, :595 injects TESTSHARDS_CLAIM_TOKEN: ${{ secrets.GITHUB_TOKEN }} unconditionally:594 gates TESTSHARDS_CLAIM on inputs.steal, :595 gates nothing — so that write-capable token is already in the environment of Pkg.test() on every run of every consumer. The exposure I described as a regression is the status quo.
  • "Deleted refs leave unreachable objects." Correct, but ci-timings is force-pushed every run by the same argument. It is a size argument, not a principled one.
  • "A fork pull_request gets a read-only GITHUB_TOKEN, so it cannot push, while artifacts work there." This one stands, and on its own it is sufficient. QAtlasHub/TestShards.jl is a public repository and General-registered; breaking external contribution is not a trade to make.

What the reviews found that this issue missed

  • Job outputs are an artifact-free channel already used here. needs.labels.outputs.timings exists (:267, :482, added by Making the uploads non-fatal moved the failure to the downloads (#74) #77 for this very invariant), and timings.tsv is under 1 KB. Base64 through a job output removes the labels → shards artifact with no new mechanism, no permission change, full fork-PR support — and it preserves the single-history invariant by construction instead of hoping for it. The honest reason the shards → collect direction is harder: GitHub does not merge outputs across matrix legs; the last leg to finish overwrites, order unspecified.
  • Coverage counters can travel without the source they are printed against (#74) #75 is merged, verified, and unwired. counter_index/restore_counters in src/coverage.jl take a 48-file shard payload from 965,565 → ~15 KB (~62×), round-trip byte-exact. The workflow still plain-cps the raw .cov at :611-624. Five lines, zero interface change.
  • A per-shard coverage percentage is not a degraded number, it is a wrong one. Percentages do not average; only line sets union. This repo already records the symptom — "reported 54.5 % for a suite that covered 94.8 %". The fallback I proposed is that bug.
  • diagnose (default true) is definitionally cross-shard — peak concurrency and arrival spread cannot be computed inside one shard. It is a fourth payload this issue's table omitted.
  • The single genuinely failing step on current main under the quota event was Every unit ran, exactly once — and failing closed is correct for a gate. What is needed is an artifact-independent evidence path for it, not a weaker check.

Superseded by

The small, measured fix, in priority order — filed as #81:

  1. cut -out-* (7 d) and -records (30 d) retention — 84 % of live bytes, two edits
  2. ship the workflow side of Coverage counters can travel without the source they are printed against (#74) #75 — ~62× off the coverage payload, already tested
  3. automate the prune (lab-sotashimozono/.github#32)
  4. move the labels → shards history onto a job output — removes the artifact that gates the matrix and hardens the invariant
  5. fix :595 to ${{ inputs.steal && secrets.GITHUB_TOKEN || '' }} (independent of all the above)

Closing rather than editing: the design was wrong at the premise, not in the details, and an issue that keeps its own refutation in the body is easier to learn from than one rewritten into agreement.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions