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

Add support for asynchronous invocation entry #146

Merged
merged 2 commits into from
Sep 14, 2018
Merged

Conversation

sczyh30
Copy link
Member

@sczyh30 sczyh30 commented Sep 12, 2018

Describe what this PR does / why we need it

Add support for asynchronous invocation entry so that users can use Sentinel in asynchronous logic.

Does this pull request fix one issue?

Resolves #31

Describe how you did it

Internal

  • Add several asyncEntry method in Sph interface and SphU class.
  • Add a new AsyncEntry class for asynchronous entry representation, which resembles the linked CtEntry. The difference:
    • When calling SphU.asyncEntry, the created async entry is set to current context. As soon as the entry passed the slot chain (or blocked), it will be cleared from current context intermediately so that it won't affect the proceeding entries.
    • When the created async entry successfully passed the slot chain in CtSph#asyncEntryInternal, the async context inside the async entry will be initialized. We can get the async context via AsyncEntry#getAsyncContext method and use with ContextUtil#runOnContext to do context switching in asynchronous callback.
  • Add runOnContext and replaceContext method in ContextUtil for context switching.

For users

We can call SphU.asyncEntry(xxx) to entry for asynchronous invocation, then exit it in the callback (or other place, often in different thread). If we want nested entry in asynchronous entry, we have to wrap the logic within the async context via ContextUtil.runOnContext(asyncContext, lambda).

Here is an example:

public void testAsync() {
    try {
        final AsyncEntry entry = SphU.asyncEntry(resourceName);

        CompletableFuture.runAsync(() -> {
            ContextUtil.runOnContext(entry.getAsyncContext(), () -> {
                try {
                    TimeUnit.SECONDS.sleep(2);
                    
                    // Call another sync resource under async context.
                    doAnotherSync();
                } catch (InterruptedException e) {
                    // Ignore.
                } finally {
                    // User should exit the resource in the callback.
                    entry.exit();
                }
            });
        });
    } catch (BlockException ex) {
        // Resource blocked. 
        ex.printStackTrace();
    }
}

Describe how to verify it

See test cases.

@sczyh30 sczyh30 added the to-review To review label Sep 12, 2018
@sczyh30 sczyh30 added this to the 0.2.0 milestone Sep 12, 2018
@CLAassistant
Copy link

CLAassistant commented Sep 12, 2018

CLA assistant check
All committers have signed the CLA.

Signed-off-by: Eric Zhao <sczyh16@gmail.com>
@codecov-io
Copy link

codecov-io commented Sep 12, 2018

Codecov Report

Merging #146 into master will increase coverage by 2.06%.
The diff coverage is 79.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master   #146      +/-   ##
==========================================
+ Coverage     45.94%    48%   +2.06%     
- Complexity      563    639      +76     
==========================================
  Files           115    120       +5     
  Lines          3844   4081     +237     
  Branches        536    577      +41     
==========================================
+ Hits           1766   1959     +193     
- Misses         1862   1886      +24     
- Partials        216    236      +20
Impacted Files Coverage Δ Complexity Δ
...java/com/alibaba/csp/sentinel/context/Context.java 69.69% <100%> (+12.55%) 15 <3> (+5) ⬆️
.../com/alibaba/csp/sentinel/context/ContextUtil.java 78.72% <100%> (+5.03%) 11 <3> (+3) ⬆️
...e/src/main/java/com/alibaba/csp/sentinel/SphU.java 80% <33.33%> (-11.67%) 12 <1> (+1)
.../src/main/java/com/alibaba/csp/sentinel/CtSph.java 54.54% <50%> (-11.37%) 12 <2> (+4)
...main/java/com/alibaba/csp/sentinel/AsyncEntry.java 88% <88%> (ø) 8 <8> (?)
...rc/main/java/com/alibaba/csp/sentinel/CtEntry.java 94.28% <94.28%> (ø) 13 <13> (?)
...ntinel/slots/statistic/metric/WindowLeapArray.java 68% <0%> (-12%) 5% <0%> (-1%)
...a/csp/sentinel/slots/statistic/base/LeapArray.java 73.17% <0%> (-3.76%) 24% <0%> (+9%)
...a/csp/sentinel/slots/DefaultSlotsChainBuilder.java 100% <0%> (ø) 2% <0%> (?)
... and 12 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 ca2f4d9...2a3e335. Read the comment docs.

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 removed the to-review To review label Sep 14, 2018
@sczyh30 sczyh30 merged commit 5edac71 into master Sep 14, 2018
@sczyh30 sczyh30 deleted the feature/async-context branch September 18, 2018 08:38
Arlmls pushed a commit to Arlmls/Sentinel that referenced this pull request Jan 8, 2019
- Add several `asyncEntry` method in `Sph` interface and `SphU` class.
- Add a new `AsyncEntry` class for asynchronous entry representation. Some refactor for `CtEntry`.
- Refactored `CtSph` and implement `asyncEntryInternal` for asynchronous entry.
- Add `runOnContext` and `replaceContext` method in `ContextUtil` for context switching.
@wavesZh
Copy link
Contributor

wavesZh commented Sep 27, 2019

as you said:

When calling SphU.asyncEntry, the created async entry is set to current context. As soon as the entry passed the slot chain (or blocked), it will be cleared from current context intermediately so that it won't affect the proceeding entries.

Other processing will get wrong entry for the context during SphU.asyncEntry changed curEntry, it's ok?

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.

[RFC] Add support for asynchronous invocation
5 participants