bound request throttle leaking _before_ adding cost of current request

fixes CNVS-39263

otherwise they may never get dinged for the cost of the current request

test plan:
 * have redis configured
 * watch the response headers on several requests
 * the X-Rate-Limit-Remaining should never be exactly equal to the
   high water mark (defaults to 600); it should be slightly below

Change-Id: I89e85f873b405e0bd93a6e89730dd504ca934104
Reviewed-on: https://gerrit.instructure.com/125866
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2017-09-12 22:46:56 -06:00
parent 92ce7b8b88
commit 49759eabc2
2 changed files with 10 additions and 2 deletions

View File

@ -21,10 +21,12 @@ count = tonumber(count or 0)
last_touched = tonumber(last_touched or current_time)
count, last_touched = leak(count, last_touched, current_time, outflow)
count = count + amount
if count > maximum then count = maximum end
if count < 0 then count = 0 end
count = count + amount
if count < 0 then count = 0 end
if count > maximum then count = maximum end
redis.call('HMSET', cache_key, 'count', count, 'last_touched', current_time)
-- reset expiration to 1 hour from now each time we write
redis.call('EXPIRE', cache_key, 3600)

View File

@ -297,6 +297,12 @@ describe 'RequestThrottle' do
expect(@bucket.count).to eq 0.0
expect(@bucket.last_touched).to be_within(0.1).of(75)
end
it "doesn't leak the current request" do
@bucket.redis.hmset(@bucket.cache_key, 'count', 1, 'last_touched', @current_time - 50)
@bucket.increment(5.0, 0, Time.at(@current_time))
expect(@bucket.count).to be_within(0.1).of(5.0)
end
end
describe "#reserve_capacity" do