Skip to content

ci: delete a PR's caches when it closes - #206

Merged
On1x merged 2 commits into
masterfrom
ci/cleanup-closed-pr-caches
Oct 30, 2026
Merged

On1x merged 2 commits into
masterfrom
ci/cleanup-closed-pr-caches

Conversation

@chiliec

@chiliec chiliec commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Caches saved by a PR run (ccache, BuildKit) are scoped to refs/pull/<N>/merge: no other branch or PR can restore them, so once the PR closes they only take quota.

Why now: the repo sits at the 10 GB cache limit (10.38 GiB on Oct 30 06:24 UTC, per On1x's measurement below). 2.01 GiB in 122 entries belongs to seven already-closed PRs (#196–#202); when I opened this it was ~4.3 GB in twelve, and LRU has evicted the rest since. Over the limit GitHub evicts least-recently-used entries, and that can include master's ccache, which every PR now falls back to after #202. Losing it would put PR builds back to cold.

What it does (.github/workflows/cache-cleanup-closed-pr.yml):

  • pull_request_target: closed: deletes every cache for that PR's merge ref (merged or just closed).
  • workflow_dispatch: a manual sweep of every PR ref that still owns caches; only PRs whose state is closed are deleted, open ones are kept. Run it once after merge to clear the current backlog.
  • Best-effort deletes: an entry evicted between list and delete is logged as a warning, not a failure.

pull_request_target is used so the token can delete caches for PRs from forks too. It is safe here: the job never checks out or runs PR code, it only calls the cache API with the PR number from the event payload. Permissions are actions: write and pull-requests: read.

Checked: I ran the step's script against this repo through the real API with the DELETE call stubbed out:

Failure handling (second commit, after review): any failed list call fails the job; a delete that returns 404 counts as already evicted; any other delete error fails the job at the end.

Nothing was deleted. bash -n and the YAML parse are clean.

Doesn't touch master caches or cache-reset.yml.

Caches saved by a PR run are scoped to refs/pull/<N>/merge and can't be
read by any other ref, so after the PR closes they only take quota. The
repo sits at ~10.1 GB of the 10 GB limit; ~4.3 GB of it belongs to
twelve closed PRs. Over the limit GitHub evicts least-recently-used
entries, which can include master's ccache that PRs fall back to (#202).

On pull_request_target closed, delete every cache for that PR's merge
ref. workflow_dispatch sweeps all closed PRs that still own caches (the
current backlog); open PRs are kept. The job never checks out PR code.
@On1x

On1x commented Oct 30, 2026

Copy link
Copy Markdown
Member

Measured the current state of the quota this workflow is about, so the effect can be checked after merge (2026-10-30 06:2x UTC, repos/…/actions/caches paginated):

scope size entries
total 10.38 GiB 539
refs/heads/master 3.94 GiB 264
open PRs (#205, #203, #206, #207, #208) 4.23 GiB 135
closed PRs #195–#202 2.31 GiB 142

Two notes on the numbers in the commit message: the "~4.3 GB in twelve closed PRs" figure has since shrunk to 2.31 GiB in eight — LRU already evicted some of them, which is exactly the churn this workflow stops. And the single largest holder is not a closed PR at all: #205 alone owns 2.84 GiB in 77 entries (per-sha keys, one set per push/rerun).

I also verified the exact call the workflow makes, unencoded slashes included: gh api "repos/$repo/actions/caches?ref=refs/pull/195/merge&per_page=100" returns 17 entries / 253 MiB, so the filter behaves as assumed.

One correctness gap worth closing before merge: an API failure currently reads as "nothing to delete". Both branches swallow it:

# manual sweep
for n in $(gh api --paginate "repos/$repo/actions/caches?per_page=100" --jq …); do

— if that call fails (rate limit, token scope), the list is empty, the loop body never runs, and the job prints Done, 0 failure(s). and exits green. Same in the per-PR branch: rows=$(gh api …) fails → [ -z "$rows" ] → #N: no caches. A cleanup job that cannot distinguish "no caches" from "could not list caches" will report success for the whole life of a revoked token. Suggested: capture the exit status (if ! out=$(gh api …); then echo "::error::cache list failed"; exit 1; fi) in both places, and consider failing when every delete returned non-zero rather than only counting.

Nothing else looked off: pull_request_target: closed with actions: write and no checkout is the right shape, the fork case is covered by the base-repo token, and the delete loop is best-effort by design (an entry evicted between list and delete is not an error).

Merge order matters for the payoff: landing this before #205/#203 closes means their 3.5 GiB is reclaimed automatically at merge time instead of waiting for LRU.

@On1x

On1x commented Oct 30, 2026

Copy link
Copy Markdown
Member

Measured the current state before this merges (2026-10-30, 06:24Z; 514 cache entries, 10.38 GiB total — still pinned at the cap):

Reclaimable backlog is exactly PRs #196–#202 — all seven are already merged/closed: 2.01 GiB in 122 entries (#196–#201 ~253 MiB each, #202 535 MiB). That is what a workflow_dispatch run of this workflow should free today. Open PRs hold 4.53 GiB (#205 2.84 GiB in 77 entries, #203 0.70, #206 0.54, #208 0.47, #207 0.07); master 3.69 GiB; windows_build 0.15 GiB.

Eviction is already active, not theoretical: between two measurements 4 minutes apart master lost 26 entries (3.94 -> 3.69 GiB) while 17 new entries appeared from in-flight PR builds. Older entries survive only while they keep being restored (oldest master entry dates from 15.10).

It has not hurt builds yet — after #202, ccache hit rates from ccache -s: master 2091/2416 = 86.6%, returning PR 85.5%, and a brand-new PR branch 2195/2589 = 84.8% via the main- restore-key fallback (previously 0/173). So this is budget pressure, not damage — but the headroom is gone, and the dead weight is what pays for it.

Review on #206: a failed "gh api" call read as "nothing to delete". With
a rate limit or a token without actions:write the cache list came back
empty, the loop never ran, and the job printed "Done, 0 failure(s)" in
green, which would keep happening for as long as the token stayed broken.

- listing caches, a PR's caches, or a PR's state now fails the job
  (::error:: + exit 1) instead of being taken as an empty result;
- a delete that returns HTTP 404 is "already evicted" and still fine;
  any other delete error is counted and fails the job at the end;
- the summary line reports deleted vs failed.
@chiliec

chiliec commented Oct 30, 2026

Copy link
Copy Markdown
Member Author

Thanks, both points taken. Fixed in 50a02024:

  • API failures now fail the job. Listing all caches, listing one PR's caches, and reading a PR's state each fail with ::error:: and exit 1, instead of being treated as an empty result.
  • Deletes: HTTP 404 still counts as "already evicted" and is fine. Any other error is counted, and the job fails at the end if there was at least one. The summary line now reports N deleted, M failure(s).

I checked it against the real API with DELETE stubbed out (nothing was deleted; #199 still has its 17 entries):

case result
single PR #199 17 deleted, exit 0
unknown PR steemit#999 "no caches", exit 0
cache list returns 403 (rate limit) ::error::could not list caches for refs/pull/199/merge, exit 1
sweep, list fails ::error::could not list caches, exit 1
every delete → 404 "gone already", exit 0
every delete → 403 17 failures, ::error::17 cache(s) could not be deleted, exit 1
sweep keeps the 6 open PRs, would delete #199–#202 (71 entries)

Agreed on merge order: landing this before #203/#205 lets their caches go at close time. I've updated the numbers in the description to your measurement.

One correction on the #202 figure, since it matters for #208: the "brand-new PR 2195/2589 = 84.8%" is almost exactly master's production build (2196/2589). ccache keeps its counters inside the cache directory, so ccache -s after a restore reports the restored history plus this run. It isn't this build's hit rate. #208 adds ccache -z before the build; I'll explain there.

@On1x On1x left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the final head 50a02024. The failure handling now matches what I asked: a failed cache list or PR-state read fails the job, a 404 on delete counts as already evicted, any other delete error fails at the end. The workflow never checks out or runs PR code (pull_request_target is used only for the token), so the write permission is limited to the cache API. The 2.01 GiB backlog measurement in the thread gives us a before/after check after merge.

@On1x
On1x merged commit 81d25ab into master Oct 30, 2026
6 checks passed
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.

2 participants