Skip to content

use caffeine instead of ConcurrentLinkedHashMap (#2871)#2878

Open
shaoqiang6 wants to merge 4 commits intoalibaba:masterfrom
shaoqiang6:dev-caffeine2
Open

use caffeine instead of ConcurrentLinkedHashMap (#2871)#2878
shaoqiang6 wants to merge 4 commits intoalibaba:masterfrom
shaoqiang6:dev-caffeine2

Conversation

@shaoqiang6
Copy link
Copy Markdown

@shaoqiang6 shaoqiang6 commented Sep 12, 2022

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
Copy Markdown
Author

shaoqiang6 commented Sep 14, 2022 •
edited
Loading

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

@sczyh30
Copy link
Copy Markdown
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
Copy Markdown
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 组件整体从项目中移除掉。
公司会做开源扫描，这个组件有漏洞不让使用。

Comment thread ...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
Copy Markdown
Contributor

@su-yh su-yh Sep 26, 2022

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
Copy Markdown
Author

@shaoqiang6 shaoqiang6 Sep 27, 2022

Choose a reason for hiding this comment

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

done.

Copy link
Copy Markdown
Contributor

@su-yh su-yh Sep 28, 2022

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 brotherlu-xcq, cdfive and sczyh30 and removed request for su-yh October 10, 2022 03:49
@shaoqiang6 shaoqiang6 requested review from su-yh and removed request for brotherlu-xcq, cdfive and sczyh30 October 20, 2022 10:44
@shaoqiang6 shaoqiang6 marked this pull request as draft October 20, 2022 10:46
@shaoqiang6
Copy link
Copy Markdown
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
Copy Markdown
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