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

The tokenlimit with redis may work incorrectly due to network latency #4072

Open
hrzq19901209 opened this issue Apr 12, 2024 · 4 comments
Open

Comments

@hrzq19901209
Copy link

hrzq19901209 commented Apr 12, 2024

Redis may get a lua cmd which generated a few seconds ago, this may make token increase incorrectly.
因为网络延迟以及reids排队处理命令,redis可能处理一个几秒前lua脚本,这会导致桶中的令牌出现非预期的增长。
比如令牌的增加速率是5/s,桶的大小是5,一次取一个令牌。(now、KEYS[2]对应lua脚本的内容)
now:1712918331 令牌数:5 拿到令牌 KEYS[2]:1712918331
now:1712918331 令牌数:4 拿到令牌 KEYS[2]:1712918331
now:1712918331 令牌数:3 拿到令牌 KEYS[2]:1712918331
now:1712918331 令牌数:2 拿到令牌 KEYS[2]:1712918331
now:1712918331 令牌数:1 拿到令牌 KEYS[2]:1712918331

now:1712918332 令牌数:5 拿到令牌 KEYS[2]:1712918332
now:1712918332 令牌数:4 拿到令牌 KEYS[2]:1712918332
now:1712918332 令牌数:3 拿到令牌 KEYS[2]:1712918332
now:1712918332 令牌数:2 拿到令牌 KEYS[2]:1712918332

now:1712918331 令牌数:1 拿到令牌 KEYS[2]:1712918331 因为网络延迟导致redis现在才处理该命令,处理完后,KEYS[2]的值前移至1712918331,下次请求token时桶中的令牌数将增加5个

now:1712918332 令牌数:5 拿到令牌 KEYS[2]:1712918332 这里理应是拿不到令牌,1712918332该时段的令牌已经被拿完了
now:1712918332 令牌数:4 拿到令牌 KEYS[2]:1712918332
now:1712918332 令牌数:3 拿到令牌 KEYS[2]:1712918332
now:1712918332 令牌数:2 拿到令牌 KEYS[2]:1712918332
now:1712918332 令牌数:1 拿到令牌 KEYS[2]:1712918332

从业务角度看,1712918332这个时间段将处理9次请求,限流没生效

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Redis may get a lua cmd which generated a few seconds ago, this may make token increase incorrectly.
Because of network delays and reids queuing commands, redis may process a Lua script several seconds ago, which will cause unexpected growth of tokens in the bucket.
For example, the token increase rate is 5/s, the bucket size is 5, and one token is removed at a time. (now, KEYS[2] correspond to the content of the lua script)
now: 1712918331 Number of tokens: 5 Get token KEYS[2]:1712918331
now: 1712918331 Number of tokens: 4 Get token KEYS[2]:1712918331
now: 1712918331 Number of tokens: 3 Get token KEYS[2]:1712918331
now: 1712918331 Number of tokens: 2 Get token KEYS[2]:1712918331
now: 1712918331 Number of tokens: 1 Get token KEYS[2]:1712918331

now: 1712918332 Number of tokens: 5 Get token KEYS[2]:1712918332
now: 1712918332 Number of tokens: 4 Get token KEYS[2]:1712918332
now: 1712918332 Number of tokens: 3 Get token KEYS[2]:1712918332
now: 1712918332 Number of tokens: 2 Get token KEYS[2]:1712918332

now: 1712918331 Number of tokens: 1 Get the token KEYS[2]:1712918331 Because of the network delay, redis is only processing the command now. After processing, the value of KEYS[2] is moved forward to 1712918331, and the next time the token is requested, the bucket The number of tokens in will be increased by 5

now: 1712918332 Number of tokens: 5 Get the token KEYS[2]:1712918332 You should not be able to get the token here. 1712918332 The tokens for this period have been taken.
now: 1712918332 Number of tokens: 4 Get token KEYS[2]:1712918332
now: 1712918332 Number of tokens: 3 Get token KEYS[2]:1712918332
now: 1712918332 Number of tokens: 2 Get token KEYS[2]:1712918332
now: 1712918332 Number of tokens: 1 Get token KEYS[2]:1712918332

From a business perspective, 9 requests will be processed during this time period of 1712918332, and the current limit does not take effect.

@kevwan
Copy link
Contributor

kevwan commented Apr 13, 2024

Thanks for submitting the issue! Any idea on the solution?

@Lansongxx
Copy link

@kevwan I think you just need to determine if now is greater than last_refreshed when updating last_refreshed, and only update it if it is. If you're okay with what I'm doing, I can submit a PR

@kevwan
Copy link
Contributor

kevwan commented Apr 17, 2024

Would you please submit a PR that we can discuss on it?

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

No branches or pull requests

4 participants