Repository navigation
ci: use remote ccache only, drop the duplicate local cache - #77
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughWhen Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Mechanism checks out: CCACHE_REMOTE_ONLY is in CCACHE_PASSTHROUGH_VARS, so it reaches both Three corrections to the text:
|
Builds already point ccache at the shared Redis (CCACHE_REMOTE_STORAGE, set per-runner by armbian/actions runner-clean). The local ccache directory is then a second copy of the same objects, written to the disk that is the builders' actual constraint: measured ~44 MB per 30 s per runner, against a device showing 2.2 s average write latency and queue depth 178 under the nightly load. CCACHE_REMOTE_ONLY is already supported by the ccache-remote extension and by ccache 4.9.1 on the runners; it just was never passed. It has to go here rather than in runner-clean's $GITHUB_ENV export, because docker.sh forwards a fixed env allowlist into the build container and CCACHE_* is not on it -- which is why CCACHE_REMOTE_STORAGE is passed as a compile.sh argument too. Set inside the existing 'remote storage is configured' guard, so a build without a remote keeps its local cache untouched. Trade-off, stated because it is real: there is no local fallback. If Redis is unreachable the cache is simply absent for that build and objects recompile. CCACHE_REDIS_CONNECT_TIMEOUT is 500 ms, so that costs lookups, not a stall. build-images-chunk.yml needs no change: it exports CCACHE_REMOTE_STORAGE to $GITHUB_ENV but never passes ccache params to compile.sh, so image builds use no ccache at all today. Kernel compiles happen in the artifacts path. Signed-off-by: Igor Pecovnik <igor@armbian.com>
514ab51 to
0054ef0
Compare
iav
left a comment
There was a problem hiding this comment.
Approving. One fix for the code comment: 500 ms is the extension's CCACHE_REDIS_CONNECT_TIMEOUT, which only applies to endpoints it discovers via DNS-SD; with the URL from github-runners.jq ccache uses its own 100 ms connect timeout.
|
✅ This PR has been reviewed and approved — all set for merge! |
|
lets try live |
Builds already point ccache at the shared Redis —
CCACHE_REMOTE_STORAGEis set per-runner by armbian/actionsrunner-cleanfromgithub-runners.jq, and passed tocompile.shat line 143 along withENABLE_EXTENSIONS=ccache-remote.The local ccache directory is then a second copy of the same objects, written to the disk that is the builders' actual constraint.
Measured
On
rack-ryzenunder the normal nightly load:io full avg10=53.7%vscpu some avg10=12.3%The box is not short of CPU; it cannot retire writes. The local cache is ~20% of each runner's write rate and buys nothing once a shared remote exists.
Why here and not in
runner-cleandocker.shforwards a fixed env allowlist into the build container (CCACHE_DIRandCPUTHREADSare on it;CCACHE_*is not). An env var exported to$GITHUB_ENVnever reaches the build — which is exactly whyCCACHE_REMOTE_STORAGEis passed as acompile.shargument rather than relying on the export.CCACHE_REMOTE_ONLYhas to travel the same way.It's set inside the existing
if [ -n "${CCACHE_REMOTE_STORAGE}" ]guard, so a build with no remote keeps its local cache untouched.Trade-off
There is no local fallback. If Redis is unreachable the cache is simply absent for that build and objects recompile.
CCACHE_REDIS_CONNECT_TIMEOUTis 500 ms, so that costs lookups rather than stalling. Builder and Redis (10.0.40.2) are on the same LAN.CCACHE_REMOTE_ONLYis already supported by theccache-remoteextension (it's inCCACHE_PASSTHROUGH_VARS, and documented in the extension header as "Disable local cache, use remote only (saves local disk space)") and by ccache 4.9.1 on the runners. It was simply never passed.Not changed
build-images-chunk.ymlexportsCCACHE_REMOTE_STORAGEto$GITHUB_ENV(line 138) but never builds aCCACHE_PARAMSarray or passes anything tocompile.sh— so image builds use no ccache at all today and this flag would be inert there. Kernel compiles happen in the artifacts path. Worth a separate look at whether that export is meant to be wired up.