search

brunoV / throttler

Control the throughput of function calls and core.async channels
286 stars 27 forks source link

Rate calculation is wrong when tens digit and unit digit aren't 0 (under 'second' unit) #11

Open visibletrap opened 5 years ago

visibletrap commented 5 years ago

For example, for (throttle-fn f 140 :second), I get 100 req/s and for (throttle-fn f 150 :second), I get 200 req/s. I believe it's related to sleep-time and token-value calculation in chan-throttler*

The relationship between input req/s and actual req/s can be demonstrated with this function.

(defn- round [n] (Math/round (double n)))

(defn real-rate-s [rate-s]
  (let [rate-ms (/ rate-s 1000)
        min-sleep-time 10
        sleep-time (max (/ rate-ms) 10)
        token-value (round (* sleep-time rate-ms))]
    (* (/ 1000 sleep-time) token-value)))
(map (juxt identity real-rate-s) [40 50 140 150 240 250 940 950 1040 1050 1400 1500])
=> ([40 40N]
 [50 50N]
 [140 100]
 [150 200]
 [240 200]
 [250 300]
 [940 900]
 [950 1000]
 [1040 1000]
 [1050 1100]
 [1400 1400]
 [1500 1500])

As you can see, the inaccurate happens when tens digit and unit digit aren't 0.

Input 150 but get 200 is about 33% higher which is significant. I understand the limitation with min-sleep-time and token-value can't be decimal so I'm not sure what's a solution for this. Maybe just only a warning messing in README?

liath commented 5 years ago

I've been looking into this and I suspect the 10ms limit is from TIMEOUT_RESOLUTION_MS in core.async/timeout's implementation. There's a write-up on it here but basically timeout returns an existing channel for calls within 10ms of each other so they don't have to track as many timeouts.

We could probably re-implement core.async/timeout here and either drop the bucketing or make it configurable. I'm not sure that's the correct solution but I don't have any other reasonable ideas.