fix #3386
Describe what this PR does / why we need it
This PR contains fixes for #3386
Does this pull request fix one issue?
Fixes #3386
Describe how you did it
Combine origin method's parameter type canonical names into register map key.
Describe how to verify it
See bug #3386 to repreduce and verify.
Special notes for reviews
这看起来确实是个问题,但是要不要在运行时支持这种同名我觉得可能得再讨论下,因为这样做会引入一些不必要的额外的复杂度,但其实只要不重名就不会有问题,另外 Issue 中的这个插件看起来是个不错的解决方案
#3397 也要改这一处。我觉得改 #3397 还得把返回值类型信息也加到 key 里,复杂度会更高些 😂
从概率论上讲,代码里出现重名导致这个bug是一定会出现的。所以要讨论的是怎么处理这个bug。 要么就这么放着不处理,相当于认为是快速失败,那么遇到这个问题的团队能不能避免生产事故就要看研发有没有了解到这个bug、测试有没有测到这个场景了;否则就进行修复。 修复方案我的想法是既要避免报错,也保证流控不失效(不打破研发设计预期)。 关于复杂度,我没觉得这是个问题,而且我变更的代码我自己觉得还挺可控的,不复杂吧。😂 @LearningGp 你有其他的修复思路吗?
P.S. 欢迎试用插件。如果觉得不错,可以留言评论+分享。插件已设置开源项目 free 🤝。
Codecov Report
Attention: Patch coverage is 71.42857% with 12 lines in your changes are missing coverage. Please review.
Project coverage is 45.85%. Comparing base (
dbd3c06) to head (945468f). Report is 1 commits behind head on 1.8.
Additional details and impacted files
@@ Coverage Diff @@
## 1.8 #3395 +/- ##
============================================
- Coverage 45.90% 45.85% -0.06%
Complexity 2148 2148
============================================
Files 431 431
Lines 12906 12932 +26
Branches 1728 1728
============================================
+ Hits 5925 5930 +5
- Misses 6281 6301 +20
- Partials 700 701 +1
| Components | Coverage Δ | |
|---|---|---|
| sentinel-adapter | 43.22% <ø> (ø) |
|
| sentinel-cluster | 23.71% <ø> (-0.08%) |
:arrow_down: |
| sentinel-core | 59.64% <ø> (+0.01%) |
:arrow_up: |
| sentinel-extension | 45.88% <71.42%> (-0.27%) |
:arrow_down: |
| sentinel-logging | 54.54% <ø> (ø) |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
如果方便的话,麻烦在sentinel-demo-annotation-spring-aop模块中完善下Demo以体现新的特性
如果方便的话,麻烦在sentinel-demo-annotation-spring-aop模块中完善下Demo以体现新的特性
OK,我这两天看时间搞下。
Done.
Run module sentinel-demo-annotation-spring-aop, and run following
requests (in curl format) to verify that fallbacks won't overlap
each other. Once the module started, you can call requests in any
sequence.
curl -XGET localhost:19966/bar?t=
curl -XGET localhost:19966/foo?t=-1
另外昨天还看了下 resilience4j 的做法。
https://github.com/resilience4j/resilience4j/blob/eeaf57a8217ded7fe14bad511454f263f9e6f06d/resilience4j-spring6/src/main/java/io/github/resilience4j/spring6/fallback/FallbackMethod.java#L85
他们的 MethodMeta 结构我觉得值得参考。(可以直接照抄😅)
另外昨天还看了下 resilience4j 的做法。 https://github.com/resilience4j/resilience4j/blob/eeaf57a8217ded7fe14bad511454f263f9e6f06d/resilience4j-spring6/src/main/java/io/github/resilience4j/spring6/fallback/FallbackMethod.java#L85 他们的
MethodMeta结构我觉得值得参考。(可以直接照抄😅)
感谢贡献🚀,欢迎后续进行进一步优化