Skip to content

Conversation

@sczyh30
Copy link
Member

@sczyh30 sczyh30 commented Sep 18, 2019

Describe what this PR does / why we need it

Add concrete exception message in AsyncEntry#cleanCurrentEntryInLocal, which could be helpful to discover errors when using AsyncEntry.

Does this pull request fix one issue?

NONE

Describe how to verify it

Run the demo and see the exception message:

public static void main(String[] args) {
    ContextUtil.enter("abc");
    final Context context = ContextUtil.getContext();
    final ExecutorService pool = Executors.newFixedThreadPool(15);
    for (int i = 0; i < 20; i++) {
        pool.submit(new Runnable() {
            @Override
            public void run() {
                ContextUtil.runOnContext(context, new Runnable() {
                    @Override
                    public void run() {
                        Entry entry = null;
                        try {
                            entry = SphU.asyncEntry("abcSync" + ThreadLocalRandom.current().nextInt());
                        } catch (BlockException ex) {
                            ex.printStackTrace();
                        } finally {
                            if (entry != null) {
                                entry.exit();
                            }
                        }
                    }
                });
            }
        });
    }
}

Special notes for reviews

NONE

@sczyh30 sczyh30 added kind/enhancement Category issues or prs related to enhancement. to-review To review labels Sep 18, 2019
@sczyh30 sczyh30 requested a review from cdfive September 18, 2019 09:35
@sczyh30 sczyh30 self-assigned this Sep 18, 2019
@codecov-io
Copy link

Codecov Report

Merging #1047 into master will increase coverage by 0.06%.
The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master   #1047      +/-   ##
===========================================
+ Coverage     42.64%   42.7%   +0.06%     
- Complexity     1472    1475       +3     
===========================================
  Files           317     317              
  Lines          9286    9291       +5     
  Branches       1267    1268       +1     
===========================================
+ Hits           3960    3968       +8     
+ Misses         4833    4832       -1     
+ Partials        493     491       -2
Impacted Files Coverage Δ Complexity Δ
...main/java/com/alibaba/csp/sentinel/AsyncEntry.java 94.28% <85.71%> (-2.39%) 11 <0> (ø)
...tinel/slots/block/flow/param/ParamFlowChecker.java 52.7% <0%> (-2.71%) 28% <0%> (-1%)
...l/cluster/server/connection/ConnectionManager.java 76% <0%> (+2%) 11% <0%> (+1%) ⬆️
...a/csp/sentinel/slots/statistic/base/LeapArray.java 70.29% <0%> (+2.97%) 34% <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 4d9a5ef...0f14fad. Read the comment docs.

Copy link
Collaborator

@cdfive cdfive 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 9ff6e47 into alibaba:master Sep 19, 2019
@sczyh30 sczyh30 removed the to-review To review label Sep 19, 2019
@sczyh30 sczyh30 added this to the 1.7.0 milestone Sep 19, 2019
@sczyh30 sczyh30 deleted the add-async-entry-log branch September 19, 2019 01:38
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

kind/enhancement Category issues or prs related to enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants