RFR: 1937: Re-evaulate all PRs when .jcheck/conf changes

Erik Joelsson erikj at openjdk.org
Tue Jun 13 18:07:03 UTC 2023


On Tue, 13 Jun 2023 16:31:24 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> As Erik said in the issue: "When we bump the version in .jcheck/conf from N to N+1 in the jdk master branch, PRs with CSRs are often not updated to reflect this."
>> 
>> This root cause of this is that changing of .jcheck/conf in target ref of the pr will not be able to trigger checkWorkItem for this pr.
>> 
>> In this patch,
>> 1. The prBot will monitor the changes to the .jcheck/conf file in any branch(except for PR branches).
>> 2. If the prBot detects an update to the .jcheck/conf file in a branch, it will trigger a checkWorkItem for all open PRs whose target ref is the branch where the .jcheck/conf file was updated.
>> 3. To solve the problem related with bot restarting, the contents of the .jcheck/conf file in the target branch of a PR will be included in the metadata hash.
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestBot.java line 189:
> 
>> 187:                 .map(HostedBranch::name)
>> 188:                 .filter(name -> !name.startsWith("pr/"))
>> 189:                 .toList();
> 
> Currently, fetching all existing branches is the main overhead in this feature. It takes about 2s for JDK repo. If this overhead is not acceptable, the other way to solve this is store the branches in bot config.

I assume this is taking a long time in mainline due to all the pr branches. We also have several update repositories (internally) with a lot of branches. With the future move to branched development in mainline, there will be even more in the main jdk repo. Those will not be filtered out either, so will add more rounds in the loop fetching .jcheck/conf below.

I don't think fetching all branches and checking them for .jcheck/conf is a good idea. Instead I think we should keep a `Map` of branch names to set of PR IDs. Then we only need to iterate over the known targeted branches when looking for .jcheck/conf changes. 

To maintain the map, look at every PR found by the poller, if it's open, add it to the set for the branch, if it's closed, remove it. The poller will return all open PRs in the first round, so at that point we know it's up to date. After that it will catch all updates to all PRs, so we know we will catch when a PR is closed and get it removed from the map.

Both GitHub and GitLab support querying for pull requests that target a specific branch, so when we get a hit on .jcheck/conf, we can fetch just the relevant pull requests. (In GitHub it's called `base`)

-------------

PR Review Comment: https://git.openjdk.org/skara/pull/1531#discussion_r1228495829


More information about the skara-dev mailing list