GCRA Rate Limiting implementation and initial rate-limited logging#164
Draft
fisherdarling wants to merge 5 commits intomainfrom
Draft
GCRA Rate Limiting implementation and initial rate-limited logging#164fisherdarling wants to merge 5 commits intomainfrom
fisherdarling wants to merge 5 commits intomainfrom
Conversation
Goal is a few-ns overhead RateLimiter for future use in log ratelimiting.
This adds the ability to rate-limit individual log invocations. It's implemented with a prefix which can be added to any of the various logs. E.g. to allow for one log every second, with a burst of two: ``` error!(ratelimit(rate=1.0, burst=2), "something bad happened!"); ```
Previously the `iters` was divided amongst threads, meaning when the number of threads increased, the amount of work per-thread decreased. This had the fun effect of causing our high contention benchmark to report ~3ns per iter... Now every thread does `iters` amount of work and we see expected (?) performance under contention. When we're below the number of CPUs on my device, I see ~30ns per-iter which is what we get for the uncontended benchmark. When we use 16 threads (more than the number of CPUs), the threads start competing for CPU time and we jump to ~45ns per-iter.
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
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.
This PR allows users to rate-limit individual log invocations and adds a GCRA-based
RateLimiter.The
RateLimitersupports a maximum rate along with a given burst. If the rate is1.0/s and the burst is3, then the limiter will allow up a burst of up to 3 requests before limiting. After being limited, a second must past before another request will be allowed.RateLimiters are thread-safe and placed instaticcontexts. They are cheap to create, 32 bytes or 16 bytes with a shared config, and cheap to evaluate. Checking aRateLimiteris a timestamp read + singlefetch_addCAS loop. We also introduce aratelimit!macro which can be used to easily rate-limit a given block of code.Logging macro invocations now support a new prefix,
ratelimit(burst=<expr>, rate=<expr>)which will rate-limit the log line with the given config.For example, to log an error once per minute with a burst of 2:
Rate-limited logs do not count against the global ratelimit.
Open Questions/Notes
Cow<'a, RateLimiterConfig>. Then I think we can provide aRateLimiter::with_config(rate, burst) -> RateLimiter<'static>API. Though I'm pretty sure this increases the cost of a singleRateLimiter(likely fine).RateLimiterhere, before remembering we use thegovernorlibrary for global log rate limits.governoris a nice library, we should explore makingRateLimiter::directconst-compatible. On the other hand, we don't need all of governor's features and it's nice to replace a dependency with a single file.ratelimit!in a more public sense? Rather than putting it behind a "private" mod or something a bit more hidden?Future Additions
I'd like foundations to provide per-level rate limits defined in the settings. Then log invocations will automatically rate limit. Also, we could extend the logging syntax to take in a static limiter shared across multiple fns, perhaps allowing users to write something like
ratelimit(limiter=<ident>).