Fix API calls failing because of secondary api limits#1853
Merged
bitwiseman merged 3 commits intohub4j:mainfrom Jun 19, 2024
Merged
Fix API calls failing because of secondary api limits#1853bitwiseman merged 3 commits intohub4j:mainfrom
bitwiseman merged 3 commits intohub4j:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1853 +/- ##
=========================================
Coverage 81.03% 81.04%
- Complexity 2411 2418 +7
=========================================
Files 233 233
Lines 7347 7349 +2
Branches 399 398 -1
=========================================
+ Hits 5954 5956 +2
Misses 1148 1148
Partials 245 245 ☔ View full report in Codecov by Sentry. |
bitwiseman
requested changes
Jun 16, 2024
Member
bitwiseman
left a comment
bitwiseman
left a comment
There was a problem hiding this comment.
Needs test to cover the new functionality. Otherwise looks good.
Contributor
Author
|
|
bitwiseman
approved these changes
Jun 19, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: After upgrading GitHub Enterprise Server (GHES) from version 3.9 to 3.11 and 3.12, we encountered issues with failing API calls during Git search (and other)operations. Further investigation revealed that the newer GHES versions consume more CPU resources, leading to secondary rate limits being reached more frequently. Below is a quote from GitHub Enterprise Support:
"Customers planning to upgrade to GitHub Enterprise Server (GHES) version 3.11 or higher should be aware of the potential for increased CPU utilization. If a system's peak CPU usage is already high, typically above 65-70%, it is strongly advisable to scale up the server resources before upgrading. This is to accommodate the additional load from new features and improvements that are part of the latest releases."
Moreover, we realized that GitHub does not guarantee the presence of the
Retry-Afterheader. However, thegh-limited-byheader is included in the response when the error is due to rate-limitingWe got ourselves unblocked after making this change. I see the issue is reported by another user here as well
#1805
Fix: The intention is not to change any existing functionality. So ended up adding an additional optional check, so that the API client can wait for another minute before retrying the API call if either of the headers are present. With the new change the client will execute this block https://github.com/hub4j/github-api/blob/main/src/main/java/org/kohsuke/github/AbuseLimitHandler.java#L82-L93 which is not happening right now