Skip to content
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

Refactor and improve fallback support for @SentinelResource annotation #693

Merged
merged 2 commits into from
Apr 23, 2019

Conversation

sczyh30
Copy link
Member

@sczyh30 sczyh30 commented Apr 21, 2019

Describe what this PR does / why we need it

Refactor and improve fallback support for @SentinelResource annotation to improve usability.

Does this pull request fix one issue?

Resolves #647

Describe how you did it

  • Refactor logic for blockHandler and fallback
  • Add fallbackClass support for fallback in global classes
  • Add defaultFallback support

Breaking changes: the behavior of fallback has changed. In previous versions, the fallback will take effect only when blockHandler is not configured and DegradeException caught, which might be confusing and inconvenient for users. So I improved the semantics of fallback: now it can handle all kinds of exceptions (except configured exceptionsToIgnore). And a defaultFallback is also added to support universal fallback shared for multiple methods.

The method signature requirement of fallback has changed. Now it can be:

  • return type should match the origin method;
  • parameter list should match the origin method, and an additional Throwable parameter can be provided to get the actual exception.

For defaultFallback, the requirement:

  • return type should match the origin method;
  • parameter list should be empty, and an additional Throwable parameter can be provided to get the actual exception.

For BlockException, when blockHandler is provided, Sentinel will use the blockHandler, or Sentinel will try to use fallback to handle the BlockException.

Describe how to verify it

Run the test cases and demo.

Special notes for reviews

This PR contains breaking changes. Please take care of the changes.

@sczyh30 sczyh30 added the to-review To review label Apr 21, 2019
@codecov-io
Copy link

codecov-io commented Apr 21, 2019

Codecov Report

Merging #693 into master will increase coverage by 0.9%.
The diff coverage is 71.76%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #693     +/-   ##
===========================================
+ Coverage     41.17%   42.08%   +0.9%     
- Complexity     1315     1398     +83     
===========================================
  Files           288      289      +1     
  Lines          8478     8771    +293     
  Branches       1147     1257    +110     
===========================================
+ Hits           3491     3691    +200     
- Misses         4551     4630     +79     
- Partials        436      450     +14
Impacted Files Coverage Δ Complexity Δ
...otation/aspectj/AbstractSentinelAspectSupport.java 69.17% <70.42%> (+5.17%) 38 <17> (+8) ⬆️
...l/annotation/aspectj/ResourceMetadataRegistry.java 79.16% <75%> (-4.17%) 12 <3> (+2)
...nel/annotation/aspectj/SentinelResourceAspect.java 83.33% <83.33%> (-0.88%) 5 <0> (+2)
...ntinel/slots/block/flow/param/ParameterMetric.java 68.25% <0%> (-6.75%) 37% <0%> (+8%)
...sentinel/slots/block/flow/param/ParamFlowRule.java 69.29% <0%> (-0.84%) 44% <0%> (+17%)
.../java/com/alibaba/csp/sentinel/util/SpiLoader.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
...ots/statistic/ParamFlowStatisticEntryCallback.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
.../command/handler/FetchTopParamsCommandHandler.java 0% <0%> (ø) 0% <0%> (?)
...a/csp/sentinel/slots/statistic/base/LeapArray.java 70.58% <0%> (+2.94%) 34% <0%> (+1%) ⬆️
...tinel/slots/block/flow/param/ParamFlowChecker.java 55.86% <0%> (+3.16%) 47% <0%> (+19%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 845b258...beae67a. Read the comment docs.

sczyh30 added 2 commits April 23, 2019 19:51
- Refactor logic for blockHandler and fallback to be more useful
- Add `fallbackClass` support for fallback in global class
- Add `defaultFallback` support
- Update test cases

Signed-off-by: Eric Zhao <sczyh16@gmail.com>
Signed-off-by: Eric Zhao <sczyh16@gmail.com>
@sczyh30 sczyh30 force-pushed the enhancement/annotation-fallback branch from 26bb295 to beae67a Compare April 23, 2019 11:51
Copy link
Contributor

@CarpenterLee CarpenterLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sczyh30 sczyh30 merged commit 82578e1 into master Apr 23, 2019
@sczyh30 sczyh30 deleted the enhancement/annotation-fallback branch April 23, 2019 15:17
@sczyh30 sczyh30 removed the to-review To review label Apr 23, 2019
@sczyh30 sczyh30 added this to the 1.6.0 milestone Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the semantic of fallback in @SentinelResource annotation
3 participants