CI: one SWORD module cache that main keeps; fetch gives up on a stall #133
No reviewers
Labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
schaefera/utStudyBIble!133
Loading…
Reference in a new issue
No description provided.
Delete branch "ci/module-cache"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
CI keeps one cache entry with all the SWORD modules (key
sword-modules-all-<hash of tools/golden/fetch.sh>). The rust job saves it and the app and e2e jobs only read it, so CrossWire is asked again only whenfetch.shchanges or the entry expires. A weeklymodules.ymlon main saves the entry again if it's missing.fetch.shskips zips it already has, and a stalled download now gives up and retries instead of taking 15 minutes. The rust job andmodules.ymlinstall zstd, because actions/cache only finds an entry compressed the same way.Blast radius
Tier: trunk
Trunk paths touched:
.forgejo/workflows/ci.yml,.forgejo/workflows/modules.ymlTesting tier: maximum (CI) (also: ship on sight: docs/09; standard:
tools/golden/fetch.sh)Gate / rollback: revert this merge. Each job then fetches its own modules again, as before.
Proof
fetch.sh, locally: a fresh KJV fetch takes 6 s, and a second run reuses the zip.NoSuchModuleXreturns 404 four times (three retries), exits 22 and leaves no.partfile.9b51e63(run 254): rust saved the entry with gzip (cache.tgz), and app and e2e, which have zstd (cache.tzst), missed it in the same run (Cache not found for input keys: sword-modules-all-8d5d…). The zstd commitc87abeafixes this.c87abeais green. In the rust job,Cache restored from key: sword-modules-all-8d5d…, the fetch step only unzips andSave SWORD modulesis skipped. app and e2e also logCache restored from key: sword-modules-all-8d5d…. cargo-registry and sccache missed once because of the zstd change and were saved again.modules.yml, run on the branch: run 257 missed, fetched and saved (Cache saved with key: sword-modules-all-8d5d…). Run 258 loggedLookup only - skipping downloadandCache found and can be restored from key: …, and skipped the save.Review
Reviewer: fresh-context
code-reviewat high (review 352): 3 low findings. 1 agreed and verified after the merge, 2 declined (comment 2609). The targeted check found the zstd mismatch, fixed inc87abea. Delta review at medium (353): 0 findings. 0 left open.Review-Confidence: 85/100. Reviewed at the right depth and CI green on
c87abea; the hand-off from main to later PRs is not shown yet (to be checked after the merge).Confidence by part
fetch.shstall limit and retries: high. Tested locally against a 404. A real stall wasn't reproduced.🤖 Generated with Claude Code
Review level: high. Commit:
9b51e63d25. Range: origin/main...HEAD.Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml, .forgejo/workflows/presets.yml, tools/golden/fetch.sh, tools/ci/main-skip.sh, docs/09-quality-gates.md, CLAUDE.md, Makefile.toml.
No high or medium findings. The restore/save split,
cache-hit != 'true'guards, lookup-only, and theneeds: rust -> app -> e2eordering hold up: the rust job saves in PR scope before app/e2e restore, and each fetch.sh call with a module subset reads from the full zip set.Low findings:
@ -0,0 +9,4 @@on:schedule:- cron: "0 4 * * 1" # Mondays, 04:00 UTCLow: weekly equals a likely 7-day cache retention; if lookup-only doesn't refresh the entry's last-use time, it can expire just before the run and PRs in between re-download. Also no failure notice like presets.yml has for a failed scheduled run.
Answers to review 352's lows:
rustjob should savesword-modules-all) and the next PR'srustjob (it should restore it, with no CrossWire download). I'll post both on this PR.cache:section in its config;~/dev/infrastructure/modules/ci-runner). It evicts stale entries when it starts. The cache server of act/forgejo-runner keeps an unused entry for about 30 days by default, as far as I know; I haven't checked it in the runner's source. Every PR'srustrestore uses the entry too, so a weekly lookup leaves a wide margin. No change.Level: medium (delta). Reviewed commit:
c87abea2ac. Range: 9b51e63d259c2e5e4e6269444eca1f26b11b14a8..HEAD.Files reviewed: .forgejo/workflows/ci.yml, .forgejo/workflows/modules.yml, tools/golden/fetch.sh, CLAUDE.md.
Targeted check (one module cache entry found by the rust, app and e2e jobs and by modules.yml, no download on a hit): no defects found. Both the rust job and modules.yml now install zstd and unzip before any cache step, so both save and read with zstd, as the app and e2e images do. Keys and paths match across all four consumers. On a hit, fetch.sh skips the download when the zip exists and only unzips.
Not verified (no finding): that the ut-app-ci image has zstd. The comment in ci.yml says it does, and docker/ was not in this worktree to confirm. If it lacks zstd, the app and e2e jobs would miss the entry and download again. That would cost time only, not break the build.
Findings outside the diff: none.
WIP: CI: one SWORD module cache that main keeps; fetch gives up on a stallto CI: one SWORD module cache that main keeps; fetch gives up on a stallGate approval on
c87abea. Reviews 352 and 353 have no high or medium findings; the 3 lows were answered in comment 2609. No commits afterc87abea. CI is green on head (success). Proof lists no unrun elevated, maximum or AI-behavior gate: runs 256, 257 and 258 are shown. Open item: the main-to-PR cache hand-off is checked after the merge, as the PR body states.After the merge:
23a11fd) is green. Its rust job loggedCache restored from key: sword-modules-all-8d5d…, so it skipped the download and the save. The entry the branch runs saved was visible to main, so this runner's cache is looser than GitHub's per-branch scopes.13:34:11) and app (13:50:58). Its e2e was cancelled by a new push. The current run (261) covers it.Nothing asked CrossWire again on either run.