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

redis_gcra retry times #18

Open
rexdjw opened this issue Sep 13, 2022 · 1 comment
Open

redis_gcra retry times #18

rexdjw opened this issue Sep 13, 2022 · 1 comment

Comments

@rexdjw
Copy link

rexdjw commented Sep 13, 2022

Hi, I am having issues with the retry timers via the redis_gcra implementation.

-How you installed rush and the versions of its dependencies (if you're using the Redis store, please include rfc3986 and redis version information).
rush was installed via pip install rush[redis]
rfc3986 v2.0.0
redis v4.3.4

What stores and limiters are you using?
redis store and redis_gcra limiter

An example that reproduces your problem
For example, consider the following basic test code:

from rush.limiters import redis_gcra
        from rush.stores import redis
        from rush import quota
        from rush import throttle

 quota_per_min = quota.Quota.per_minute(2)

        limiter = redis_gcra.GenericCellRatelimiter(
            store=redis.RedisStore(REDIS_CONN)
        )

        throttle_min = throttle.Throttle(rate=quota_per_min, limiter=limiter)

        results = throttle_min.check(key="mins", quantity=1)

        print(results)

When ran 3x in a row, the following RateLimit objects are shown:

RateLimitResult(limit=2, limited=False, remaining=1, reset_after=datetime.timedelta(seconds=1800), retry_after=datetime.timedelta(days=-1, seconds=86399))

RateLimitResult(limit=2, limited=False, remaining=0, reset_after=datetime.timedelta(seconds=3597, microseconds=697241), retry_after=datetime.timedelta(days=-1, seconds=86399))

RateLimitResult(limit=2, limited=True, remaining=0, reset_after=datetime.timedelta(seconds=3595, microseconds=418555), retry_after=datetime.timedelta(seconds=1795, microseconds=418555))

Therefore, after 3 attempted calls in a minute (with a limit of 2 per minute), on the third call, it suggests a retry time of ~1800 seconds. I don't understand this behavior as that's largely padded against the 2 calls per minute ideal window. Furthermore, if I use hours instead of minutes, the buffer is even more egregious (to the tune of days)

@sigmavirus24
Copy link
Owner

I believe we're not accounting for the fact that the redis lua script is thinking in microseconds rather than seconds and so we need to appropriately convert in the Python to seconds for things to be correct. Good catch on this. Would you like to send a PR to fix 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

2 participants