Renovate: Redesign: Improve branch/PR limit logic

[jlsjonas]

What Renovate type are you using?

Self-hosted for Bitbucket Server

Describe the bug

Quite similar to #5162, from what we're experiencing more branches are created than PRs are opened (which is fine) due to several of them not reaching stability days yet. However it seems that once they do reach stability-days, a PR is created without taking into account the concurrency limit. Our config defines a max. of 3 PRs, yet there's 8 currently open.

Relevant debug logs

Been running all weekend on a schedule, sadly wasn't preserving debug logs for long enough.

To Reproduce

From what I see, have a repo with several packages that have been updated less then stability-days ago; with not-pending pr creation and prConcurrentLimit set.

Additional context

[rarkins]

This is behaving as originally designed. prConcurrentLimit is checked at branch creation time, not PR creation time, because most people wouldn't think "it's ok to have 100 branches as long as there's only 3 PRs". Once a branch is created, PRs are subsequently created without a further check of the limit. We may rename the feature though to make it clearer to people.

Would you prefer that Renovate leaves the excess branches orphaned without PRs and maintains a hard 3 PR limit?

[rarkins]

I forgot to clarify, the feature was created before we had the concept of prCreation=not-pending. i.e. back then PRs were created immediately after branches. Once we split it, we kept the check at branch creation time where it always was.

[jlsjonas]

I actually based myself on your comment here where you say the exact opposite

What goes wrong is that we're counting PRs, not branches.

I had already spotted references where it is based on branches though; but then it's definitely not by design, as way more branches are created than the concurrency limit.

An issue (sorry, forgot to mention it in the OP) with counting branches and not PRs is that, using branch-based automerge, you might get "stuck" until branches are manually merged, even when there's more work to actually do.

[rarkins]

where you say the exact opposite

That description wasn't meant to describe the exact design/state but rather where it was going wrong. Anyway I think it's time to revise things so I'm going to hijack this issue accordingly.

[rarkins]

So as described earlier, we have two limits called prConcurrentLimit and prHourlyLimit however these are actually evaluated at branch creation time and not PR creation time, which is not always the same job.

Recall that the original purpose of these limits were to save CI systems from getting swamped, particularly at onboarding time. For many, it's the branch/commit that causes CI tests to start and not the PR.

Here are some possibilities to consider:

• Rename the current config options to be branch instead of PR. Keep behavior the same
• Keep a single option but check it both at branch creation time as well as PR creation. This could leave orphan branches with no PR though.
• Have both branch and PR configuration options that apply at each stage. Probably we'd want the option to configure just one of them and have the other "inherit" if it's unconfigured. But are there really scenarios where people need different limits?

[jlsjonas]

But are there really scenarios where people need different limits?

Actually our case prefers it, (and ,as we're self-hosted, we use prCommitsPerRunLimit to avoid swamping our CI)
Especially since BitBucket (server at least) marks the lockfile as conflicted even if no conflicting changes happened, we want to have a limited amount of open PRs to manually verify, but don't mind a few more so automerge (via branch) for patching can still happen even if the concurrency limit is reached for open PRs.

To consider: If PR concurrency is reached, and a new patch branch is created, but has failing tests, we should _prioritize_ that branch for PR creation once a "PR slot opens"

Let's take a simple example, where a project has 6 patch-updates, and the project is configured with not-pending, branch automerge & a concurrency limit of 3 PRs and 6 branches:

• 6 branches are created
• 2 branches have green tests and are automerged to master
• 3 failed, PRs are opened; the last is still pending
• all 4 remaining branches get rebased due to lock conflicts
• the last branch also fails, it sits there _waiting_ for a PR slot
• one/multiple PRs get manually resolved
• the last branch gets a PR _first_, before any new branches get created

[rarkins]

We can maybe have a plan like this:

• Start checking PR limits at PR creation time (in addition to existing branch creation time)
• Add new options for branch limits. if unconfigured then they'll inherit from the pr limits for backwards compatibility