[ISSUE 15677] Support method level TPS in DefaultTPSLimiter
#15679
Conversation
… corresponding unit tests
DefaultTPSLimiter
DefaultTPSLimiterDefaultTPSLimiter
DefaultTPSLimiterDefaultTPSLimiter
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 3.3 #15679 +/- ##
=========================================
Coverage 61.01% 61.02%
+ Complexity 11698 11697 -1
=========================================
Files 1923 1923
Lines 87069 87073 +4
Branches 13112 13113 +1
=========================================
+ Hits 53124 53134 +10
+ Misses 28495 28490 -5
+ Partials 5450 5449 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements method-level TPS (Transactions Per Second) limiting in DefaultTPSLimiter by modifying the key generation strategy for the internal statistics map. Previously, method-level TPS configurations were ignored because all methods of a service shared the same StatItem using only the service key.
- Modified key generation to include method name when method-level TPS is configured
- Added method-level TPS precedence logic to prioritize method configuration over service configuration
- Added comprehensive tests to verify method-level TPS isolation and precedence behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| DefaultTPSLimiter.java | Updated key generation logic to support method-level TPS by appending method name to service key when method-level configuration exists |
| DefaultTPSLimiterTest.java | Added three new test methods to verify method-level TPS precedence, service-level fallback, and isolation between methods |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| String key = isMethodLevelTpsConfigured | ||
| ? url.getServiceKey() + "#" + RpcUtils.getMethodName(invocation) | ||
| : url.getServiceKey(); |
There was a problem hiding this comment.
[nitpick] The hardcoded '#' separator could be extracted as a constant to improve maintainability and make the key format more explicit.
There was a problem hiding this comment.
It doesn’t support method overloading here, but that should be fine.
|
Is there any update on this? Please let me know if you have any questions about this. |
Please refer to issue 15677 for more details.
Although I can now configure tps on a method level, the stats map in
DefaultTPSLimiteris still usingserviceKeyas the key. So the method-level TPS doesn't work. For example, if I configure different TPS numbers on different methods, they are actually sharing the same StatItem, which is obtained by the service key.Here is what I have done:
serviceKey + methodif the tps is configured for a method. Otherwise, usesserviceKey. TPS for a method has a higher priority than for a service.