Description
Two related bugs in how the code picks a GitHub App, found while looking into rate-limit stalls when more than one GitHub App is configured:
- No switching to another App when a rate limit hits. The code already picks the App with the most budget left before a request starts, but once it picks one, it keeps using that same App for the whole request. If that App hits a rate limit partway through, the code just waits out the
retry-after time, even if another configured App still has budget. This gets worse when several invocations run at the same time and pick the same "best" App, because the rate-limit info is only updated after a request finishes or fails.
- Retry-path calls always counted against App 0.
getOctokit() picks a GitHub App internally, but only returns the Octokit client, not which App it picked. The retry Lambda calls isJobQueued() without saying which App was used, so metricGitHubAppRateLimit() always counts the call against App 0, no matter which App actually made the call. This breaks the per-App rate-limit numbers that the App-picking logic itself depends on, for every call going through the retry path.
Impact
With 2 or more GitHub Apps configured, adding more Apps to spread out rate-limit load only lowers the chance of hitting an exhausted App — it doesn't remove the problem, because there's no code that switches Apps mid-request. And any call going through the retry Lambda messes up the per-App rate-limit numbers that the App-picking logic depends on.
Proposed fix
- When a rate limit hits, switch right away to another configured App that has room left, instead of waiting out the retry time (with a limit, so it stops once every configured App has been tried).
- Pass the actually-picked App's index through the retry path, so the rate-limit numbers get counted against the right App.
See PR (to follow) for the implementation.
Description
Two related bugs in how the code picks a GitHub App, found while looking into rate-limit stalls when more than one GitHub App is configured:
retry-aftertime, even if another configured App still has budget. This gets worse when several invocations run at the same time and pick the same "best" App, because the rate-limit info is only updated after a request finishes or fails.getOctokit()picks a GitHub App internally, but only returns the Octokit client, not which App it picked. The retry Lambda callsisJobQueued()without saying which App was used, sometricGitHubAppRateLimit()always counts the call against App 0, no matter which App actually made the call. This breaks the per-App rate-limit numbers that the App-picking logic itself depends on, for every call going through the retry path.Impact
With 2 or more GitHub Apps configured, adding more Apps to spread out rate-limit load only lowers the chance of hitting an exhausted App — it doesn't remove the problem, because there's no code that switches Apps mid-request. And any call going through the retry Lambda messes up the per-App rate-limit numbers that the App-picking logic depends on.
Proposed fix
See PR (to follow) for the implementation.