Skip to content

New option commitsHourlyLimit #35268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rarkins opened this issue Apr 8, 2025 · 3 comments · May be fixed by #36226
Open

New option commitsHourlyLimit #35268

rarkins opened this issue Apr 8, 2025 · 3 comments · May be fixed by #36226
Labels
core:config Related to config capabilities and presets core:git Related to our git platform layer priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others

Comments

@rarkins
Copy link
Collaborator

rarkins commented Apr 8, 2025

Discussed in #35068

Originally posted by felipecrs March 29, 2025

Tell us more.

This is derived from this other discussion.

Currently, there is prHourlyLimit to help preventing overloading of CI machinery. That option will rate-limit the number of PRs created per hour, and helps a lot during onboard of new projects.

However, for projects which are already onboarded and has most of the PRs created already, this option will not help much.

For example, imagine the following configuration:

{
  "prConcurrentLimit": 50,
  "prHourlyLimit": 5,
  "rebaseWhen": "behind-base-branch"
}

This would create 5 PRs hourly and stopping when it reaches 50. When one of the PRs are merged, another one will be created, but all the other 49 will also be rebased in the same run, because there is no option to rate-limit the number of commits that can happen per hour.

When a PR is changed with new commits, the CI workflows will be triggered, and in this case, 50 workflows will be triggered, potentially causing CI overload.

It would be great if an option like commitsHourlyLimit was implemented. This option would supersede prHourlyLimit and would prevent Renovate from pushing new commits to existing PRs if they surpass the limit configured.

One implementation idea is to adjust the following code:

// API will know whether to create new branch or not
return scm.commitAndPush({
baseBranch: config.baseBranch,
branchName: config.branchName,
files: updatedFiles,
message: config.commitMessage!,
force: !!config.forceCommit,
platformCommit: config.platformCommit,
// Only needed by Gerrit platform
prTitle: config.prTitle,
});

- return scm.commitAndPush({
+ const result = await scm.commitAndPush({
    baseBranch: config.baseBranch,
    branchName: config.branchName,
    files: updatedFiles,
    message: config.commitMessage!,
    force: !!config.forceCommit,
    platformCommit: config.platformCommit,
    // Only needed by Gerrit platform
    prTitle: config.prTitle,
  });
+
+ if (result) {
+    incCountValue('HourlyCommits');
+  }
+
+  return result;

From what I understand, this function only returns a value when the push operation successfully happened.

@rarkins rarkins added core:config Related to config capabilities and presets core:git Related to our git platform layer priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Apr 8, 2025
@HonkingGoose
Copy link
Collaborator

Explain to user what's happening due to the commit rate-limit

We should explain why Renovate is rate-limited to the user. I guess we can refactor the code that explains the "old rate limit based on PRs per hour" into something like this:

Rate limited PRs (due to commit per hour limit)

  • Rate-limited PR number 1
  • Rate-limited PR number 2
  • Rate-limited PR number 3

Should we replace prHourlyLimit with commitsHourlyLimit?

It would be great if an option like commitsHourlyLimit was implemented. This option would supersede prHourlyLimit and would prevent Renovate from pushing new commits to existing PRs if they surpass the limit configured.

I'm not sure we should replace the prHourlyLimit with commitsHourlyLimit? I think it will be easier to mess Renovate up by setting a commit limit that's too low, than messing up with a low PR limit. At least with the low PR limit Renovate still does things, but with a low commit limit you may run into unexpected behavior somehow?

@rarkins
Copy link
Collaborator Author

rarkins commented Apr 9, 2025

We should not replace prHourlyLimit

@felipecrs
Copy link
Contributor

That's right. Functionality-wise, it supersedes prHourlyLimit when set, but prHourlyLimit remains there.

BTW, maybe the best place for it is:

renovate/lib/util/git/index.ts

Lines 1180 to 1182 in 4f60a8b

logger.debug({ result: pushRes }, 'git push');
incLimitedValue('Commits');
result = true;

@felipecrs felipecrs linked a pull request May 29, 2025 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core:config Related to config capabilities and presets core:git Related to our git platform layer priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants