-
Notifications
You must be signed in to change notification settings - Fork 295
Merge rate limiting code into the feature/throttling branch #6900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+2,882
−150
Conversation
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
The token bucket library implements the token bucket algorithm, to be used for rate-limiting. This commit implements basic token buckets, which contain tokens that are refilled over time according to their refill parameter, up to a maximum determined by the burst parameter. Tokens can be consumed in a thread-safe way - consuming returns false when there are not enough tokens available, and true when the operation was successful. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Bucket tables map client identifiers to their token buckets, and are the main data structure for rate limiting. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
To be replaced with a proper datamodel. Bucket tables are used for mapping requests to their respective token bucket so that they can be rate limited. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Zero or negative rate limits can cause issues in the behaviour of rate limiting. In particular, zero fill rate leads to a division by zero in time calculations. Rather than account for this, we forbid the creation of token buckets with a bad fill rate by returning None. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Make token bucket type abstract to hide Hashtbl.t Use `replace` rather than `add` for adding a new bucket Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
The current implementation of rate limiting had severe fairness issues. These have been resolved through the addition of a request queue, to which rate limited requests are added. A worker thread sleeps until its associated token bucket has enough tokens to handle the request at the head of the queue, calls it, and sleeps until the next request is ready. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Creating a token bucket fails if the rate limit supplied is 0 or negative - this can lead to unexpected and undesirable behaviour, such as division by 0 or negative token counts. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
The rate limiting can no longer be set from xapi_globs. Instead, the rate limiter is initialised from the database on startup now. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Synchronous requests can be long-running, which can cause issues if they are all processed on the same worker thread. This commit updates the code to process synchronous requests on the original caller thread - the worker thread is now only responsible for signalling on a provided channel to wake up the caller. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
When an async request is rate limited, we confirm receipt immediately but enqueue the actual request, rather than rate limiting the first response too. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Rather than have the same token costs for all calls, we want to rate limit clients based on how expensive the functions they are calling are. I measured the average runtime of most xapi calls, which are stored in a hashtable and are used as the token cost for each call. This does not account for any overheads, and will need to be adjusted - it is merely a starting point. We also use the median as a default. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Make Bucket_table a functor that accepts a Map.OrderedType module parameter, allowing the key type to be customised at instantiation. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Until now, the only way to identify rate limited hosts was through their user agent. This commit adds the option to rate limit by IP address. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
This module was helpfully provided by Christian Lindig. It will be used for adding a cache to rate limiting client lookups. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Until now, we were using a hashtable to index rate limited clients by user agent only. This does not work when indexing by multiple fields, so this commit fixes this by replacing the hashtable with a list that matches both fields. The algorithm used for matching is a linear search with caching. If this turns out to be too slow, then we can use a more complex algorithm - the literature for packet switching mentions tries. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
…d range Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
Contributor
Author
|
Will need to update the versions for all the datamodel fields, but I assume that the version should reflect when these changes get merged into master so I'm leaving it as-is for now. |
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 pull request comprises the rate limiting feature for xapi. Users can now add rate limiters to clients by specifying an IP address, a user-agent, or both, together with a burst size and refill rate for the underlying token bucket.
These changes are accompanied by comprehensive unit tests, a quicktest, and a more involved XenRT test where two clients run stress tests to confirm that rate limiting is performed as expected.