poc: bucket state as consumed tokens instead of remaining tokens #60
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.
Description
Context
Currently, we use Redis to store a count of the remaining tokens in a bucket. This is effective and conceptually simple.
For example, using the following configuration:
The request would return
remaining:2
, the second callremaining:1
and the 3rdremaining:0
. Internally, when a key does not exist we consider this to be a 'full' bucket, so with a value of3
in this example, and then we store the remaining tokens at the key for subsequent calls.The 'drip' functionality represents tokens being added over time to the bucket. Based on the refill rate and the time of the last call, we can say that X tokens would have been added to the token. This means that we add those tokens to the bucket when we calculate the drip.
Problem
When introducing ERL, we must also consider the previous states of the bucket. This is because we need to 'top up' the bucket when ERL is activated.
Consider the following example:
erl_size: 3
, we have to 'top up' the bucket internally or it will stay at 0 and will be considered non-conformant.The issue is: How many tokens should we 'top up'?
In the above example, we should 'top up' by 2. This is because we know the user has just consumed their normal
size
configuration, in this example 1. So, we should provide the user with theerl_size - size
so they can take full advantage of ERL.However, consider that a user has consumed all their
erl_size
requests, and their activation period expires. In the next request, we know that the user has just consumed theirerl_size
configuration, so we should not 'top up' any extra tokens.If we we topped up using the first method, then a user would be able to consume all their
erl_size
requests, and then would be allowed to consumeerl_size + (erl_size - size)
requests directly afterwards. In fact, this is what we currently allow, and has lead to the following spiky patterns in permitted traffic;The underlying issue is as follows: With our current implementation, we cannot 'top up' a bucket because we do not know the previous bucket consumption at the point of rate limiting. All we know if that there are 0 tokens remaining from either an ERL or a normally configured bucket.
Proposed Solution
Somehow, we need to keep this information.
The solution proposed in this PR is to invert the information stored in Redis. Instead of counting remaining tokens, it would count consumed tokens.
This change means a few things;
conformant
is now different for ERL and normal modes.conformant = bucket > 0
conformant = bucket < size
conformant = bucket < erl_size
conceptually, this is not as simple as just counting the number of remaining tokens in a bucket. However, it has the advantage that when a rate limit is exceeded, the information is retained for how many tokens were used to exceed the limit. This is irrelevant when a bucket is a fixed size, but useful for our current purposes of variable sized buckets.
References
Testing
Checklist