Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
Ethan-Zhang-ZH commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4341466935 > [@niemne](https://github.com/niemne) Thanks for the detailed proposal! The overall direction looks good and references the right PRs. A few adjustments are needed though: > > 1. **Remove `header_prefix`**: Unlike `limit-count` which sets `X-RateLimit-*` response headers, `limit-req` does not set any rate-limiting response headers currently. So `header_prefix` has no practical effect here. If we want to add response headers for `limit-req` in the future, it should be a separate enhancement. > 2. **Remove `key_type` from rules**: Following the pattern of `limit-conn` and `limit-count` rules implementation (see `apisix/plugins/limit-conn/init.lua:64-98`), the `key` in rules is always resolved as a variable combination via `core.utils.resolve_var`. If no variables are resolved (`n_resolved == 0`), the rule is skipped. There's no need for a `key_type` field in rules. > > The rest of the proposal looks good: > > * `oneOf` variable support for `rate`/`burst` > * `oneOf` schema ensuring legacy and rules modes are mutually exclusive > * Removing `lrucache.plugin_ctx` in favor of per-request limiter creation > > Please update the proposal with the above adjustments and start the implementation. I'd also suggest extracting the core logic into `apisix/plugins/limit-req/init.lua` to keep the directory structure consistent with `limit-conn`. I’d like to propose that we keep key_type and support the constant value in the rules for limit-req. The main use case is: when the same route serves multiple endpoints (e.g. /api/v1/users and /api/v1/orders), we often want different rate limits for each endpoint without defining separate routes. By allowing key_type: "constant" and a static key, each rule can define a fixed, unchanging key that represents a specific interface or group. For example: ``` rules: - key: "users_api" key_type: "constant" rate: 10 burst: 0 _meta: filter: [["uri", "==", "/api/v1/users"]] - key: "orders_api" key_type: "constant" rate: 5 burst: 0 _meta: filter: [["uri", "==", "/api/v1/orders"]] ``` This makes the config intuitive and avoids coupling the rate limit key to dynamic variables when the intention is simply “a limit for that interface”. Without constant, users would have to coerce a variable like $uri or misuse $host even when the limit target is fixed. What do you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
niemne commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4249459678 @Baoyuantop Thank you for the suggestion, I will begin implementation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
Baoyuantop commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4248719841 @niemne Thanks for the detailed proposal! The overall direction looks good and references the right PRs. A few adjustments are needed though: 1. **Remove `header_prefix`**: Unlike `limit-count` which sets `X-RateLimit-*` response headers, `limit-req` does not set any rate-limiting response headers currently. So `header_prefix` has no practical effect here. If we want to add response headers for `limit-req` in the future, it should be a separate enhancement. 2. **Remove `key_type` from rules**: Following the pattern of `limit-conn` and `limit-count` rules implementation (see `apisix/plugins/limit-conn/init.lua:64-98`), the `key` in rules is always resolved as a variable combination via `core.utils.resolve_var`. If no variables are resolved (`n_resolved == 0`), the rule is skipped. There's no need for a `key_type` field in rules. The rest of the proposal looks good: - `oneOf` variable support for `rate`/`burst` - `oneOf` schema ensuring legacy and rules modes are mutually exclusive - Removing `lrucache.plugin_ctx` in favor of per-request limiter creation Please update the proposal with the above adjustments and start the implementation. I'd also suggest extracting the core logic into `apisix/plugins/limit-req/init.lua` to keep the directory structure consistent with `limit-conn`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
niemne commented on issue #13179:
URL: https://github.com/apache/apisix/issues/13179#issuecomment-4245575413
refer: (#12977, #12967, #13000, #13004)
1. Schema Changes:
- `rate` and `burst` will accept both numbers and strings (for variable
expressions like `$http_a`).
- Introduce an optional `rules` array for multi-level limiting. Each rule
contains its own `rate`, `burst`, `key`, `key_type`, and optional
`header_prefix`.
- Use `oneOf` validation to require either the legacy fields (`rate` +
`burst`) or the `rules` array, ensuring strict backward compatibility.
2. Implementation Approach:
- Add a runtime helper to resolve `rate`/`burst` string variables to numbers
via `core.utils.resolve_var`.
- In the `access` phase, normalize the configuration into a unified rule
list: legacy mode wraps single fields into one rule; multi-rule mode iterates
the `rules` array, resolves variables, and filters invalid entries.
- Dynamically create the `resty.limit.req` object per request. The
`lrucache.plugin_ctx` will be removed to avoid stale/colliding state when rates
change dynamically, fully aligning with the recent `limit-count` and
`limit-conn` PRs.
- Execute the leaky bucket algorithm (`lim:incoming`) for each normalized
rule, apply delay/reject logic, and support per-rule `header_prefix` for
differentiated response headers.
3. Backward Compatibility:
- Existing configurations without the `rules` field will continue to work
exactly as before.
- Removing the LRU cache guarantees accurate dynamic rate limiting without
side effects.
4. Example Configurations:
- Variable rate (legacy mode):
```json
{"rate": "$http_a", "burst": 10, "key": "remote_addr", "policy": "local"}
```
- Multi-level rules:
```json
{"rules": [{"rate": 100, "burst": 20, "key": "remote_addr"}, {"rate":
"$http_a", "burst": 50, "key": "http_x_user_id", "header_prefix": "user"}],
"policy": "redis", "rejected_code": 429}
```
@Baoyuantop Please review this proposal. Welcome any feedback or
suggestions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
Baoyuantop commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4241646531 Hi @niemne, please describe your technical solution in this issue before submitting a PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
niemne commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4238091686 @Baoyuantop Hi, I'd like to work on this, could you please assign it to me? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [I] feat: Why doesn't the limit-req plugin support `rules` like limit-conn and ai-rate-limiting? [apisix]
Baoyuantop commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4204312881 Welcome contributions to this feature. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
