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

Fix hookOnCancel handing logic in SentinelReactorSubscriber #1089

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

sczyh30
Copy link
Member

@sczyh30 sczyh30 commented Oct 16, 2019

Describe what this PR does / why we need it

Fix the hookOnCancel handing logic in SentinelReactorSubscriber to avoid Entry-exit leaking.

Does this pull request fix one issue?

Fixes #1084

Describe how you did it

Add tryCompleteEntry() in hookOnCancel hook of SentinelReactorSubscriber.

Note that this is only a temporary solution. We might need to improve or refactor the SentinelReactorSubscriber later to handle all events more appropriately according to the reactive stream specification.

Describe how to verify it

Run the test cases.

Special notes for reviews

NONE

…scriber

Signed-off-by: Eric Zhao <sczyh16@gmail.com>
@sczyh30 sczyh30 added the to-review To review label Oct 16, 2019
@codecov-io
Copy link

Codecov Report

Merging #1089 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1089      +/-   ##
============================================
+ Coverage      42.7%   42.78%   +0.08%     
- Complexity     1491     1493       +2     
============================================
  Files           319      319              
  Lines          9378     9379       +1     
  Branches       1279     1279              
============================================
+ Hits           4005     4013       +8     
+ Misses         4878     4872       -6     
+ Partials        495      494       -1
Impacted Files Coverage Δ Complexity Δ
...nel/adapter/reactor/SentinelReactorSubscriber.java 86.88% <100%> (+6.88%) 21 <1> (+2) ⬆️
.../slots/block/flow/controller/WarmUpController.java 78.84% <0%> (-3.85%) 10% <0%> (-1%)
...a/csp/sentinel/slots/statistic/base/LeapArray.java 67.32% <0%> (-2.98%) 33% <0%> (-1%)
...tinel/slots/block/flow/param/ParamFlowChecker.java 55.4% <0%> (+2.7%) 29% <0%> (+1%) ⬆️
...p/sentinel/datasource/consul/ConsulDataSource.java 69.56% <0%> (+4.34%) 7% <0%> (ø) ⬇️
...ava/com/alibaba/csp/sentinel/node/ClusterNode.java 100% <0%> (+4.76%) 8% <0%> (+1%) ⬆️

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 826447b...272fd27. Read the comment docs.

Copy link

@ghost ghost 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 b05502f into alibaba:master Oct 23, 2019
@sczyh30 sczyh30 removed the to-review To review label Oct 23, 2019
@sczyh30 sczyh30 added this to the 1.7.0 milestone Oct 23, 2019
@sczyh30 sczyh30 deleted the fix-reactor-20191016 branch October 23, 2019 11:35
hughpearse pushed a commit to hughpearse/Sentinel that referenced this pull request Jun 2, 2021
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.

Incorrect entry-exit state when assembling multiple chained SentinelReactorTransformer
2 participants