Add configurable cache size and per-derivation cache stats - #23
Add configurable cache size and per-derivation cache stats#23mdarocha wants to merge 19 commits into
Conversation
Expose the cache-nix-action garbage-collection threshold as a new `max-cached-store-size` input (default 8G, up from a hardcoded 2G). The old 2G ceiling triggered a store GC before every cache save that evicted freshly-built outputs - including unrooted `nix flake check` results - so they were rebuilt on every run. A generous default keeps them warm; the value maps to the uncompressed store size (what the GC measures), and can be set to an empty string to disable collection entirely. Also report a per-derivation cache breakdown to the job summary (pondinfra#23): how many store paths were restored from the GitHub Actions cache, substituted from upstream binary caches, or built locally on the runner, with locally-built paths listed individually when there are fewer than 100. Built-vs-substituted is determined exactly from Nix's own `ultimate` flag, comparing store snapshots taken before/after the cache restore and after the build. The post-build report runs in the job's post phase via pyTooling/Actions/with-post-step, since composite actions can't declare a post step of their own. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
Add a `change-sentinel` input that is passed through to comment-flake-lock-changelog's `build-filter`, so flake.lock changelog commits that don't affect the build output can be filtered out of the PR comment. Temporarily pins comment-flake-lock-changelog to its main branch (its `build-filter` feature is unreleased) so the option can be exercised end-to-end. Revert to a tagged release once it ships. See mdarocha/comment-flake-lock-changelog#301. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
mdarocha
left a comment
There was a problem hiding this comment.
the stats should also list: what cache was restored (whether it was primary cache or a different cache), its size, and what happened with the cache at the finish - if it uploaded a new cache item, its size (with delta compared to original restore cache) or if it was skipped (for what reason - failure/primary cache hit)
Address review feedback: keep action.yml input descriptions to a single line and leave the longer explanation to the README, sync the config table rows to the same short wording, and restore the unchanged cache-nix-action feature bullet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
Extend the job-summary stats (per review feedback) to cover the whole cache lifecycle: - which cache was restored (exact primary vs a prefix match) and its size; - the save outcome: a new cache uploaded (size + delta vs the restored cache), skipped due to an exact primary-key hit, or a warning when no new cache is found afterwards. Cache sizes come from the GitHub Actions Cache API (needs actions: read, already required). Because cache-nix-action saves in its own post step and exposes no size outputs, the reporter is split into two post steps ordered around it: a capture step (registered after the cache step, so its post runs before the GC/save) snapshots the built store for the per-derivation breakdown, and the report step (registered before the cache step, so its post runs after the save) renders the summary and queries the API. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
|
Pushed Style comments (descriptions, README line 14): done in Expanded stats — the summary now covers the whole lifecycle:
Cache sizes come from the GitHub Actions Cache API — cache-nix-action exposes no size outputs, and it saves in its own post step. To observe the save, the reporter is split into two post steps ordered around it: a capture step (registered after the cache step → its post runs before the GC/save) snapshots the built store for the breakdown, and the report step (registered before the cache step → its post runs after the save) renders and queries the API. Caveat: the runner-dependent parts — the post-step ordering, step-env reaching the post phase for the token, and the live Cache API calls — can't be exercised in my sandbox. I validated the classification and rendering logic against a mocked On optimal knobs (from reading cache-nix-action): the current config is already the right generic setup — the Generated by Claude Code |
| | `token` | Github authentication token to use | `${{ github.token }}` | | ||
| | `free-up-all-storage` | Aggressively free up all possible disk space on the runner before installing Nix, using [wimpysworld/nothing-but-nix](https://github.com/wimpysworld/nothing-but-nix) | `false` | | ||
| | `max-cached-store-size` | Maximum uncompressed Nix store size to keep in the cache (e.g. `8G`, `512M`); an empty string disables garbage collection. See [Cache size](#cache-size). | `8G` | | ||
| | `change-sentinel` | Shell command forwarded to [comment-flake-lock-changelog](https://github.com/mdarocha/comment-flake-lock-changelog)'s `build-filter`, to hide `flake.lock` changelog commits that don't affect your build output. See its README. | `""` | |
mdarocha
left a comment
There was a problem hiding this comment.
https://github.com/mdarocha/pondinfra/pull/141
https://github.com/mdarocha/pondinfra/actions/runs/30020016538

- node v20 deprecation warning
- incorrect post-run cache detection - it should report that no cache was uploaded to to matching primary key
…ction
Real-world smoke testing on pondinfra#141 (nix-magic-setup#23 review)
surfaced two bugs:
1. The save-outcome line always rendered an empty key ("No new cache
found for `` after save"). Root cause, confirmed in cache-nix-action's
dist source: in this combined restore+save usage, its `primary-key`
output is never actually set - `setState` only persists it as internal
cross-phase state (core.saveState), not a step output, unlike
hit-primary-key/hit-first-match/restored-key which are set directly.
Fixed by computing the key ourselves from the same expression already
given to the `primary-key` input, shared via a YAML anchor so it can't
drift.
2. A real run showed "restored a different cache via prefix match" (a
primary-key miss at restore) immediately followed by a "may have
failed or been skipped" warning - but the job log showed cache-nix-action
legitimately skipping the save because a cache for that exact key
already existed by save time (saved by a concurrent workflow run
evaluating the same flake.lock/*.nix state). cache-nix-action decides
whether to save via its own check at save time, independent of the
restore-time hit, so a restore miss doesn't imply this run's save
succeeded or even ran. Fixed by having cache-stats-capture.sh (which
already runs immediately before cache-nix-action's own save step)
also check whether a cache for the primary key exists at that point;
the report now distinguishes a genuine new upload from a save skipped
because the key was already taken, and only warns on a real failure
(neither before nor after).
Also fixes a review nit: the change-sentinel row's "See its README" is
now an actual link.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
|
Thanks for smoke-testing this on a real deploy — both findings were real bugs, now fixed in Root cause of the incorrect cache detection (the Why it reported "no cache … may have failed": your run's log showed the real story — restore missed the primary key (hence "restored a different cache via prefix match"), but by save time a cache for that exact key already existed, and That's I verified both the original bug and the fix against the exact scenario from your run (mocked, since I can't run a real workflow from here) — confirmed the old code reproduces the empty-key/false-failure output, and the new code produces the correct message. The Node.js 20 warning is in Generated by Claude Code |
Picks up the node24 runtime fix (mdarocha/comment-flake-lock-changelog#315, merged) so the temporary main pin used for testing change-sentinel no longer triggers the Node.js 20 deprecation warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
The previous commit's YAML anchor/alias (to share the cache primary-key expression across three steps without duplicating it) broke the action entirely: ##[error]mdarocha/nix-magic-setup/.../action.yml: Anchors are not currently supported. Remove the anchor 'primary-key' Confirmed via a real smoke-test run on pondinfra#141 - the job failed at "Set up job", before any step ran. Local YAML validation (PyYAML) didn't catch this because it resolves anchors just fine; GitHub's own action manifest parser is stricter than generic YAML. Reverted to spelling out the literal key expression in all three places, with a comment explaining why and where to keep them in sync. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
| description: "Maximum uncompressed Nix store size to keep in the cache (e.g. 8G, 512M); an empty string disables garbage collection. See the Cache size section in the README." | ||
| required: false | ||
| default: "8G" | ||
| change-sentinel: |
There was a problem hiding this comment.
better rename to changelog-filter
Per review feedback - it forwards to comment-flake-lock-changelog's build-filter, and "changelog-filter" names what it does more directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MYz49D9GAahQN6haYW3Nb
Picks up #317 (changelog comment formatting) and #318 (build-filter now bisects against the actual head commit and paginates past the compare API's 250-commit cap, instead of silently bisecting only the first 250 commits of a range).
Picks up #321: build-filter now resolves CFLC_INPUT_NAME to the flake's real input path (instead of the flake.lock node key, which silently no-ops --override-input for deduplicated inputs like nixpkgs), and no longer crashes with EPIPE on large commit ranges.
Smoke-test reportTested both features live on mdarocha/pondinfra#141, pinned to this PR's branch (
|
…nabled The nix-magic-setup smoke test on a real nixpkgs bump (2500+ commits) hit "no space left on device" in a later, unrelated step once build-filter's classification bug was fixed and it started doing real work: bisecting a large range with comment-flake-lock-changelog's recommended path: fetcher usage copies the entire checked-out input into the Nix store on every build, and nothing reclaimed those copies before the rest of the job needed the disk. comment-flake-lock-changelog#322 adds an opt-in build-filter-gc input that runs `nix store gc` between builds to bound that growth, but it's only safe to use if nothing else in the job depends on Nix store paths that aren't rooted yet - a cache that was merely *restored* isn't necessarily a GC root, so gc'ing after that restore risks deleting the cache that was just pulled in. Moving this step to run before the cache is restored (instead of near the end, after the cache/build steps) makes build-filter-gc: true safe: the store is still essentially empty at this point, so there's nothing valuable for the between-build gc to collect away.
Points at #322's branch (build-filter-gc: true isn't in a merged commit yet) so this PR's smoke test actually exercises it instead of silently no-op'ing against a pin that predates the feature.
Pairs with comment-flake-lock-changelog#322: build-filter-gc alone still reclaims one full "checkout + store copy" per commit before moving to the next; build-filter-skip-checkout avoids the checkout copy entirely by having Nix read each commit straight out of the local clone's git object database via git+file://?rev= instead of path:, which needs this action to check the commit out to disk first. Updated here since deploy.yml's changelog-filter command needs to switch to that pattern for skip-checkout to be safe to enable.
comment-flake-lock-changelog reverted it (comment-flake-lock-changelog#322) after a real run on pondinfra#141 failed with a libgit2 "object not found" error: Nix's git fetcher can't lazily fetch missing blobs from the blobless clone's promisor remote the way `git checkout` (the real git CLI) can, so skipping that checkout leaves libgit2 unable to find what it needs. Keeps build-filter-gc and the git+file://?rev= pattern in changelog-filter, both of which are unaffected - the checkout still runs, it just no longer gets skipped.
Picks up #322 (build-filter-gc) and #323 (fixes the GitHub API result cache, which never actually persisted across runs due to an immutable, unversioned cache key), both merged to main.
Raises the per-derivation summary's "list individually" threshold for locally-built paths from 100 to 200, matching what the summary reports above it. Also folds max-cached-store-size into the Nix store cache's primary key. Previously, changing that config value alone (nix/flake.lock unchanged) still hit the same primary key, so cache-nix-action would report an exact-match "nothing to do" even though the target gc size had changed. Keying on it too means a config change always saves a fresh cache under the new target.
Picks up the EPIPE fix for the per-commit PR-lookup logging loop, found live on pondinfra#141 with a 1847-commit range.
Picks up the fix for eager bulk-fetching the whole commit range instead of each commit lazily per build, found live on pondinfra#141 with a 1847-commit nixpkgs bump.
Picks up the revert of the shallow-fetch experiment back to a full blobless clone, after two live failures against real nixpkgs.
Summary
Two related caching improvements:
1. Configurable cache size (
max-cached-store-size, default8G)The cache-nix-action garbage-collection threshold was hardcoded at
2G. That ceiling triggers a store GC before every cache save, and it was evicting freshly-built outputs — includingnix flake checkresults, which aren't kept as GC roots and so count as garbage — meaning they were rebuilt from scratch on every run.8Gso a full build's outputs survive collection and get saved (then restored fully warm next run).2. Per-derivation cache stats in the job summary (closes mdarocha/pondinfra#23)
After the build, the action writes a breakdown to the job summary showing where each store path came from:
Locally-built paths are listed individually when there are fewer than 100, so you can see exactly what wasn't cached.
How the stats work
(after − before)is exactly what the GitHub cache provided (the baseline toolchain from Nix install is excluded as noise).ultimateflag frompath-info --json(true = built locally, false = pulled from a binary cache) — exact, with no need to reconfigure or restart the Nix daemon..drvfiles are excluded so the counts reflect realised outputs.pyTooling/Actions/with-post-step(pinned by SHA). No configuration required — it reports automatically.Changes
action.yml: newmax-cached-store-sizeinput wired togc-max-store-size; before/after store snapshot steps; post-phase reporter viawith-post-step.scripts/cache-stats-snapshot.sh,scripts/cache-stats-report.sh: new (bash, matching the existingscripts/style).README.md: documents both features (Cache size, Cache stats), updates Features and Roadmap.Testing
The report/classification logic was unit-tested locally against a mocked
nix(both the object- and array-shapedpath-info --json), covering exact hit, prefix hit, cold miss, the ≥100-built path, the nothing-built path, and.drvexclusion. The runner-dependent wiring (snapshots on a real store, thewith-post-steppost phase) hasn't been exercised on an actual runner yet — worth a smoke-test run before tagging a release.🤖 Generated with Claude Code
Generated by Claude Code