You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "Abacn (via GitHub)" <gi...@apache.org> on 2026/09/10 16:59:08 UTC
[PR] [CI] Stop infinite reviewer reassignment, batch state commits, and cl… [beam]
Abacn opened a new pull request, #40096:
URL: https://github.com/apache/beam/pull/40096
…ean up stale state
* Stop infinite reviewer reassignment loops: label with "awaiting triage" if a PR has both "reassigned-reviewers" and "Next Action: Reviewers", and review started >60 days ago.
* Track initial reviewer assignment timestamp (reviewersAssignedAt) in persistent PR state, falling back to PR creation time for legacy PRs.
* Skip reviewer assignment for PRs labeled "awaiting triage" across new PR processing and daily reminder workflows.
* Batch persistent state updates into a single commit and push per workflow execution instead of committing on every file write.
* Incrementally prune closed PR state files (oldest 100 per daily run) from the pr-bot-state branch.
* Document core PR bot logic in scripts/ci/pr-bot/README.md.
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5623895152
> Batch persistent state updates into a single commit and push per workflow execution instead of committing on every file write.
I think I didn't do this because it increases the chances of conflicts and the impact of them (e.g. if multiple files should've been written to)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5623718498
R: @kennknowles @damccorm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5635630906
Stepping back for a moment - what problem are you trying to solve? Is having many commits a problem?
The only downside I'm aware of is that too many commits (at huge scale) can slow down git operations. But currently checking out the branch takes ~1 second, so I don't think we're in danger of having problems there.
I guess each commit induces some latency, but these workflows are running in ~1 minute (with the time dominated by other GitHub API calls).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5624708750
How about this (latest commit): the commit remains per file, however it now batches state changes for single PR (in findPrsNeedingAttention, processNewPrs, pr-updates' assignToNextReviewer and assignReviewerSet) so the number of commit is reduced by 3x while conflict remains unlikely (because they remains single PR)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5624564520
> I did this because I realize we commit multiple times during a single run of the workflow, e.g. in processNewPrs.ts, a single PR with 2 labels triggers 3 separate commits (writePrState + 2 × writeReviewersForLabelState) seconds apart. We now record a timestamp on PR creation and will make it worse.
Does this actually cause problems though? 2 (or 10) small commits are not really meaningfully different than a single large commit here.
> To avoid merge conflict we can add the 3 pr-bot workflow into a concurrency_group (pr-bot-prs-needing-attention.yml, pr-bot-pr-updates.yml, pr-bot-new-prs.yml). Checking running history all of them are running for 1-2 minutes. If number of PR comments is less than "~50" per hour it's managable.
This seems less desirable to me. In particular, there are some issues with slow pr-bot-pr-updates. As an example, consider:
1. pr-bot-new-prs gets triggered but queued
2. I comment `R: foo` and mark my PR ready for review (not draft)
Today, there is a small chance of the PR getting assigned before the `R:` comment is processed (you'd need pr-bot-new-prs to have pathological timing). If there is backlog, the odds get much worse with this change.
I think this is ok if there is a problem we're solving, but I don't think we're actually seeing poor behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5623733940
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment `assign set of reviewers`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5623957002
So I think my only feedback is that I think we should keep the previous commit semantics (AFAIK they haven't caused any issues, but let me know if that's wrong)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5636139814
Checked an example run: https://github.com/apache/beam/actions/runs/34609044939/job/103294594744
```
Fri, 11 Sep 2026 14:15:55 GMT /usr/bin/git fetch origin pr-bot-state
Fri, 11 Sep 2026 14:16:29 GMT From https://github.com/apache/beam
Fri, 11 Sep 2026 14:16:29 GMT * branch pr-bot-state -> FETCH_HEAD
Fri, 11 Sep 2026 14:16:29 GMT * [new branch] pr-bot-state -> origin/pr-bot-state
Fri, 11 Sep 2026 14:16:37 GMT /usr/bin/git checkout pr-bot-state
Fri, 11 Sep 2026 14:16:39 GMT Switched to a new branch 'pr-bot-state'
```
checkout itself takes 2 s (39 - 37) but fetch the branch `pr-bot-state` currently takes 42 s (16:37 - 15:55).
https://github.com/apache/beam/tree/pr-bot-state says "This branch is 52776 commits ahead of and 13373 commits behind master."
fetch and checkout onto `pr-bot-state` already takes half of the time of a typical run( while the initialization that checkout onto master takes 11 s)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5636647616
Oh that's true, I was looking at the wrong log line as the base. With that said, I think it is actually because of the fetch depth. The initial checkout does fetch-depth=1 by default (https://github.com/actions/checkout/blob/f548e57e544e1ff5a4c46bf1e1b8685f8e4a348a/action.yml#L74)
On a fresh machine, I tried:
- `git clone https://github.com/apache/beam --depth=1`
- `time git fetch origin pr-bot-state` (took 1m10s)
Then I tried:
- `git clone https://github.com/apache/beam --depth=1`
- `time git fetch origin pr-bot-state --depth=10` (took 5s) - `--depth=1` is even faster
So I think if we want to drive down fetch times, we should just reduce the fetch depth.
Since we don't have real locking semantics, I'm interested in keeping the bot state as close as possible to the real state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5623904098
Other than that, this set of changes SGTM. With that said, I think Kenn is trying to reimagine some of this in https://github.com/apache/beam/pull/39980
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5624490049
> > Batch persistent state updates into a single commit and push per workflow execution instead of committing on every file write.
>
> I think I didn't do this because it increases the chances of conflicts and the impact of them (e.g. if multiple files should've been written to)
I did this because I realize we commit multiple times during a single run of the workflow, e.g. in processNewPrs.ts, a single PR with 2 labels triggers 3 separate commits (writePrState + 2 $\times$ writeReviewersForLabelState) seconds apart. We now record a timestamp on PR creation and will make it worse.
To avoid merge conflict we can add the 3 pr-bot workflow into a concurrency_group (pr-bot-prs-needing-attention.yml, pr-bot-pr-updates.yml, pr-bot-new-prs.yml). Checking running history all of them are running for 1-2 minutes. If number of PR comments is less than "~50" per hour it's managable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn merged PR #40096:
URL: https://github.com/apache/beam/pull/40096
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CI] Stop infinite reviewer reassignment, batch state commits, clean up stale state [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #40096:
URL: https://github.com/apache/beam/pull/40096#issuecomment-5637268459
> Since we don't have real locking semantics, I'm interested in keeping the bot state as close as possible to the real state.
Currently the actual guard of diverge is the `git pull` statement right before committing
https://github.com/apache/beam/blob/2e7e9d371e8da0286cce358c18fc07fa3cf556da/scripts/ci/pr-bot/shared/persistentState.ts#L36
For fetch, bot state could already diverge from the remote branch from the second write state on:
https://github.com/apache/beam/blob/2e7e9d371e8da0286cce358c18fc07fa3cf556da/scripts/ci/pr-bot/shared/persistentState.ts#L125-L126
- because the second `git fetch origin pr-bot-state && git checkout pr-bot-state` is always a no-op (fetch won't affect current branch that already exist)
In other words, adding `--depth=1` to fetch wouldn't introduce new concern (git supports both fast-forward-pull and push from shallow branch, as local branch is guaranteed to have common ancestor with the remote). I'll make the change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org