Add rate limiter #121
Add rate limiter #121
Conversation
| self, | ||
| task: ToolUsageTask, | ||
| *, | ||
| model: str = "gpt-3.5-turbo-16k", |
There was a problem hiding this comment.
ooc: why do we need a default here?
There was a problem hiding this comment.
I can remove in a separate PR -- probably shouldn't be here
| self.available_tokens += elapsed * self.requests_per_second | ||
| self.last = now | ||
|
|
||
| self.available_tokens = min(self.available_tokens, self.requests_per_second) |
There was a problem hiding this comment.
For noobs, could we add a comment saying something like "requests_per_second is also the maximum bucket size?
| fractions of a second. | ||
| """ | ||
| if requests_per_second < 1: | ||
| raise ValueError("Rate must be at least 1 request per second") |
There was a problem hiding this comment.
For the lazy user who doesn't read the docstring, thoughts on adding a more instructional recommendation here? I naively would assume I could let there be 1/4 requests per second, for instance
| self, | ||
| *, | ||
| requests_per_second: float = 1, | ||
| tokens_per_request: int = 1, |
There was a problem hiding this comment.
Given our domain, could we clarify that tokens are NOT LLM tokens? I could imagine someone thinking this being the number of ~words we can send to the LLM per request. Maybe link to the token bucket algorithm for complete novices as well.
This is especially the case given that some providers rate limit based on e.g., prompt tokens or total tokens per minute/hour
There was a problem hiding this comment.
Similarly, should call requests_per_second something like tokens_per_second to make it more clear?
| self.last: Optional[time.time] = None | ||
| self.check_every_n_seconds = check_every_n_seconds | ||
|
|
||
| def consume(self) -> bool: |
There was a problem hiding this comment.
Should this be private?
| # at a given time. | ||
| self._consume_lock = threading.Lock() | ||
| # tokens per request sets how many tokens | ||
| self.tokens_per_request = tokens_per_request |
There was a problem hiding this comment.
Do we also need to check that tokens_per_request <= requests_per_second ?
Given that we are treating requests_per_second as the max bucket size, we'd never consume tokens if that condition doesn't hold, right?
| return input | ||
|
|
||
| return ( | ||
| RunnableLambda(_rate_limited_passthrough).with_config({"name": "Rate Limit"}) |
There was a problem hiding this comment.
You could also just have the function name and docstring be prettier to make the default name nice. The function is rendered in the metadata
3 participants
This PR adds a simple rate limiter based on a token bucket.
I would love to extend RunnableBinding with this, we just need to make
sure there's no funny async vs. threading business.
This should be sufficient for benchmarking for now.