Request for Consideration: Optimizations to Reduce API Requests Count

[south-mer]

What are we trying to achieve

We want to be able to use Policy Bot without reaching the rate limit in a large org with over 4000 repositories and a peak of 300 PRs opened per hour. In our initial deployment of Policy Bot, the rate limit was reached during every work day.

Having an instance of Policy Bot across over 4000 repositories, the 12,500 (GraphQL) / 15,000 (REST) requests/hour rate limit is easy to reach. Therefore, we would like to reduce the number of requests made per webhook event processed. Even though caches already reduce the number of requests, we found space for optimization in Actors.IsActor and GitHubContext.Commits.

How we are currently achieving the goal

Corresponding code change: mercari#7, mercari#8

In our case, we reduced the rate limit usage by

1. limiting the types of webhook events that are sent to the ones that are actually used (and preventing their use in the configuration in our configuration generator), and
2. modifying the code to make fewer requests to GitHub.

For the code change,

• Optimize actors mercari/policy-bot#7 optimizes by not calling prctx.CollaboratorPermission where not needed. The Actors.Permissions field is used to express the set of users who have one of the permission levels (in the repository) in the list, so if the list is empty we should not need to check the permission of the user.
• [Stacked] Optimize commits mercari/policy-bot#8 optimizes by coalescing the call to fetch commits and the call to fetch comments and reviews together. Even though this means that having more than 50 commits would require fetching more than 1 page, this is uncommon and the reduction in request count by requiring only 1 request to fetch commits, comments and reviews is large. We checked the GraphQL response to ensure that only 1 point of the rate limit is consumed.

[bluekeyes]

I think both of these code optimizations make sense and would be be good upstream contributions - we're always open to optimizations that reduce API usage while keeping functionality.

We actually used to load the commits as part of the paged data, but removed it in #78 when adding retries to work around a problem. We've since moved to a different method for determining push times which removed the need for the retries, but we never integrated commit loading back in to the GraphQL query.

Thanks also for documenting the GraphQL query costs. I tend to default to maximum page sizes to minimize request counts, but it's good to know that logic doesn't apply to GraphQL in the same way it does with the REST API. Using smaller page sizes may actually be better to reduce the query cost if it still returns all of the data in the majority of cases.

Disabling the webhooks you don't use is clever, but I don't think that's something we can officially recommend. Future features may use webhooks in new ways and right now I don't want to commit to mapping webhooks to features as part of the public API.