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

GatewayApiMatcherManager BUG #2436

Merged
merged 3 commits into from Nov 29, 2021
Merged

GatewayApiMatcherManager BUG #2436

merged 3 commits into from Nov 29, 2021

Conversation

su-yh
Copy link
Contributor

@su-yh su-yh commented Oct 31, 2021

Describe what this PR does / why we need it

网关流控API
当我添加一个API 定义之后,会同时更新GatewayApiMatcherManager
但是当我再将这个API 定义删除之后,也会更新GatewayApiMatcherManager,不过它只会添加不会删除。

Does this pull request fix one issue?

#2433

Describe how you did it

Describe how to verify it

Special notes for reviews

@CLAassistant
Copy link

CLAassistant commented Oct 31, 2021

CLA assistant check
All committers have signed the CLA.

@sczyh30
Copy link
Member

sczyh30 commented Nov 1, 2021

Could you please sign the CLA here: https://cla-assistant.io/alibaba/Sentinel?pullRequest=2436

@sczyh30 sczyh30 added the area/gateway-flow-control Issues or PRs related to API gateway flow control label Nov 1, 2021
/**
* @author Eric Zhao
* @since 1.6.0
*/
public final class GatewayApiMatcherManager {

private static final Map<String, WebExchangeApiMatcher> API_MATCHER_MAP = new ConcurrentHashMap<>();
private static Map<String, WebExchangeApiMatcher> API_MATCHER_MAP = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just volatile HashMap is enough?

Copy link
Contributor Author

@su-yh su-yh Nov 3, 2021

Choose a reason for hiding this comment

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

使用volatile 会更好吗?
你这是出于什么考虑,而建议使用它呢?

Copy link
Member

Choose a reason for hiding this comment

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

ConcurrentHashMap -> volatile HashMap

Copy link
Member

@sczyh30 sczyh30 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 5382b3d into alibaba:master Nov 29, 2021
sczyh30 pushed a commit that referenced this pull request Nov 29, 2021
…re not removed (#2436)

* Fix the bug that legacy API matchers in SC gateway adapter GatewayApiMatcherManager were not removed after the API group has been removed
@sczyh30
Copy link
Member

sczyh30 commented Nov 29, 2021

Thanks for contributing!

Zhang-0952 pushed a commit to Zhang-0952/Sentinel that referenced this pull request Mar 4, 2022
…re not removed (alibaba#2436)

* Fix the bug that legacy API matchers in SC gateway adapter GatewayApiMatcherManager were not removed after the API group has been removed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-flow-control Issues or PRs related to API gateway flow control kind/bug Category issues or prs related to bug.
Projects
None yet
3 participants