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

feature:add global or ip or header rate limiting plugin #686

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bfyxzls
Copy link

@bfyxzls bfyxzls commented Dec 12, 2023

Ⅰ. Describe what this PR did

global or ip or header rate limiting plugin

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

envoy filter config this wasm plugin,then limit refresh page.

Ⅴ. Special notes for reviews

@bfyxzls bfyxzls requested a review from johnlanni as a code owner December 12, 2023 08:35
@johnlanni
Copy link
Collaborator

683 需要基于redis实现

var err error
for i := 0; i < 10; i++ { // cas冲突时会重试10次
binary.LittleEndian.PutUint64(buf, uint64(value+1))
err = proxywasm.SetSharedData(cacheKey, buf, cas)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个 shareddata 没有过期机制,这样写会导致内存泄漏

Copy link
Author

Choose a reason for hiding this comment

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

这块的目的是,如果出现并发冲突,就进行重试,一共最多重拾10次

Copy link
Collaborator

Choose a reason for hiding this comment

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

嗯,我的意思是这里的key太发散了,跟时间戳相关

Copy link
Author

Choose a reason for hiding this comment

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

是的,key的话,每秒都会有新key出现,我又做了一个时间窗口的,基于totp生成key,比这个稍微好一些。

Copy link
Collaborator

Choose a reason for hiding this comment

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

key不会过期还是个问题,会导致内存一直上涨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants