tps/tph: per-second / per-hour token-rate counters need a different counter strategy than FixedWindowCounter + post-deduct add
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 157
- Forks
- 32
- Avg merge
- 1h 25m
- Merged PRs (30d)
- 145
Description
Context
Follow-up to PR fixing api7/AISIX-Cloud#426. That PR adds per-second (`rps`) and per-hour (`rph`) request counters to `RateLimit` so a standalone `rate_limit_policies` row with `{window: second, max_requests: N}` actually enforces 1-second windows (was incorrectly upscaled to `rpm = N*60`, allowing 60× bursts).
Tokens at the same windows (`tps`/`tph`) are intentionally NOT shipped in that PR. This issue tracks the design work.
Why tokens differ from requests
Token counts arrive post-deduct (after the upstream LLM response completes with its terminal `usage` frame). The existing `tpm` counter uses `FixedWindowCounter::add(now, delta)` which calls `roll_if_stale` before adding. At a window boundary, this silently grants a freebie:
- Request fires at `t=0.9s`, runs 200ms, terminal usage arrives at `t=1.1s`
- `s.tps.add(t=1.1, 1000)` → `roll_if_stale` rolls the stale 1s window, sets `count=1000` in the new window
- Previous window's actual 1000-token consumption is dropped on the floor
- Pre-commit at `t=1.2` sees `count=1000` which is correct for the current window, but the previous window's overshoot was effectively free
For `tpm` (60s window) this is sub-optimal but bounded: response time / window = ~30s/60s = 50% worst-case freebie ratio.
For `tps` (1s window) this is exploitable: response time / window = ~5s/1s = 500% — a customer can routinely 5× their declared per-second token budget by timing requests to land on window boundaries.
For `tph` (3600s window) the same race applies but ratio drops to ~5s/3600s = 0.14% — likely acceptable but worth confirming.
Proposed designs (to investigate)
- Sliding window — carry overflow from previous window so a tail-arriving token batch is charged against the bucket it actually consumed
- Pre-deduct estimate + reconcile — reserve estimated max_tokens at pre-commit, refund unused after usage frame
- Track last-N windows + decay — generalized sliding decay
- Defer in product — make per-second / per-hour tokens not a supported window combination; reject at `validate_rate_limit_policy` schema layer
Need to compare against reference implementations (LiteLLM, Portkey) — see what they do for sub-minute token budgets, if anything.
Scope
This issue blocks the symmetry of:
- `crates/aisix-proxy/src/quota.rs::policy_to_rate_limit` for the `max_tokens` arm under `window=second` and `window=hour`
- `crates/aisix-ratelimit/src/limiter.rs` token-counter wiring at sub-minute scales
Until this lands, `policy_to_rate_limit` should silently ignore `max_tokens` on sub-minute windows (with a tracing log or schema-side rejection). The cp-api admission already accepts `max_tokens` on any window per `crates/aisix-core/src/models/schema.rs:531`; that's a contract gap surfaced by this work.
Filed by @claude (AI assistant) during the #426 fix work. cc @moonming for triage.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with crates/aisix-proxy/src/quota.rs::policy_to_rate_limit, crates/aisix-ratelimit/src/limiter.rs, and crates/aisix-core/src/models/schema.rs:531; compare the proposed counter strategies with LiteLLM and Portkey. Done means selecting and documenting a safe strategy for tps/tph, resolving the max_tokens contract gap, and defining the resulting policy behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend, backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100