XAPI throttling proof of concept

[cplaursen]

Proof of concept for XAPI throttling.
This allows users to specify the user-agent rate limited clients in xapi.conf, which then consume from a token bucket whenever a request is made, and have to wait for it to refill if they exceed its capacity.

[psafont]

It's a bit tough to understand how the datastructure does rate-limitting. I had to find documentation online to undestand it. As such, I would appreciate a more in-depth explanation at the header of the mli file. For example, it would be good to understand when is the "refill" timestamp changed, and explain that some of the methods are only meant to be used for testing.

[psafont]

fill_rate has an implicit unit (Hz), I think that it can be worth representing the fill rate as the amount of time it takes the bucket from being empty to becoming full. This is a timespan, and we can use a known datatype with explicit unit here: Mtime.span.

How some operations would be changed:

let peek_with_delta time_delta tb =
  let fill_time = Mtime.Span.to_float_ns tb.fill_time in
  let time_delta = Mtime.Span.to_float_ns time_delta in
  min tb.burst_size (tb.tokens +. (time_delta /. fill_time)

let delay_until_available_with_delta delta tb amount =
  let current_tokens = peek_with_delta delta tb in
  let required_tokens = max 0. (amount -. current_tokens) in
  required_tokens *. (Mtime.Span.to_float_ns tb.fill_time)
  |> Float.to_int64
  |> Mtime.Span.of_unit64_ns 

[cplaursen]

I can see the appeal of adding units to the fill_rate but I'd rather keep the burst size decoupled from fill rate, and I think it's more intuitive as tokens per second than seconds per token.

[lindig]

How can this fail, i.e., return None?

[edwintorok]

I think in some extreme cases, if you have an arithmetic overflow. Although that'll be like an Mtime span of ~584 years.

[psafont]

Is there any reason this cannot be a counter? They return the time difference directly: https://ocaml.org/p/mtime/latest/doc/mtime.clock/Mtime_clock/index.html#counters

This would mean that the peek functions would turn into:

let peek_with_delta time_delta tb =
  let time_delta_seconds = Mtime.Span.to_float_ns time_delta *. 1e-9 in
  min tb.burst_size (tb.tokens +. (time_delta_seconds *. tb.fill_rate))

let peek tb = peek_with_delta (Mtime_clock.count tb.last_refill) tb

[cplaursen]

Keeping a counter does simplify things a bit, but if I understand correctly it forces us to make two system time calls when consuming tokens - one to obtain the difference from the counter, and another to produce a new counter. It probably won't make much of a difference, I can try profiling.

[cplaursen]

Added some documentation to the token bucket module to explain the rate limit application.

[lindig]

How can this fail, i.e., return None?

[lindig]

Could use a simpler name like bucket, consumer or similar.

[psafont]

I don't think the idl should mention tokens or buckets at all, instead I would try to communicate the meaning of the parameters in a way that allows users to make a mental model of how rate limiting works:

Suggested change

	"Amount of tokens that can be consumed in one burst"
	"Amount of RPC calls that the client can do in burst"

[cplaursen]

I agree, we shouldn't talk about token buckets and I'll change that. The plan is to assign higher token costs to more expensive calls, e.g. VM create, so we can't simplify to the level of RPC calls, but I'll figure out how to document this for users.

[psafont]

Suggested change

	"Tokens added to token bucket per second" ~ignore_foreign_key:true
	"Calls per second afforded to the client" ~ignore_foreign_key:true

[minglumlu]

Would it be possible to let the caller know how long it should delay before moving ahead?
For example:

consume t amount |> Option.iter Thread.delay ;
handle ()

The schedules of different threads can be partially sorted by the delays returned to them.