workflows: run external-package purge once per workflow, not per chunk#286
workflows: run external-package purge once per workflow, not per chunk#286igorpecovnik merged 2 commits intomainfrom
Conversation
The repository-update workflow fans out infrastructure-download-external across 4 chunks so download-time aptly operations stay under strategy.matrix's 256-entry cap, and the actual per-package download step does use CHUNK_INDEX to slice its work. preclean / postclean don't. Both build the same (release × package) matrix on every invocation and run repo.sh -c delete against the shared /publishing/repository-debs repository. At 4 chunks that meant four parallel copies of aptly's delete op touching the same database at the same time. aptly serialises via its own lockfile so it usually doesn't corrupt, but it's wasted work at best and a corruption hazard the moment a lock-wait times out or is bypassed. Gate both jobs on CHUNK_INDEX == 0 so the purge runs exactly once per workflow, independent of the download fanout. Chunks 1..3 now skip preclean/postclean entirely and jump straight to their slice of the download matrix. Download parallelism is preserved; purge goes back to being the single-threaded operation aptly's data model requires.
WalkthroughA GitHub Actions workflow file was modified to change the conditional execution of Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/infrastructure-download-external.yml:
- Around line 138-143: Add a job-level concurrency block to the job that runs
when inputs.PURGE == true and inputs.CHUNK_INDEX == 0 so purge operations across
workflow runs are serialized; specifically, add a concurrency: group expression
that is enabled only when inputs.PURGE is true (e.g. derive a constant group
name like "infrastructure-purge-repo-debs" when inputs.PURGE is true) and set
cancel-in-progress: false to avoid cancels; update the job that contains the
CHUNK_INDEX gate (the job that performs the repo.sh -c delete / postclean steps)
to include this concurrency block so concurrent workflow runs do not race
mutations to /publishing/repository-debs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 211f2441-05f2-4a25-bd86-787df5597081
📒 Files selected for processing (1)
.github/workflows/infrastructure-download-external.yml
The self-hosted 'repository' runner pool is shared across workflows (and across runs of the same workflow). Several jobs already pre-chown $GITHUB_WORKSPACE back to the runner user before actions/checkout@v6 fires — update-repository, Sync, Index — precisely because prior sudo operations on tools/repository/repo.sh output can leave root-owned files in the workspace. Checkout's default clean: true then fails with EACCES unlink when it hits one: Error: File was unable to be removed Error: EACCES: permission denied, unlink '/home/actions-runner-23/_work/armbian.github.io/armbian.github.io/build/.coderabbit.yaml' Copying and prepare-repos in the caller workflow, and postclean in the reusable download-external workflow, were the three remaining repo-runner jobs missing this guard. Add the same 'Fix workspace ownership' step to each. Makes workspace state robust to whatever the previous job left behind, without having to reason about which specific sudo op produced the root-owned file.
Summary
infrastructure-download-external.yml'spreclean/postcleandidn't useCHUNK_INDEXto filter. Both built the same(release × package)matrix on every chunk invocation and ranrepo.sh -c deleteagainst the shared/publishing/repository-debsrepo.infrastructure-repository-update.yml, that was 4 parallel copies of aptly's delete op contending on the same repository state. aptly's lockfile usually saves it from corruption, but it's redundant work at best and a corruption hazard the moment a held lock times out or gets bypassed.inputs.CHUNK_INDEX == 0so the purge runs exactly once per workflow. Chunks 1..3 now skippreclean/postcleanentirely and go straight to their slice of the download matrix.CHUNK_INDEXand is the reason for the 4-way fanout. Purge goes back to being the single-threaded operation aptly's data model requires.Test plan
Infrastructure: APT repositories updatewithpurge_external=true— confirmPurgejobs showcompletedon chunk 0 only andskippedon chunks 1..3.Mirror/ download matrix jobs run as before (unaffected by the gate).Purge old <pkg> from <release>sub-matrix runs once per (release, package) tuple total, not 4× per tuple.code,google-chrome-stable, etc. (one version each, older ones deleted).