RFR: 1731: Improve RestRequestCache rate limiter

Zhao Song zsong at openjdk.org
Wed Jan 4 00:09:42 UTC 2023

On Tue, 3 Jan 2023 18:15:36 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> In the RestRequestCache, we are trying to limit non GET calls for a specific host and user to one call per second (and all calls, including GET, for a specific host and user, need to be serialized). This throttling is based on recommendations from GitHub (https://docs.github.com/en/rest/guides/best-practices-for-integrators?apiVersion=2022-11-28#dealing-with-abuse-rate-limits). The current implementation isn't really doing that however. Calls are serialized, but depending on timing, we can definitely send more than one non GET call per second, at least in bursts.
> This patch tries to rework this, using two locks. During high contention, it's possible that this new implementation will be less favorable to non GET calls than the current implementation, but I'm pretty sure we will at least adhere to the recommendation of performing at most one non-GET call per second. See bug for breakdown of locking order.

It really took me a while to understand how it was able to send two non-GET requests in one second, but once I understood it, I checked your revision and I believe that the clever two-lock solution will solve the problem perfectly.


Marked as reviewed by zsong (Committer).

PR: https://git.openjdk.org/skara/pull/1453

More information about the skara-dev mailing list