fix: atomic token-bucket refill with race-condition fixes and jitter - #57
Merged
MaryammAli merged 1 commit intoAug 20, 2026
Merged
Conversation
RUKAYAT-CODER
force-pushed
the
fix/56-atomic-rate-limit-refill
branch
from
August 20, 2026 04:52
07b2ed3 to
2f1378c
Compare
The per-issuer bucket tracked remaining tokens with a best-effort counter that only ever decremented (never refilled), so status()/reset_at drifted and concurrent check() calls could race on the shared counter. Replace it with a lock-free AtomicTokenBucket: token count and last-refill timestamp are packed into one AtomicU64 and mutated via a compare-and-swap retry loop, so refill+consume is a single atomic read-modify-write. Racing callers on the same issuer either succeed on disjoint CAS iterations or retry — no token is lost or double-spent — and elapsed time is folded into the stored timestamp even on rejection so accrued refill is never dropped. Refill is exact integer math on milli-tokens (no float, no clock drift), and reset times are computed deterministically with a per-issuer FNV-1a jitter offset to spread synchronized retries (thundering herd). - rate_limit.rs: AtomicTokenBucket, deterministic_jitter, two-tier limiter now backs tier 2 with the atomic bucket; adds concurrency + 10,000-issuer stress tests proving no token loss, plus jitter/reset determinism tests. - config.rs: add validated PER_ISSUER_RATE_LIMIT_JITTER_SECONDS setting. - metrics.rs: add rate_limit_tokens_refilled_total counter.
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.
Closes #56
Problem
The per-issuer rate limiter tracked remaining tokens with a best-effort counter that only ever decremented and never refilled, so
status()/reset_atdrifted from reality and concurrent.check()calls raced on the shared counter. There was no atomic read-modify-write for the bucket state and no jitter, leaving refills vulnerable to synchronized thundering-herd retries.Change
Replace the counter with a lock-free
AtomicTokenBucket:AtomicU64and mutated with acompare_exchangeretry loop, so every refill+consume is one atomic read-modify-write. Concurrent callers on the same issuer either succeed on disjoint CAS iterations or retry on a lost race — no token is lost or double-spent. Elapsed time is folded into the stored timestamp even when a call is rejected, so accrued refill is never dropped.PER_ISSUER_RATE_LIMIT_JITTER_SECONDS.Files
src/rate_limit.rs—AtomicTokenBucket+deterministic_jitter; the two-tier limiter now backs tier 2 (per-issuer) with the atomic bucket. Adds concurrency and 10,000-issuer stress tests proving no token loss, plus jitter/reset-determinism and refill-after-rejection tests.src/config.rs— new validatedPER_ISSUER_RATE_LIMIT_JITTER_SECONDSsetting (must be a validu64and< ISSUER_RATE_LIMIT_TTL_SECONDS).src/metrics.rs— newrate_limit_tokens_refilled_totalcounter for time-based refills.Acceptance criteria
.check()calls eliminated with no data lossVerification
Ran the repository's CI steps locally:
cargo testpasses (232 tests, including the new atomic/concurrency/stress tests — the 10,000-issuer stress test completes in well under a second) andcargo build --target wasm32-unknown-unknown --releasesucceeds;cargo checkis clean across the lib and binary.