fix(ci): stop cancelling in-progress Backport Approvals runs - #8523
Open
mengw15 wants to merge 3 commits into
Open
fix(ci): stop cancelling in-progress Backport Approvals runs#8523mengw15 wants to merge 3 commits into
mengw15 wants to merge 3 commits into
Conversation
Backport Approvals is a required status check, and GitHub takes the latest check run with that name. With cancel-in-progress on, two events close together (a push right after a label or review request) let the newer run cancel the older one, whose cancelled state then landed after the newer run's success and blocked the merge as a failed required check (seen on apache#8516). The job runs in seconds, so queue instead. Closes apache#8522. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8523 +/- ##
=========================================
Coverage 95.90% 95.91%
Complexity 459 459
=========================================
Files 586 586
Lines 33635 33635
Branches 4182 4182
=========================================
+ Hits 32257 32260 +3
Misses 811 811
+ Partials 567 564 -3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 427 | 0.26 | 22,538/32,454/32,454 us | 🔴 -11.9% / 🔴 +115.4% |
| 🟢 | bs=100 sw=10 sl=64 | 917 | 0.56 | 106,476/126,922/126,922 us | 🟢 -17.4% / 🔴 +23.6% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,081 | 0.66 | 920,802/1,067,737/1,067,737 us | 🔴 +11.1% / 🔴 +9.0% |
Baseline details
Latest main 0b103f8 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 427 tuples/sec | 483 tuples/sec | 807.77 tuples/sec | -11.6% | -47.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.26 MB/s | 0.295 MB/s | 0.493 MB/s | -11.9% | -47.3% |
| bs=10 sw=10 sl=64 | p50 | 22,538 us | 20,824 us | 12,280 us | +8.2% | +83.5% |
| bs=10 sw=10 sl=64 | p95 | 32,454 us | 31,052 us | 15,066 us | +4.5% | +115.4% |
| bs=10 sw=10 sl=64 | p99 | 32,454 us | 31,052 us | 19,166 us | +4.5% | +69.3% |
| bs=100 sw=10 sl=64 | throughput | 917 tuples/sec | 938 tuples/sec | 1,054 tuples/sec | -2.2% | -13.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.56 MB/s | 0.573 MB/s | 0.643 MB/s | -2.3% | -12.9% |
| bs=100 sw=10 sl=64 | p50 | 106,476 us | 102,058 us | 96,023 us | +4.3% | +10.9% |
| bs=100 sw=10 sl=64 | p95 | 126,922 us | 153,603 us | 102,660 us | -17.4% | +23.6% |
| bs=100 sw=10 sl=64 | p99 | 126,922 us | 153,603 us | 114,950 us | -17.4% | +10.4% |
| bs=1000 sw=10 sl=64 | throughput | 1,081 tuples/sec | 1,081 tuples/sec | 1,086 tuples/sec | 0.0% | -0.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.66 MB/s | 0.66 MB/s | 0.663 MB/s | 0.0% | -0.5% |
| bs=1000 sw=10 sl=64 | p50 | 920,802 us | 923,947 us | 940,412 us | -0.3% | -2.1% |
| bs=1000 sw=10 sl=64 | p95 | 1,067,737 us | 960,898 us | 979,930 us | +11.1% | +9.0% |
| bs=1000 sw=10 sl=64 | p99 | 1,067,737 us | 960,898 us | 1,015,625 us | +11.1% | +5.1% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,468.82,200,128000,427,0.260,22537.76,32454.29,32454.29
1,100,10,64,20,2180.77,2000,1280000,917,0.560,106476.41,126922.34,126922.34
2,1000,10,64,20,18498.43,20000,12800000,1081,0.660,920802.24,1067736.75,1067736.75
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Narrow the documented guarantee or remove the shared concurrency group to ensure every event is evaluated.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates the required Backport Approvals workflow to avoid cancelling in-progress runs.
Changes:
- Sets
cancel-in-progresstofalse. - Adds concurrency rationale and documentation.
File summaries
| File | Summary |
|---|---|
.github/workflows/backport-approval-check.yml |
Adjusts workflow concurrency; pending runs may still be replaced, so the FIFO/every-run guarantee is not met. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
GitHub keeps at most one running and one pending run per concurrency group and replaces the pending one on a newer event, so 'queue, every run completes' overstated it. What holds and matters: the in-progress run is never cancelled, and a replaced pending run is cancelled before its successor even starts, so the latest check run always ends as a real verdict. A skipped intermediate evaluation loses nothing: the job reads the live labels and reviews at run time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Backport Approvalsis a required status check onmain, and GitHub evaluates the LATEST check run with that name. The workflow's concurrency group hadcancel-in-progress: true, so when two PR events land close together (a push right after a review request or a label change), the newer run cancels the older one — and when the older run's "cancelled" terminal state is recorded after the newer run's success, the required check reads as failed and the merge button is blocked. This happened on #8516: the check showed a cancelled run as its latest state while a sibling run of the same commit had already passed; re-running the cancelled run unblocked the merge.The job completes in seconds, so cancellation saves nothing. This stops cancelling the in-progress run (
cancel-in-progress: false). GitHub still keeps at most one pending run per concurrency group and replaces it on a newer event, but a replaced pending run is cancelled before its successor even starts, so its cancelled state can never land last: the latest check run on the commit always ends as a real verdict. A skipped intermediate evaluation loses nothing, because the job reads the live labels and reviews at run time rather than the event payload.Any related issues, documentation, discussions?
Closes #8522. Observed on #8516 (a cancelled
Backport Approvalsrun blocked an otherwise green merge).How was this PR tested?
Config-only change to the workflow's concurrency setting; no executable code path changes. Verified the failure mechanism on #8516: the blocked merge showed the cancelled run as the latest
Backport Approvalscheck run, and re-running it (6s pass) unblocked the merge immediately.Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (Claude Fable 5, Anthropic). Reviewed by the author before submission.
🤖 Generated with Claude Code