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

Add delay between pull request updates #278

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

bluekeyes
Copy link
Member

@bluekeyes bluekeyes commented Jan 22, 2022

In projects with many open PRs using the automatic updates feature,
Bulldozer can trigger GitHub's secondary rate limits by updating (and
possibly querying) pull requests too quickly. To avoid this, add some
delay between operations.

For queries, use a fixed 250ms delay for now. This shouldn't slow things
down too much, but does prevent Bulldozer from blasting through PRs as
fast as possible.

For updates, use an exponential delay that starts at 1s and maxes out at
60s. I believe the secondary limits (at least for GitHub Enterprise) are
computed over a minute, so this will limit us to one update per minute
for large batches. I don't think this will be prohibitively slow, but we
could explore making this configurable in the future.

As part of this, refactor the logic so that we don't block queries on
previous updates.

This should help with the issues reported in #276.
Requires #277 to merge first.

Copy link
Member

@asvoboda asvoboda left a comment

Choose a reason for hiding this comment

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

While I think the approach of sleeping ahead of time works, it'd be nice to consider handling github rate limit errors and retrying with delays. I know the latter approach needs to handle state a bit differently from what we're doing today, so maybe its not a great use of time atm.

Base automatically changed from bkeyes/single-config-fetch to develop January 24, 2022 16:42
In projects with many open PRs using the automatic updates feature,
Bulldozer can trigger GitHub's secondary rate limits by updating (and
possibly querying) pull requests too quickly. To avoid this, add some
delay between operations.

For queries, use a fixed 250ms delay for now. This shouldn't slow things
down too much, but does prevent Bulldozer from blasting through PRs as
fast as possible.

For updates, use an exponential delay that starts at 1s and maxes out at
60s. I believe the secondary limits (at least for GitHub Enterprise) are
computed over a minute, so this will limit us to one update per minute
for large batches. I don't think this will be prohibitively slow, but we
could explore making this configurable in the future.

As part of this, refactor the logic so that we don't block queries on
previous updates.
@bluekeyes
Copy link
Member Author

Based on @daiglebg's testing in #276, I've changed this to use a fixed 2 second delay for updates. With the maximum value that low, exponential backoff hit the limit so fast that the complexity didn't seem worthwhile. To compensate, I've added configuration options for both delays so that users can increase them or disable them if needed.

The configuration is undocumented for now, as I think most people should use the defaults.

@asvoboda
Copy link
Member

@bluekeyes are you still interested in merging this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants