-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add SmartOrder annotation for intelligent RateLimit prioritization #1016
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This feature introduces a new SmartOrder annotation that enables intelligent prioritization of multiple RateLimit annotations on the same method. Key changes: - Added SmartOrder annotation class for marking methods that need smart ordering - Modified RateLimitAspect to support priority-based RateLimit processing - RateLimits are now processed in order of strictness (maxAttempts/decay ratio) - More restrictive rate limits are evaluated first for better efficiency The SmartOrder annotation allows developers to optimize rate limiting behavior by ensuring stricter limits are checked before more lenient ones.
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Walkthrough在RateLimit组件中引入了AutoSort功能,通过SplPriorityQueue根据maxAttempts/decay值对多个RateLimit注解进行优先级排序,使用优先级队列改进了注解处理的顺序逻辑,同时补充了相关文档和示例说明。 Changes
Sequence DiagramsequenceDiagram
participant Client
participant RateLimitAspect
participant Queue as SplPriorityQueue
participant RateLimit as Rate Limit Logic
Client->>RateLimitAspect: 调用带RateLimit的方法
alt AutoSort enabled
RateLimitAspect->>RateLimitAspect: 检测AutoSort元数据
RateLimitAspect->>Queue: 遍历所有RateLimit注解<br/>计算优先级(0 - maxAttempts/decay)
Queue->>Queue: 按优先级自动排序
loop 按优先级顺序处理
Queue->>RateLimit: 出队下一个注解
RateLimit->>RateLimit: 执行速率限制检查
alt 超出限制
RateLimit-->>Client: 抛出RateLimitException
else 未超出限制
Note over RateLimit: 继续处理下一个注解
end
end
else AutoSort disabled
RateLimitAspect->>RateLimit: 按原始顺序顺序处理注解<br/>(优先级默认为0)
end
RateLimit-->>Client: 返回执行结果
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…imit prioritization
Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-Authored-By: huangdijia <8337659+huangdijia@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/rate-limit/src/Annotation/AutoSort.php (1)
17-20: 建议确认 AutoSort 名称与文档/PR 描述中的 SmartOrder 是否统一目前公开 API 类名是
AutoSort,而 PR 描述里提到的是 “SmartOrder 注解”(README 里也叫 “Smart Order” 功能)。如果最终对外的注解名称希望是 SmartOrder,建议统一类名 / 命名空间 / 文档示例,避免用户在阅读文档和代码时产生混淆;如果决定沿用AutoSort作为正式名称,也建议在 README 中显式说明#[AutoSort]是启用“智能排序/Smart Order”功能的入口,并在这里加一个简单的 PHPDoc 注释说明用途,提升可读性。src/rate-limit/src/Aspect/RateLimitAspect.php (2)
42-55: 建议仅在启用 AutoSort 时使用优先队列,未启用时保持原先顺序遍历现在即使方法上没有
#[AutoSort],也会将所有RateLimit注解塞进SplPriorityQueue,并在第二个foreach ($queue as $annotation)中消费。若Hyperf\Stdlib\SplPriorityQueue对相同优先级元素不保证原始插入顺序,那么默认场景(未加 AutoSort)里,多重RateLimit的执行顺序就有可能与旧版本不一致,这和“功能为可选且保持向后兼容”的目标不太吻合。建议结构上改成仅在
$isAutoSort为真时才启用优先队列,否则继续按注解声明顺序直接遍历,例如:- /** @var SplPriorityQueue<RateLimit> $queue */ - $queue = new SplPriorityQueue(); - foreach ($annotations?->toAnnotations() ?? [] as $annotation) { - /** @var RateLimit $annotation */ - $priority = 0; - if ($isAutoSort) { - $priority = 0 - $annotation->maxAttempts / $annotation->decay; - } - $queue->insert($annotation, $priority); - } - - foreach ($queue as $annotation) { + $items = $annotations?->toAnnotations() ?? []; + + if ($isAutoSort && $items !== []) { + /** @var SplPriorityQueue<RateLimit> $queue */ + $queue = new SplPriorityQueue(); + foreach ($items as $annotation) { + /** @var RateLimit $annotation */ + $priority = 0; + // 见下方关于优先级公式的建议 + $queue->insert($annotation, $priority); + } + $items = $queue; + } + + foreach ($items as $annotation) { /** @var RateLimit $annotation */ $key = $this->resolveKey($annotation->key, $proceedingJoinPoint); …这样可以确保:未启用 AutoSort 时,行为与旧版尽量保持一致;启用 AutoSort 时才引入额外排序逻辑,也能减少一点运行时开销。
49-51: 优先级公式中的取反可能与“更严格优先”的语义不符,且缺少对 decay==0 的保护当前实现:
if ($isAutoSort) { $priority = 0 - $annotation->maxAttempts / $annotation->decay; }几个点建议确认:
“更严格先执行”的排序方向
PR 描述中举例说 10 次/分钟 应该先于 100 次/小时 执行;如果你以maxAttempts / decay作为“吞吐率”指标,并且SplPriorityQueue是典型的“优先级值越大越先出队”的 max-heap,那么直接使用maxAttempts / decay就能让吞吐率更高的规则(如 10/min,对应约 0.1667)排在前面,而加上负号后,吞吐率较低的规则(如 100/hour,对应约 0.0278,负号更接近 0)反而会被视为“优先级更高”。这和注释/文档中的预期可能相反,建议用单元测试验证一下排序是否符合业务语义。避免
decay == 0时的除零风险
虽然业务上大概率不会把decay配置为 0,但这里直接做除法仍然存在潜在除零问题。可以在计算前做一次简单保护。综合考虑,可以改成类似下面这样(仅示意,具体排序依据按你定义的“严格程度”调整):
- if ($isAutoSort) { - $priority = 0 - $annotation->maxAttempts / $annotation->decay; - } + if ($isAutoSort && $annotation->decay > 0) { + // 示例:按 maxAttempts/decay 降序,吞吐率越高优先级越高 + $priority = $annotation->maxAttempts / $annotation->decay; + }或根据你的定义,如果认为“吞吐率越低越严格”,可以在上面的基础上反向排序,但建议在代码或文档中写明该策略,并加一两个包含
10/min与100/hour的测试用例来锁定预期行为。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/rate-limit/README.md(4 hunks)src/rate-limit/src/Annotation/AutoSort.php(1 hunks)src/rate-limit/src/Aspect/RateLimitAspect.php(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/rate-limit/README.md
🧰 Additional context used
🧬 Code graph analysis (1)
src/rate-limit/src/Aspect/RateLimitAspect.php (2)
src/rate-limit/src/Exception/RateLimitException.php (1)
RateLimitException(17-27)src/rate-limit/src/RateLimiterFactory.php (1)
RateLimiterFactory(23-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
…ion (#1016) * feat: add SmartOrder annotation for intelligent RateLimit prioritization This feature introduces a new SmartOrder annotation that enables intelligent prioritization of multiple RateLimit annotations on the same method. Key changes: - Added SmartOrder annotation class for marking methods that need smart ordering - Modified RateLimitAspect to support priority-based RateLimit processing - RateLimits are now processed in order of strictness (maxAttempts/decay ratio) - More restrictive rate limits are evaluated first for better efficiency The SmartOrder annotation allows developers to optimize rate limiting behavior by ensuring stricter limits are checked before more lenient ones. * feat: replace SmartOrder annotation with AutoSort for improved rate limit prioritization * Update README.md Co-Authored-By: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-Authored-By: huangdijia <8337659+huangdijia@users.noreply.github.com> * feat: 添加中文文档以支持限流组件的使用说明 --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Summary
This PR introduces a new SmartOrder annotation that enables intelligent prioritization of multiple RateLimit annotations on the same method.
Changes
SmartOrderannotation class (src/rate-limit/src/Annotation/SmartOrder.php)RateLimitAspectto support priority-based RateLimit processingSplPriorityQueuefor intelligent prioritizationFeatures
The SmartOrder annotation allows developers to optimize rate limiting behavior by:
Usage Example
With SmartOrder, the stricter limit (10/minute) will be evaluated first, improving performance by avoiding unnecessary checks of more lenient limits.
Testing
The changes maintain full backward compatibility. Existing RateLimit annotations continue to work exactly as before. The SmartOrder annotation is opt-in and only affects methods where it's explicitly used.
Summary by CodeRabbit
新功能
文档