Skip to content
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

feat: Limit HTTP concurrency and frequency by default #27621

Merged
merged 19 commits into from
Mar 5, 2024

Conversation

zharinov
Copy link
Collaborator

@zharinov zharinov commented Feb 28, 2024

Changes

  • Set default concurrency to 5 requests to a single host
  • Allow 5 requests per second to a single host by default

Context

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these instead be in a default hostRules value?

@zharinov
Copy link
Collaborator Author

I think it doesn't make too much sense, since we still have to hard-code this default value if there is no host rules found.

@rarkins
Copy link
Collaborator

rarkins commented Feb 28, 2024

HostRules are mergeable so it should always be found? Plus has more visibility in config docs

@zharinov
Copy link
Collaborator Author

zharinov commented Feb 28, 2024

image

Unfortunately, it doesn't seem to work during testing nor during runtime.

@zharinov
Copy link
Collaborator Author

HostRuleSearchResult all-optional fields make me think it probably is not so easily fixable

@zharinov zharinov changed the title feat: Throttle and rate limit HTTP by default feat: Limit HTTP concurrency and frequency by default Feb 28, 2024
@zharinov zharinov requested a review from viceice March 4, 2024 15:29
viceice
viceice previously approved these changes Mar 4, 2024
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/config/options/index.ts Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
lib/config/options/index.ts Outdated Show resolved Hide resolved
@rarkins rarkins added this pull request to the merge queue Mar 5, 2024
Merged via the queue into renovatebot:main with commit 147d38a Mar 5, 2024
35 checks passed
@rarkins rarkins deleted the feat/http-rate-limit-by-default branch March 5, 2024 15:07
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.228.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@erezrokah
Copy link

erezrokah commented Mar 6, 2024

Hi 👋 I believe this caused our self hosted GitHub Action running time to increase from 10m to 2h, is there a way to disable it for all hosts? See #27759

@zharinov
Copy link
Collaborator Author

zharinov commented Mar 6, 2024

Hi @erezrokah, could you try this host rule:

"hostRules": [
  {
    "concurrentRequestLimit": 0,
    "maxRequestsPerSecond": 0
  }
]

It seems to work for my local setup, hopefully it will help you.

@zharinov
Copy link
Collaborator Author

zharinov commented Mar 6, 2024

@rarkins Should we somehow make unlimited default for actions?

And one more unrelated question: it's recommended to persist repository cache, but what about package cache? If it doesn't, would you recommend it?

@rarkins
Copy link
Collaborator

rarkins commented Mar 6, 2024

Which host/type is needing the increased rate and/or concurrency? I'm confused where/how actions comes into it

@rarkins
Copy link
Collaborator

rarkins commented Mar 6, 2024

Actually let's switch the discussion to #27759. We can revert this if necessary

rarkins added a commit that referenced this pull request Mar 7, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants