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

Added wait and retries for rate limit response code from Bitbucket server #433

Closed
wants to merge 6 commits into from
Closed

Added wait and retries for rate limit response code from Bitbucket server #433

wants to merge 6 commits into from

Conversation

shankarinece
Copy link

Added 5 seconds wait and retry the same API if Bitbucket responds with rate-limit response code (429). Relates to JENKINS-62479,

Your checklist for this pull request

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

@ryancurrah
Copy link

This is a really handy improvement that would be beneficial for everyone.

@Zauxst
Copy link

Zauxst commented May 10, 2021

@bitwiseman , are you in charge of merging PRs?
Can you take a look over this one?

The rate API is quite important to be fixed especially in large organizations…
Also, Bitbucket does not have any response headers regarding RATE API as mentioned in #414

@tarioch
Copy link
Contributor

tarioch commented May 10, 2021

If you have many open prs, #445 might be important for you as well, on our setup it reduced the number of api calls massively

Copy link
Contributor

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

This is great start! Please address the comments and add some tests of the behavior.

Comment on lines +885 to +891
LOGGER.fine("Bitbucket API rate limit reached, sleeping for 5 sec then retry...");
try {
Thread.sleep(API_RATE_LIMIT_WAIT_TIME_DEFAULT);
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

These two blocks are identical, convert to a method.

} else if (API_RATE_LIMIT_CODE == status) {
LOGGER.fine("Bitbucket API rate limit reached, sleeping for 5 sec then retry...");
try {
Thread.sleep(API_RATE_LIMIT_WAIT_TIME_DEFAULT);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just waiting for 5 seconds will cause a lot of polling and spamming of the server. Please shift to some kind of progressive back off where each of retry waits longer (up to some reasonable limit.

Alternative: see if they the headers of the 429 provide information about when to the limit will reset and sleep for something like that long.

@bitwiseman
Copy link
Contributor

See #414

@smirkybg
Copy link

smirkybg commented Sep 3, 2021

Can we get some progress on this? I'm constantly hitting 429 on our Jenkins setup and we definitely welcome this rate limit progressive back off strategy.

@lifeofguenter
Copy link
Contributor

Investigation was done here: #414 (comment) - there is unfortunately no headers we can use.

Any chance @shankarinece you can have a last look at the comments? E.g. some type of exponential back-off?

@lifeofguenter lifeofguenter added enhancement java Pull requests that update Java code labels Jan 16, 2022
@lifeofguenter
Copy link
Contributor

Closing in favor of #581

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants