CI maintenance: count-cap + low-water eviction for lake-packages cache#2191
Conversation
Age-based pruning alone cannot bound the lake-packages cache: one ~6 GB entry exists per PR/branch, so two weeks of PR churn accumulated 664 GB and filled the runner host disk to 100% on 2026-07-12 — the runner listener crash-looped on log writes, GitHub showed the runner offline, and build-compiler-binaries jobs sat queued indefinitely with no logs. - cap_tree_entries: keep only the newest LAKE_PACKAGES_MAX_ENTRIES (default 20) regardless of age - evict_until_free: LRU-evict below MIN_FREE_GB (default 100) so a churn spike between runs cannot take the filesystem to zero - maintenance timer moves from weekly to daily (the weekly run had executed hours before the incident and pruned nothing)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_96a1af50-7208-467e-99e7-981f3f47bed6) |
| \n### CI Failure Hints\n\nFailed jobs: `checks`\n\nCopy-paste local triage:\n```bash\nmake check\nlake build\nFOUNDRY_PROFILE=difftest forge test -vv\n``` |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df0086c53b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| local removed=0 | ||
| while IFS= read -r entry; do | ||
| rm -rf "$dir/$entry" |
There was a problem hiding this comment.
When the daily timer overlaps a running self-hosted job, this can delete the directory that job has mounted: .github/actions/setup-lean/action.yml attaches .lake/packages from ${local_cache_root}/lake-packages, and scripts/ci_local_persistence.sh does not lock or refresh active cache entries before symlinking them. A long-lived PR cache with an old directory mtime can fall past the count cap here, or be selected by the free-space eviction below, and rm -rfed while Lake is reading it, breaking in-flight CI. Please skip/lock active entries or mark them live before removing them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in decbb39: touch-on-attach in ci_local_persistence.sh mount + MIN_ENTRY_AGE_HOURS (default 6h) floor in both cap and eviction — a mounted entry is refreshed at attach time and never deleted while recent.
|
|
||
| [Timer] | ||
| OnCalendar=Sun *-*-* 04:30:00 | ||
| OnCalendar=*-*-* 04:30:00 |
There was a problem hiding this comment.
Changing the installed timer to daily leaves scripts/test_ci_infra_maintenance.py::test_weekly_host_maintenance_prunes_ci_disk_journald_and_docker still asserting the old weekly OnCalendar=Sun *-*-* 04:30:00 value. I ran python3 scripts/test_ci_infra_maintenance.py -k weekly_host_maintenance and it fails on that assertion, so this commit will break the Python test suite until the expectation/name is updated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in decbb39: updated (weekly→daily expectation) and extended with coverage for the cap/eviction/in-use invariants; python3 scripts/test_ci_infra_maintenance.py passes (8 tests).
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
OpenCodeReview first-pass review
Status: success · Mode: config-docs · 4 finding(s) · 1 files · 94000 tokens · 12 tool calls
✅ Posted 4 inline comment(s).
OCR pilot metrics
- Routing: config-docs (router-v8)
- Changed files: 1 supported / 1 total; Lean 0, trust docs 0, workflow/scripts 1, contracts 0, docs 0
- Changed lines: 58 supported; thresholds large Lean >=3 files or >800 lines
- OCR: status success; comments 4; files 1; tokens 94000; tool calls 12; warnings 0; duration 220s
- Largest changed files: scripts/ci_host_maintenance.sh (+56/-2)
Pilot mode: advisory only. Codex Review remains the merge gate.
| fi | ||
| rm -rf "$dir/$oldest" | ||
| removed=$((removed + 1)) | ||
| done |
There was a problem hiding this comment.
OpenCodeReview [bug · high]
Infinite loop risk: if rm -rf "$dir/$oldest" fails to remove the entry (e.g., filename contains newlines so ls -t | tail -1 returns a truncated name, a filesystem error occurs, or the directory is busy), ls -t "$dir" | tail -1 returns the same entry on the next iteration. Since there is no check on the return code of rm and no iteration guard, the loop spins forever as root inside a systemd oneshot service that has no RuntimeMaxSec/TimeoutStartSec limit. This is ironic since the function exists to prevent CI disruption. Add a guard: track the name from the previous iteration and break if it hasn't changed, or check rm's exit code and break on failure.
Suggested change:
if ! rm -rf "$dir/$oldest"; then
break
fi
removed=$((removed + 1))
done
There was a problem hiding this comment.
Fixed in decbb39: evict_until_free now walks a single oldest-first snapshot and never retries a failed rm — a stuck entry is skipped and reported, the loop cannot spin.
| while IFS= read -r entry; do | ||
| rm -rf "$dir/$entry" | ||
| removed=$((removed + 1)) | ||
| done < <(ls -t "$dir" | tail -n +"$((max_entries + 1))") |
There was a problem hiding this comment.
OpenCodeReview [bug · medium]
Both cap_tree_entries and evict_until_free parse ls output line-by-line. Filenames containing newlines (valid on Linux filesystems) would cause read -r entry or tail -1 to capture only a fragment of the name, producing incorrect rm -rf targets. In evict_until_free this causes the infinite-loop issue described above; in cap_tree_entries it causes silent miscounts and failed deletions. The cache entries are named after PR/branch identifiers (e.g. lake-packages-verify-<bucket>-<hash>…), but the codebase doesn't guarantee the absence of special characters. Consider using null-delimited output with find ... -printf '%T@ %p\0' and sort -zn, consistent with the existing prune_tree function which already uses find -print0.
Suggested change:
while IFS= read -r -d '' entry; do
rm -rf "$entry"
removed=$((removed + 1))
done < <(find "$dir" -mindepth 1 -maxdepth 1 -printf '%T@ %p\0' | sort -zrn | tail -z -n +"$((max_entries + 1))" | while IFS= read -r -d '' _ entry; do printf '%s\0' "$entry"; done)
There was a problem hiding this comment.
Fixed in decbb39: both helpers now list entries via find -printf '%T@\t%p\0' (NUL-delimited), no ls parsing; verified against a newline-named entry on the runner OS.
| cap_tree_entries() { | ||
| local dir="$1" | ||
| local max_entries="$2" | ||
| local label="$3" |
There was a problem hiding this comment.
OpenCodeReview [maintainability · medium]
Neither cap_tree_entries nor evict_until_free checks whether a cache entry is actively being read or written by a concurrently running CI job. Both functions can rm -rf an in-use lake-packages directory mid-build, causing missing-dependency build failures — the same class of CI disruption this PR is trying to prevent. The maintenance timer runs at 04:30 with a 30m jitter (between ~04:30 and ~05:00), but nothing stops a long-running job from touching the cache at that time. Consider adding a lightweight advisory lock (e.g., flock on a per-entry lockfile, or checking for a sentinel .in-use file) before deleting, or at minimum document that maintenance should run during a known idle window.
Suggested change:
# Before deleting each entry, optionally check for an in-use marker:
if [ -f "$dir/$entry/.in-use" ]; then
continue
fi
rm -rf "$dir/$entry"
There was a problem hiding this comment.
Fixed in decbb39: same fix as the in-use thread: attach-time touch + MIN_ENTRY_AGE_HOURS floor protect entries of running/recent jobs from both functions.
| removed=$((removed + 1)) | ||
| done < <(ls -t "$dir" | tail -n +"$((max_entries + 1))") | ||
|
|
||
| echo "${label}: removed ${removed} entries beyond the newest ${max_entries}" |
There was a problem hiding this comment.
OpenCodeReview [style · low]
cap_tree_entries unconditionally prints its summary line (including when removed is 0), whereas evict_until_free only logs when removed > 0. With a daily timer, this produces a noisy removed 0 entries line every morning, making it harder to spot real eviction events in logs. Apply the same conditional logging for consistency.
Suggested change:
if [ "$removed" -gt 0 ]; then
echo "${label}: removed ${removed} entries beyond the newest ${max_entries}"
fi
There was a problem hiding this comment.
Fixed in decbb39: done — summary only when something was removed or failed.
…n, test update - ci_local_persistence.sh mount now touches the cache entry on attach, and maintenance never deletes entries younger than MIN_ENTRY_AGE_HOURS (default 6) — a running job's mounted lake-packages entry is spared by both the count cap and the low-water eviction - entries are listed via find -printf '%T@\t%p\0' (NUL-delimited, newline- safe) instead of parsing ls output - evict_until_free iterates a single oldest-first snapshot and never retries a failed rm, so a stuck entry cannot spin the loop forever; still-below-threshold is reported instead - cap_tree_entries logs only when it removed or failed something - test_ci_infra_maintenance.py: weekly→daily expectation updated and new coverage for the cap/eviction/in-use-protection invariants (8 tests pass)
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4e4ce8a5-fe6e-4398-8a4f-c93e5ab8018e) |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: decbb3942c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$index" -le "$max_entries" ]; then | ||
| continue | ||
| fi | ||
| if entry_is_recent "$mtime"; then |
There was a problem hiding this comment.
In the setup-lean workflow I checked .github/actions/setup-lean/action.yml:208-212: jobs attach .lake/packages through ci_local_persistence.sh mount, which only protects the entry by touching it at scripts/ci_local_persistence.sh:90. This check uses the mtime captured before sorting, so if a job attaches an old cache entry after list_entries_with_mtime has snapshotted it but before this loop reaches rm -rf, the count-cap/LRU pass can still delete the symlink target under the running job. Re-stat or lock the entry immediately before deletion so the touch can actually protect concurrent attaches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8343f0a: both functions now re-stat the entry (stat -c %Y) immediately before rm — an entry attached/touched after the listing snapshot is spared, a vanished one skipped.
There was a problem hiding this comment.
OpenCodeReview first-pass review
Status: success · Mode: config-docs · 1 finding(s) · 3 files · 74995 tokens · 8 tool calls
✅ Posted 1 inline comment(s).
OCR pilot metrics
- Routing: config-docs (router-v8)
- Changed files: 3 supported / 3 total; Lean 0, trust docs 0, workflow/scripts 3, contracts 0, docs 0
- Changed lines: 134 supported; thresholds large Lean >=3 files or >800 lines
- OCR: status success; comments 1; files 3; tokens 74995; tool calls 8; warnings 0; duration 272s
- Largest changed files: scripts/ci_host_maintenance.sh (+106/-2), scripts/test_ci_infra_maintenance.py (+20/-2), scripts/ci_local_persistence.sh (+4/-0)
Pilot mode: advisory only. Codex Review remains the merge gate.
|
|
||
| avail_gb() { | ||
| df --output=avail -BG "$1" | tail -1 | tr -dc '0-9' | ||
| } |
There was a problem hiding this comment.
OpenCodeReview [bug · medium]
If df fails (e.g. the path becomes inaccessible between the -d check and the call, or df encounters an unexpected error), tr -dc '0-9' will output an empty string. The subsequent [ "$(avail_gb "$dir")" -ge "$min_free_gb" ] comparisons in evict_until_free will then fail with bash: [: : integer expression expected (exit code 2). While set -e won't abort because these are in if conditions, the comparison silently evaluates to false — causing over-eviction (the function never breaks or returns early) and noise on stderr. Consider defaulting to 0 on failure or validating the output is numeric.
Suggested change:
avail_gb() {
local avail
avail="$(df --output=avail -BG "$1" 2>/dev/null | tail -1 | tr -dc '0-9')"
echo "${avail:-0}"
}
There was a problem hiding this comment.
Fixed in 8343f0a: avail_gb now fails safe: df errors report effectively-infinite free space, so eviction stops instead of comparing an empty string (or over-deleting).
- both cap and eviction re-stat an entry's mtime immediately before rm: a job that attached (touched) the entry after the listing snapshot is spared; a vanished entry is skipped - avail_gb fails safe: if df errors, report effectively-infinite free space so eviction stops rather than looping or over-deleting on an empty-string arithmetic comparison
|
@codex review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1c1e0d25-b08b-4bb3-91f6-3cac8bfbc2a8) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8343f0a0c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Refresh the entry mtime on attach: host maintenance treats a recently | ||
| # touched entry as in-use (MIN_ENTRY_AGE_HOURS) and will not delete the | ||
| # cache out from under a running job. | ||
| touch "$primary_dir" |
There was a problem hiding this comment.
When a cold primary is seeded from --fallback-key, this touch is undone by the later cp -a "$fallback_dir/." "$primary_dir/"; checked GNU cp --help, where -a is --preserve=all and includes timestamps. That can copy the fallback directory's old mtime onto the just-attached primary, so the new cap/LRU eviction can classify an active PR cache as old and remove it during the same job. Re-touch the primary after the fallback copy as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8ac2667: the entry is touched again right after the fallback seed, so the cp -a timestamp restore can no longer make a freshly attached entry look old.
There was a problem hiding this comment.
OpenCodeReview first-pass review
Status: success · Mode: config-docs · 0 finding(s) · 3 files · 155930 tokens · 17 tool calls
No comments generated. Looks good to me.
OCR pilot metrics
- Routing: config-docs (router-v8)
- Changed files: 3 supported / 3 total; Lean 0, trust docs 0, workflow/scripts 3, contracts 0, docs 0
- Changed lines: 159 supported; thresholds large Lean >=3 files or >800 lines
- OCR: status success; comments 0; files 3; tokens 155930; tool calls 17; warnings 0; duration 1009s
- Largest changed files: scripts/ci_host_maintenance.sh (+131/-2), scripts/test_ci_infra_maintenance.py (+20/-2), scripts/ci_local_persistence.sh (+4/-0)
Pilot mode: advisory only. Codex Review remains the merge gate.
cp -a fallback/. primary/ re-applies the fallback's old mtime onto the primary directory, undoing the attach-time touch; refresh it after seeding so maintenance still treats the entry as in-use.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ad5f70c6-6744-4718-b563-de9549ec0168) |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
OpenCodeReview first-pass reviewStatus: completed_with_errors · Mode: config-docs · 0 finding(s) · 3 files · 233105 tokens · 22 tool calls Some files could not be reviewed due to errors. Warnings
OCR pilot metrics
Pilot mode: advisory only. Codex Review remains the merge gate. |
1 participant
Incident (2026-07-12)
The runner host (
build-88-99-4-254-1) filled its disk to 100%:/srv/verity-ci-cache/lake-packageshad grown to 664 GB (117 entries, one ~6 GB per PR/branch). The runner listener crash-looped on trace-log writes, GitHub showed the runner offline, andbuild-compiler-binariesjobs (e.g. PR #2153's) sat queued forever — surfacing as 'CI failure with no logs'.The existing weekly age-based maintenance had run the same morning and pruned nothing: every entry was younger than 14 days. Age alone cannot bound a per-PR cache under churn.
Fix
cap_tree_entries: keep only the newestLAKE_PACKAGES_MAX_ENTRIES(default 20)evict_until_free: LRU-evict untilMIN_FREE_GB(default 100) free, so churn spikes between runs can't reach zeroHost state today: I purged to the 15 newest entries (100% → 26% used), restarted the runner (back online), and installed an interim daily cap timer (
verity-ci-cache-cap.timer) which this PR supersedes — remove it after merge +install-systemd.Note
Medium Risk
Deletes large cache directories on the self-hosted runner; wrong thresholds or mtime logic could evict caches mid-job, though recent-entry guards and re-stat before delete mitigate that.
Overview
Prevents runner disk exhaustion from unbounded per-PR
lake-packagescaches (age-only pruning left 117 young entries and filled the host in the 2026-07-12 incident).ci_host_maintenance.shnow capslake-packagesto the newest 20 entries (cap_tree_entries) and LRU-evicts until at least 100 GiB free (evict_until_free), skipping entries younger than 6 hours (MIN_ENTRY_AGE_HOURS). Eviction re-checks mtime before delete to avoid races with active jobs. The systemd timer moves from weekly Sunday to daily at 04:30.ci_local_persistence.shtouches the cache directory on mount (and again aftercp -afrom a fallback) so maintenance treats attached caches as in-use.Tests in
test_ci_infra_maintenance.pyassert the daily schedule and the new lake-packages limits plus mounttouchbehavior.Reviewed by Cursor Bugbot for commit 8ac2667. Bugbot is set up for automated code reviews on this repo. Configure here.