vutran1710 / PyrateLimiter

⚔️Python Rate-Limiter using Leaky-Bucket Algorithm Family
https://pyratelimiter.readthedocs.io
MIT License
338 stars 36 forks source link

FEATURE REQUEST: monotonic time being made passed parameter #53

Closed navyamehta closed 2 years ago

navyamehta commented 2 years ago

Hey! Was wondering if we could make now an optional parameter passed to limiter.try_acquire? Currently it uses monotonic time, but that wouldn't work in a distributed setting where different computers may use their own monotonic clock. Thus, maybe we can't use it in a distributed setting. Thanks! Or maybe we can change it to unix seconds?

navyamehta commented 2 years ago

Essentially, could we replace now=monotonic() with now=datetime.utcnow().timestamp()?

vutran1710 commented 2 years ago

That got me thinking maybe we need to anchor the start time somewhere else. Sound reasonable? Ill see what I can do.

navyamehta commented 2 years ago

I was thinking maybe we take a benchmark=0 parameter in the constructor for the limiter (so defaults to 0 but some value can be given). Then, now=datetime.utcnow().timestamp() - self.benchmark and that ensures you dont store very large numbers in the bucket since you can set a comfortable benchmark? Would love to know your thoughts

vutran1710 commented 2 years ago

That sounds like a right approach. But regarding the clock, i prefer to stick with the monotonic since there was a discussion about this some time ago that came out as valid point to use monotonic. So yes I think we can combine your approach with the current clocking mechanism. What do you think about this?

navyamehta commented 2 years ago

Since the monotonic clock is dependent on the instance (and will not reconcile between different instances in a distributed setup), how exactly would we combine the two approaches?

vutran1710 commented 2 years ago

To be frank, I dont know yet 😂.

But I think it can be figured out along the development process.

Probably need to solve some linear equation system as well. 😂

insspb commented 2 years ago

@vutran1710 @navyamehta

Just to comment on benchmark. I do not think that it required with mine proposed approach. You are free to write you own function to modify time before using.

Like:

short_time = lambda: (datetime.utcnow().timestamp() - 1636304302)
vutran1710 commented 2 years ago

Handled in #58