Skip to content

Making the uploads non-fatal moved the failure to the downloads (#74) - #77

Merged
sotashimozono merged 1 commit into
mainfrom
fix/74-timings-download-must-degrade
Aug 19, 2026
Merged

sotashimozono merged 1 commit into
mainfrom
fix/74-timings-download-must-degrade

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

Follow-up to #76, which was incomplete. Closes the rest of #74's resilience half.

What #76 got wrong

#76 stopped a full artifact storage quota from failing the label job, and it worked — the shards actually ran for the first time. All eight then failed one step later:

Unable to download artifact(s): Artifact not found for name: testshards-timings

An upload that is allowed to fail has a consumer that is not. Both halves have to degrade together.

…and the obvious fix would be wrong

continue-on-error on the download is not available here, and the reason is already written next to it:

The ONE history labels read, not one this shard fetched for itself. A per-shard fetch let a single transient failure put one shard on a different assignment function from the rest — which double-runs some units and skips others, with every shard still green.

A per-shard tolerance reintroduces exactly that divergence, and it is the silent kind.

The shape that works

The decision is made once, in the job that knows, and shared:

  labels:
    outputs:
      timings: ${{ steps.timings.outcome }}
  test:
    - uses: actions/download-artifact@v8
      if: ${{ needs.labels.outputs.timings == 'success' }}

needs.labels.outputs.timings is one value for the whole matrix, so either every shard has the history or none does — the invariant the comment protects is preserved, not weakened.

"None" is a state the package already defines. load_timings returns an empty table for a missing path (ownership.jl:200), which is what the first run of any suite gets. So the fallback is the ordinary unweighted assignment, not a new code path. A ::warning:: says the balance may be worse and that the result is not.

prebuild gets the same shape

It is an optimisation outright: a shard without the shared compiled cache precompiles for itself and computes the same answer, slower. Its upload becomes non-fatal, its outcome an output, and both the download and the adopt step gate on it — otherwise a full quota still takes the whole matrix down through prebuild, which is what would have happened next.

Unchanged

Still fatal, still ungated: Every unit ran, exactly once, and the refusal inside restore_counters. Coverage and timings are reports; completeness is a claim.

0.3.36 → 0.3.37.

#76 stopped a full artifact storage quota from failing the label job, and the
shards then ran — and all eight failed one step later:

    Unable to download artifact(s): Artifact not found for name: testshards-timings

An upload that is allowed to fail has a consumer that is not. Both halves have to
degrade together, and for `timings` the degradation is NOT free to make per shard:

> The ONE history `labels` read, not one this shard fetched for itself. A per-shard
> fetch let a single transient failure put one shard on a different assignment
> function from the rest — which double-runs some units and skips others, with
> every shard still green.

So `continue-on-error` on the DOWNLOAD would reintroduce exactly that. Instead the
label job publishes its upload's outcome as an output, and every shard gates on
that one value: either the whole matrix has the history or none of it does. "None"
is a state the package already defines — `load_timings` returns an empty table for
a missing path, which is what the first run of any suite gets — so the fallback is
the ordinary unweighted assignment, not a new code path. A `::warning::` says the
balance may be worse and that the result is not.

`prebuild` gets the same shape for the same reason, and it is an optimisation
outright: a shard without the shared compiled cache precompiles for itself and
computes the same answer. Its upload becomes non-fatal, its outcome an output, and
both the download and the adopt step gate on it.

What stays fatal is unchanged: `Every unit ran, exactly once`, and the refusal
inside `restore_counters`.
@github-actions

Copy link
Copy Markdown
Contributor

📚 Docs preview: https://codes.sota-shimozono.com/TestShards.jl/previews/PR77/

(updates on each push to this PR)

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant