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 concurrentHashMap get and put in large numbers concurrency #1918

Merged
merged 1 commit into from
Jan 28, 2021

Conversation

samuelllin
Copy link
Contributor

fix concurrentHashMap get and put in large numbers concurrency when MetricFetcher handler response from client

@samuelllin samuelllin marked this pull request as draft December 26, 2020 08:39
@samuelllin samuelllin marked this pull request as ready for review December 26, 2020 08:43
@samuelllin samuelllin marked this pull request as draft December 26, 2020 08:43
@samuelllin samuelllin closed this Dec 26, 2020
@samuelllin samuelllin reopened this Dec 26, 2020
@samuelllin samuelllin marked this pull request as ready for review December 26, 2020 08:47
@sczyh30 sczyh30 added the area/dashboard Issues or PRs about Sentinel Dashboard label Dec 28, 2020
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

@cdfive
Copy link
Collaborator

cdfive commented Jan 26, 2021

The map in handleBody method is ConcurrentHashMap, I suddenly found that it was a local variable, which declared in
fetchOnce method, fetchOnce->handleResponse->handleBody, does it affects multithreading safety and performance?

Anyway, using computeIfAbsent, it is at least a kind of optimization, which simplifies the writing method and enhances the readability.

@cdfive
Copy link
Collaborator

cdfive commented Jan 26, 2021

fix concurrentHashMap get and put in large numbers concurrency when MetricFetcher handler response from client

Oh, in large numbers concurrency, the size of map may be too big, computeIfAbsent is better than get and put?

If we make sure it's a local variable, how about we use LinkedHashMap instead of ConcurrentHashMap? since LinkedHashMap use default computeIfAbsent method in Map interface.

How do you think about it? @samuelxw @linlinisme @jasonjoo2010 @sczyh30

@cdfive
Copy link
Collaborator

cdfive commented Jan 26, 2021

Sorry, I didn't read it carefully. Although it is a local variable in the fetchonce method, it is shared by multiple threads.

@jasonjoo2010 jasonjoo2010 merged commit 1f4614c into alibaba:master Jan 28, 2021
shenbaoyong pushed a commit to shenbaoyong/Sentinel that referenced this pull request Feb 5, 2021
hughpearse pushed a commit to hughpearse/Sentinel that referenced this pull request Jun 2, 2021
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 2, 2021
linkolen added a commit to shivagowda/Sentinel that referenced this pull request Aug 2, 2021
Improve MetricFetcher under concurrent conditions (alibaba#1918)
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 14, 2021
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 14, 2021
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 15, 2021
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dashboard Issues or PRs about Sentinel Dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants