kkoomen/nestjs-throttler-storage-redis

Not working for limit greater than 10_000

Zippersk opened this issue · 5 comments

Zippersk commented

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 commented

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

wips commented

@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 commented

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

wips commented

@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 commented

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

  • 👍1