Add rate limiting feature to xapi#7114
Conversation
The rate limit library is currently called xapi_rate_limit, which will clash with the xapi file associated to the rate limit datamodel. Renaming to rate_limit_lib. Signed-off-by: Christian Pardillo Laursen <christian.pardillolaursen@citrix.com>
As per the rate_limit design, this commit adds two new datamodels to xapi: - Caller: tracks usage statistics on clients calling into xapi. Clients are currently identified by their ip address and HTTP user agent - Rate_limit: lets xapi admins apply rate limits to groups of callers. Rate limiters allow a fixed number of "tokens" (number correlated with call cost) to be used per second; if the token count is exceeded, the calls get throttled. 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>
cplaursen
force-pushed
the
feature/throttling2
branch
from
001cf30 to
198af98
Compare
robhoes
left a comment
robhoes
left a comment
There was a problem hiding this comment.
Some initial feedback on the Caller definition.
| (** Total order on patterns: fewer wildcards first, then lexicographic | ||
| by patterns. *) | ||
|
|
||
| val is_all_wildcard : pattern_key -> bool |
There was a problem hiding this comment.
This change and the addition of to_list are in the rename commit, but strictly speaking shouldn't be.
cplaursen
force-pushed
the
feature/throttling2
branch
from
8d31c94 to
4b9ca96
Compare
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>
cplaursen
force-pushed
the
feature/throttling2
branch
from
2e50449 to
5fdf69c
Compare
| ; flags= [] | ||
| } | ||
| ) | ||
| ; ( "rate-limit-set-burst-size" |
There was a problem hiding this comment.
You don't need this, because you have a set function in Records (alongside the get). This means that you get xe rate-limit-param-get and xe rate-limit-param-set automatically. Same for fill-rate.
|
|
||
| let get_origin ctx = string_of_origin ctx.origin | ||
|
|
||
| let is_internal_origin ctx = ctx.origin = Internal |
There was a problem hiding this comment.
I think that Internal here just means that the call did not come in over the network. This does not include calls from other hosts in a pool.
robhoes
left a comment
robhoes
left a comment
There was a problem hiding this comment.
Some minor comments, but overall I think this looks good.
| Hashtbl.of_seq | ||
| (List.to_seq | ||
| [ | ||
| ("VDI.pool_migrate", 2500.) |
There was a problem hiding this comment.
It would be better to store these in a file that is read on startup. That would make it possible to tweak the values and add other calls without having to recompile xapi.
| ) | ||
|
|
||
| let insert_entry ~caller_ref ~pattern_key ~rate_limit_ref = | ||
| let uuid = Uuidx.(to_string (make () : [`Caller] t)) in |
There was a problem hiding this comment.
Why is a new uuid created here rather than using the one from the DB record?
| maybe_autocreate ~task_create ~user_agent ~client_ip ~existing:matches ; | ||
| match bucket with | ||
| | Some rl -> | ||
| Rate_limit.submit_async rl ~callback amount |
There was a problem hiding this comment.
With the exception of this line, the function looks identical to submit_sync, so there seems to be scope for reducing duplication.
| task_create (fun __context -> | ||
| try | ||
| let caller_ref = | ||
| create ~__context ~name_label:"" ~name_description:"" |
There was a problem hiding this comment.
I think we can do better than empty strings for name_label and name_description here? At the very least we could map the user_agent to a "friendly name", where the mapping can be stored in a file.
2 participants
This commit implements the changes laid out in the rate_limit design document.
In particular, we add two datamodels:
These changes include intercepting RPC calls as they arrive for logging and rate limiting purposes. The feature is currently behind a feature flag - to activate, write rate_limit=true into xapi.conf.