Supersede push runs by branch, not run id - #965
Conversation
The concurrency group fell back to github.run_id when there was no pull request number, which is unique per run, so consecutive pushes to dev never collided and each kept a full matrix alive.
|
Warning Review limit reached
Next review available in: 21 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
![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: 635d6e4577
ℹ️ 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".
| # Supersede the previous run: by pull request, or by branch for pushes. | ||
| concurrency: | ||
| group: CodSpeed-${{ github.event.pull_request.number || github.run_id }} | ||
| group: CodSpeed-${{ github.event.pull_request.number || github.ref }} |
There was a problem hiding this comment.
Because this workflow also supports workflow_dispatch, the fallback now groups a manually requested benchmark with push runs for the selected ref. With cancel-in-progress: true, a subsequent push to that branch cancels the manual benchmark (and a manual dispatch can cancel the push benchmark), although the stated behavior only intends branch supersession for pushes; retain a unique fallback such as github.run_id for non-push, non-PR events.
Useful? React with 👍 / 👎.
1 participant
Description
Every test workflow's concurrency group falls back to
github.run_idwhen there is no pull request number:run_idis unique per run, so on a push the group never collides andcancel-in-progresshas nothing to cancel. Consecutive pushes todevtherefore each keep a full matrix alive. This morning three dev pushes (08:57, 08:58, 09:14) were running simultaneously —TestCodealone is 14 jobs (setup+conda_env+ 3 OS × 3 Python + 3network_tests), so that is ~42 jobs for one branch against the org's 20-concurrent-job cap. Queued jobs peaked at 84 and PR CI stalled behind it.Switching the fallback to
github.refputs all pushes to a branch in one group, so a new commit ondevsupersedes the in-flight run for the previous one. Pull requests are unaffected: they still group bypull_request.numberand never reach the fallback.masteranddevgroup separately, sincegithub.refdiffers.Six workflows carried the same fallback:
runtests.yml,run_min_dep_tests.yml,lint.yml,profile.yml,test_free_threaded.yml,test_wasm.yml.Trade-off worth stating: a rapid series of merges to
devnow leaves intermediate commits untested, since only the newest push keeps its run. That is the usual reason to group by branch, and it is what the fallback already does for pull requests.Two related contributors to the same pile-up are left alone here and can be handled separately:
CancelPRRunsis an 8-job matrix ofechostatements, so it queues behind the very backlog it exists to drain (jobs sat queued for over an hour this morning).network_testsadds 3 jobs to everyTestCoderun and is subject to the known stall in Fix remote HTTP test stalls and tune HTTP HDF5 caching #784.Changelog
Checklist
I have:
docs/contributing/general_guidelines.qmd).