Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Not working for limit greater than 10_000 #392

Closed
Zippersk opened this issue May 14, 2021 · 5 comments
Closed

Not working for limit greater than 10_000 #392

Zippersk opened this issue May 14, 2021 · 5 comments

Comments

@Zippersk
Copy link

I think, we should at least add a warning to Readme, that for a limit greater than 10 thousand this will not work because of this redis scan query: https://github.com/kkoomen/nestjs-throttler-storage-redis/blob/master/src/throttler-storage-redis.service.ts#L23 .

I believe that a limit smaller than 10 000 is fine for most cases. I have an API service where the free tier has 15 000 requests in a month (30 days), so I get stuck for a while why is this not working in production. Locally I tested it with a smaller limit and everything worked fine.

You can close this if you think that nobody else will hit this problem.

@kkoomen
Copy link
Owner

kkoomen commented May 18, 2021

If people run into the problem then I will increase it, maybe even allowing a configurable option for it.

@wips
Copy link

wips commented Nov 17, 2021

@kkoomen , thanks for the scanCount parameter, but it really took me awhile to find the issue. It especially nasty because locally you don't usually test with big limits as @Zippersk mentioned. I believe having an example in readme with full parameter set would be beneficial.

@kkoomen
Copy link
Owner

kkoomen commented Nov 17, 2021

@wips This does work right? It has been implemented so I guess I can close this. I totally forgot to close this.

@kkoomen kkoomen closed this as completed Nov 17, 2021
@wips
Copy link

wips commented Nov 18, 2021

@kkoomen , It works, thanks, but in a weird way. For example when I set LIMIT to 1k I need to set scanCount somewhere between 10k and 20k to make it reliably cut-off requests after limit is reached. It happens when I try to load the endpoint at 5-10req/s rate. I assume it has nothing to do with this project, looks like a weird behaviour of ioredis or Redis itself, I don't have much experience with it, so maybe it's expected.

@Zippersk
Copy link
Author

Is it possible to throw error when limit is less than scanCount? I think it would help...

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

No branches or pull requests

3 participants