fix(ets): return the real time-to-next-token on token bucket deny - #198
Merged
epinault merged 1 commit intoSep 2, 2026
Merged
Conversation
TokenBucket.hit/5 answered a flat {:deny, 1000} regardless of how far the
bucket actually was from paying the cost. At 55 tokens/sec a caller short one
token is told to wait ~53x longer than the limiter requires, which puts the
throughput loss back in the caller's retry loop after ExHammer#192 took it out of the
bucket.
fix_window, fix_window_per_key and sliding_window all already return a real
computed wait, and the @callback types the value as a timeout -- the two
bucket algorithms were the inconsistent ones. The flat 1000 was defensible
under whole-second refill because nothing finer was expressible; it is not
once the clock is in milliseconds.
Uses integer ceiling division so the answer is exact at every magnitude and
can never round down into a wait that is still too short. A sweep over eight
refill rates, three capacities and several cost/deficit combinations pins
that sleeping the advertised wait always succeeds.
Also updates guides/Tutorial.md. Both retry-after examples truncated the wait
with div(retry_after, 1000), which was always 1 while the value was hardcoded
but becomes 0 for any sub-second wait -- and retry-after: 0 tells the client
to retry immediately. They now round up and floor at 1.
Every existing assertion in the suite still passes unchanged, including the
literal {:deny, 1000} expectation -- at refill_rate 1 the real wait is
genuinely 1000ms.
epinault
added a commit
that referenced
this pull request
Sep 2, 2026
- Bump version 7.4.1 -> 7.5.0 in mix.exs - Promote Unreleased changelog section to 7.5.0 (#194, #197, #198) Minor bump: the release adds the mix hammer.install Igniter task and carries two TokenBucket ETS fixes, one of which changes the deny wait from a flat 1000ms to the real time-to-next-token. Claude-Session: https://claude.ai/code/session_011dH6kRtZqUkVpoGo79CKYG Co-authored-by: epinault <dev@pinault-family.us>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Hammer.ETS.TokenBucket.hit/5answers a hardcoded{:deny, 1000}no matter how far the bucket actually is from paying the cost. Atrefill_rate: 55a caller short one token is told to wait ~53x longer than the limiter requires.This is the same throughput loss #192 fixed, relocated: the bucket no longer throttles below its configured rate, but the wait it advertises makes the caller's own retry loop do it instead.
fix_window,fix_window_per_keyandsliding_windowall already return a real computed wait, and the@callbacktypes this value as atimeout. The two bucket algorithms are the inconsistent ones. A flat 1000 was defensible under whole-second refill because nothing finer was expressible — that stopped being true in 7.4.1.The change
Integer ceiling division rather than
ceil/1on a float, so the answer is exact at every magnitude and can never round down into a wait that is still too short.Sleeping the advertised wait is always sufficient. The refill path truncates down and this rounds up, so it is worth stating why they cannot conspire: the returned value is at least
deficit * 1000 / refill_ratems, which accrues at leastdeficitwhole tokens on top of whatevertruncalready credited. A test sweeps eight refill rates, three capacities and several cost/deficit combinations to pin it.This is a behaviour change — two things it affects
1. Your tutorial, which this PR also fixes. Both
retry-afterexamples inguides/Tutorial.mddodiv(retry_after, 1000). That is always1while the value is hardcoded, but becomes0for any sub-second wait — andretry-after: 0tells the client to retry immediately. Left alone, this change would turn the documented pattern into an invitation to hammer the server. Both examples now round up and floor at 1.2. Denied callers wake more often. A caller that sleeps on this value now rechecks in ~19ms instead of 1000ms. Nobody is served faster — the limit is the limit — but a crowd of simultaneously-denied callers costs proportionally more ETS traffic while they wait.
If you would rather existing users not see the change, I am happy to follow up with a
min_retry_after_msoption; a floor is one number and needs no plumbing, whereas an on/off flag does not fit —hit/5receives no config. Say the word and I will send it.Not addressed (pre-existing, happy to file separately)
cost > capacitycan never be satisfied, so the caller loops forever being told to wait. That is true of the flat 1000 today too — this change does not introduce it, but it does make the returned number look more authoritative than it is.Test plan
mix test— 145 tests, 0 failuresassert {:deny, 1000}intoken_bucket_test.exs— atrefill_rate: 1the real wait is genuinely 1000msIndependent of #197; happy to rebase whichever lands second.