docs(proposal): add implementation to token rate-limiting - #658
Conversation
bef5abd to
12b0800
Compare
shaneutt
changed the title
shaneutt
changed the title
c84806d to
7115e45
Compare
7115e45 to
253b698
Compare
Co-authored-by: Jamie Land <hokie10@gmail.com> Signed-off-by: Shane Utt <shaneutt@linux.com>
1e26820 to
394e469
Compare
| by default, treats all requests under a `default` | ||
| rule). |
There was a problem hiding this comment.
I'd suggest avoiding having "a default". Most generally buckets "stack", e.g. with hourly & daily counters when any hits the threshold, the call is considered "above budget". While it seems easy enough to say "if there are no bucket rule matching for this request, then delegate to this other set of buckets", it actually comes at a very high cost in terms of complexity. otoh, if the user is to specify "how to qualify" the default, that complexity, while being pushed on the user, is gone and makes for a system that much easier to reason about and less prone to bugs & weird corner cases.
| - **[M5]** Hard deny with 429 when a budget is | ||
| exhausted, with standard rate limit response | ||
| headers (`Retry-After`, `X-RateLimit-*`). | ||
| - **[M3]** Configurable estimation: allow opeartor to |
| tracked separately with configurable weights so | ||
| quotas reflect real cost differences. | ||
| - **[M5]** Flexible bucket keys: quotas keyed by | ||
| request infromation. Headers, model identity, or |
There was a problem hiding this comment.
information (not infromation)
| - **[M7]** Soft limits: usage tiers that modify request | ||
| including estimated cost, actual cost, and remaining | ||
| budget. The ability to log tokenomic results | ||
| distinctly for accounting. |
There was a problem hiding this comment.
I'd suggest not to compound "accounting" & TRLP. This makes for very brittle systems, mostly in the face of failure. It'd be preferable to have that (configurable?) auditing trail happen "outside this system", while it probably could be reusing some of the information from TRLP (e.g. the "bucket keying logic").
Now it can be part of this problem definition, but it would be desirable to model that separately in the implementation.
| independent budgets. (TBD - this one may need to be | ||
| teased out more - possibly using CEL note there is an | ||
| upstream effor around this found | ||
| [here](https://github.com/kubernetes-sigs/wg-ai-gateway/pull/57)) |
There was a problem hiding this comment.
This problem is interesting. In my experience, a key is "calculated" (vs. being a compound key, where all individual bits remains usable by themselves) - which is another reason to not compound the rate limiting with the auditing (see below).
| daily budget). Unmatched requests fall to a `default` | ||
| rule. |
There was a problem hiding this comment.
Repeating myself, but I'd suggest leaving that "fall to a default rule" out, unless there is an actual real use case for it.
| inject: | ||
| headers: | ||
| X-Token-Day-Tier: warning | ||
| - capacity: 1_000_000 | ||
| action: deny |
There was a problem hiding this comment.
How is inject, not an action here? nitpick, but from a modeling/normalization perspective I think we can do better here by having a "tighter API surface", which then would probably also be more open to extensions in the future, wdyt?
| 3. **Reconciliation** - Actual provider-reported token | ||
| usage replaces the estimate. The difference between | ||
| estimated and actual cost is logged, and usage past | ||
| tier capacities is still tracked for observability | ||
| when no `deny` tier applies. (In future iterations, | ||
| we may refund the estimate-vs-actual difference back | ||
| to the budget; for MVP, budgets consume at the | ||
| estimated rate.) | ||
|
|
||
| 4. **Cleanup** - If a request is lost (timeout, | ||
| connection reset, upstream failure), release the | ||
| reservation after a configurable hold period. |
There was a problem hiding this comment.
If we want, which I think we do and certainly recommend so, to be able to deal with all failure scenarios, this flow will probably need to be somewhat different:
- Reservations need to be first class citizens in the
admissionphase and have their own timeout Reconciliationneeds to deal with "applying the actual diff" using the reservation, where the "refund" is actually the easiest problem, overshoots are more annoying - as users may want different outcomes.- There is a note to be added here possibly, about how a reservation behaves when it "overlaps two (or more buckets)". Not a huge problem, but probably clearly explaining what happens would probably be good.
Cleanuphere I think is what the proxy does... but the proxy can only do it if it's still around (see first bullet point above)
| estimation: | ||
| strategy: input_plus_max_tokens | ||
| multiplier: 1.2 |
There was a problem hiding this comment.
Unless there is a reason not to support an expression here, I'd stay away from these "explicit YAML" strategies. Now, if on the other hand, the goal is to actually not use an expression then... maybe ¯_(ツ)_/¯
praxis-bot
left a comment
praxis-bot
left a comment
There was a problem hiding this comment.
6 comments: 2 Large, 4 Medium.
The rule-based budgeting model and tier system are well-structured. The estimation strategy table and token-type capture/weighting design address real operator needs. The main concerns are: a contradiction in the reconciliation lifecycle, an MVP item (M8) with no corresponding design, unclear window semantics, and scope ambiguity around soft limits vs. the tier model.
| response-only accounting. | ||
|
|
||
| 2. **Inference** - The request is forwarded upstream. | ||
| The provider performs inference and returns token |
There was a problem hiding this comment.
[Large] The reconciliation phase says "actual provider-reported token usage replaces the estimate" but then concludes "for MVP, budgets consume at the estimated rate." These contradict each other. If actual usage replaces the estimate, the budget reflects actual cost. If budgets consume at the estimated rate, actual usage does not replace anything -- it is only logged.
Clarify which behavior MVP implements: (a) replace the estimate with actual cost in the budget (true reconciliation), or (b) keep the budget at the estimated rate and only log the actual-vs-estimated delta. Then reword this paragraph to be internally consistent.
| including estimated cost, actual cost, and remaining | ||
| budget. The ability to log tokenomic results | ||
| distinctly for accounting. | ||
| - **[M8]** Multi-environment observability: aggregate |
There was a problem hiding this comment.
[Large] M8 (multi-environment observability) is listed as MVP must-have, but the Design section has no subsection addressing it. Cross-environment aggregation is architecturally significant -- it requires either shared state (e.g. an external store that all proxy instances read/write budgets against) or a centralized aggregation service. Without at least a sketch of the approach, this goal is unimplementable.
Either add a design subsection for multi-environment aggregation, or move M8 to "Should have" and note that MVP budgets are per-instance.
| rejected with 429. Otherwise the request continues and | ||
| inject headers from all currently exceeded soft tiers | ||
| are unioned onto the request. If two budgets inject the | ||
| same header name, the last budget in config order wins; |
There was a problem hiding this comment.
[Medium] The tier model described here -- multiple capacity thresholds with inject.headers for graduated signals before hard deny -- is exactly what S1 (Soft limits) describes: "usage tiers that modify request headers instead of rejecting." But S1 is "Should have," not MVP. This creates ambiguity: is the multi-tier injection model part of MVP, or is the MVP tier design just a single capacity with hard deny?
Clarify the MVP boundary. Either move S1 into "Must have" (since the design already assumes it), or add a note that the MVP tier model supports only action: deny and the inject.headers / action: continue mechanism is S1 scope.
| subscription: team-alpha-key | ||
| x-praxis-ai-model: gpt-4o | ||
| # Dynamic matchers (post-MVP / CEL) — optional | ||
| - cel: > |
There was a problem hiding this comment.
[Medium] The config uses window: 1h and window: 24h but the proposal never defines the window type. Is this a sliding window (continuously rolling), a tumbling/fixed window (resets on the hour/day), or calendar-aligned? The non-goals list "Token Rate Limiting over a static window" as future work, which implies the MVP uses sliding windows, but this is never stated.
Add a sentence to the Token Budgeting section defining the window semantics (e.g. "Windows are sliding: a 1h window tracks usage in the most recent 60 minutes from the current instant").
| length). | ||
|
|
||
| Built-in strategies (MVP starting point): | ||
|
|
There was a problem hiding this comment.
[Medium] Three of the four estimation strategies depend on max_tokens, but not all LLM requests include this field (providers apply high defaults when it is omitted). The proposal does not specify what happens when max_tokens is absent.
Add a fallback behavior for each strategy when max_tokens is missing -- for example: reject the request, use a configurable default, or fall back to a different strategy.
| - **[M5]** Hard deny with 429 when a budget is | ||
| exhausted, with standard rate limit response | ||
| headers (`Retry-After`, `X-RateLimit-*`). | ||
| - **[M3]** Configurable estimation: allow opeartor to |
There was a problem hiding this comment.
[Medium] Several typos throughout the proposal:
- Line 59: "opeartor" -> "operator"
- Line 68: "infromation" -> "information"
- Line 72: "effor" -> "effort" (also has a double space before "this")
- Line 142: "thefore" -> "therefore"
- Line 146: "requrest" -> "request", "therfore" -> "therefore"
Fix these typos.
praxis-bot
left a comment
praxis-bot
left a comment
There was a problem hiding this comment.
4 comments: 2 Medium, 2 Small.
Following up on the new commit. The How? section with the full design is a strong addition — the rule/budget/tier structure, estimation strategies table, and token-type capture/accounting separation are well thought out. A few new items below.
| forced to report usage in the response body or | ||
| headers. By the time actual counts are known, the | ||
| tokens have been consumed. Admission decisions | ||
| however must happen at requrest time. And therfore |
There was a problem hiding this comment.
[Medium] Three new typos in the rewritten Motivation paragraph:
- Line 142: "thefore" → "therefore"
- Line 146: "requrest" → "request"
- Line 146: "therfore" → "therefore"
Also, the two sentences starting with "And" (lines 142, 146) read as fragments. Consider joining them to the preceding sentence:
Providers are unaware of output token counts before receiving the output and are therefore forced to report usage in the response body or headers. … Admission decisions however must happen at request time, and therefore we must rely on token count estimates …
| rule** (not per `token_budget`). Hourly and daily | ||
| budgets on the same rule share one cost model. | ||
| Strategies operate on request metadata available before | ||
| forwarding (e.g. `max_tokens`, model identity, content |
There was a problem hiding this comment.
[Medium] Estimation is configured per-rule, but the proposal doesn't specify behavior when a rule omits the estimation key. The default catch-all rule example (line 290) has no estimation config, and neither does the batch example (line 468). Is there a filter-level default strategy? Is omission an error? Does it skip admission estimation entirely (response-only accounting)?
Line 327 hints that estimation at admission "can be optionally disabled in favor of response-only accounting" but the mechanism for that opt-out isn't defined. Clarifying the default/fallback behavior would close this gap.
| action: deny | ||
| # Optional nested batch rule (same parent identity, | ||
| # different budgets). Exact nesting TBD. | ||
| batch: |
There was a problem hiding this comment.
[Small] The Batch Workloads section presents a concrete nested batch: config key with a detailed example, but the Open Questions section (line 552–568) says: "Open shape questions remain: separate rules vs nested rules under a parent, both, or something else entirely." Consider marking this example as illustrative/draft (e.g. "one possible shape") so readers don't treat the nested batch: key as settled design.
| connection reset, upstream 5xx)? How long should a | ||
| reservation be held before it is considered lost? | ||
|
|
||
| [#210]: https://github.com/praxis-proxy/ai/issues/210 |
There was a problem hiding this comment.
[Small] [#210] is defined here but never referenced in the document. Either link it where relevant or remove the dead reference.
5 participants
This adds a high-level overview of the strategy for implementing Token Rate Limiting in Praxis.