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.

Sign up for GitHub

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

Jump to bottom

use caffeine instead of ConcurrentLinkedHashMap (#2871) #2878

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

use caffeine instead of ConcurrentLinkedHashMap (#2871) #2878

wants to merge 4 commits into from

Conversation

shaoqiang6
Copy link

Describe what this PR does / why we need it

use caffeine instead of ConcurrentLinkedHashMap

Does this pull request fix one issue?

Fixes #2871

Describe how you did it

Describe how to verify it

Special notes for reviews

@sczyh30 sczyh30 added kind/enhancement Category issues or prs related to enhancement. to-review To review labels Sep 13, 2022
@shaoqiang6
Copy link
Author

shaoqiang6 commented Sep 14, 2022 •
edited
Loading

com.alibaba.csp.sentinel.slots.statistic.data.ParamMapBucketTest#testAddEviction 这个测试用例我写错了，需要增加sleep,来验证key驱逐
请问我是仅仅修改后提交，还是要合并commit 重新发起PR？

@sczyh30
Copy link
Member

sczyh30 commented Sep 15, 2022

com.alibaba.csp.sentinel.slots.statistic.data.ParamMapBucketTest#testAddEviction 这个测试用例我写错了，需要增加sleep,来验证key驱逐 请问我是仅仅修改后提交，还是要合并commit 重新发起PR？

接着提交即可

su-yh
su-yh reviewed Sep 26, 2022
View reviewed changes
Copy link
Contributor

@su-yh su-yh left a comment

Choose a reason for hiding this comment

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

如果使用了Caffeine ，期望能将concurrentlinkedhashmap-lru 组件整体从项目中移除掉。
公司会做开源扫描，这个组件有漏洞不让使用。

...w-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java Outdated
initialize(rule, true);
}

public void initialize(ParamFlowRule rule, boolean useCaffeine) {
Copy link
Contributor

Choose a reason for hiding this comment

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

如果使用了Caffeine ，期望能将concurrentlinkedhashmap-lru 组件整体从项目中移除掉。
公司会做开源扫描，这个组件有漏洞不让使用。

Copy link
Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor

Choose a reason for hiding this comment

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

3Q

@shaoqiang6 shaoqiang6 requested a review from su-yh October 8, 2022 00:49
@sczyh30 sczyh30 requested review from sczyh30, brotherlu-xcq and cdfive and removed request for su-yh October 10, 2022 03:49
@shaoqiang6 shaoqiang6 requested review from su-yh and removed request for sczyh30, brotherlu-xcq and cdfive October 20, 2022 10:44
@shaoqiang6 shaoqiang6 marked this pull request as draft October 20, 2022 10:46
@shaoqiang6
Copy link
Author

shaoqiang6 commented Oct 20, 2022 •
edited
Loading

com.alibaba.csp.sentinel.slots.statistic.data.ParamMapBucketTest#testAddEviction 这个测试用例我写错了，需要增加sleep,来验证key驱逐 请问我是仅仅修改后提交，还是要合并commit 重新发起PR？

接着提交即可

刚刚我点错了review请求, 烦请重新指定一下 review 人员, 谢谢您

@shaoqiang6 shaoqiang6 marked this pull request as ready for review October 20, 2022 10:51
@shaoqiang6
Copy link
Author

shaoqiang6 commented Nov 13, 2022 •
edited
Loading

merge master, please review thanks

@su-yh su-yh mentioned this pull request Nov 17, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@su-yh su-yh Awaiting requested review from su-yh

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
kind/enhancement Category issues or prs related to enhancement. to-review To review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace deprecated concurrentlinkedhashmap-lru with Caffeine in param-flow-extension
3 participants
@shaoqiang6 @sczyh30 @su-yh