proxyd: Custom rate limiter implementation#3679
Conversation
|
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
|
This PR changes implementation code, but doesn't include a changeset. Did you forget to add one? |
mslipper
force-pushed
the
10-09-proxyd_Custom_rate_limiter_implementation
branch
from
ada9def to
f711c48
Compare
Inphi
left a comment
Inphi
left a comment
There was a problem hiding this comment.
Have some comments to improve the implementation. But they aren't blockers.
Our current proxyd deployment does not share rate limit state across multiple servers within a backend group. This means that rate limits on the public endpoint are artifically high. This PR adds a Redis-based rate limiter to fix this problem. While our current rate limiting library (github.com/sethvargo/go-limiter) _does_ support Redis, the client library it uses is not type safe, is less performant, and would require us to update the other places we use Redis. To avoid these issues, I created a simple rate limiting interface with both Redis and memory backend. Note that this PR only adds the new implementations - it does not integrate them with the rest of the codebase. I'll do that in a separate PR to make review easier.
mslipper
force-pushed
the
10-09-proxyd_Custom_rate_limiter_implementation
branch
from
f711c48 to
c2b8efa
Compare
2 participants
Our current proxyd deployment does not share rate limit state across multiple servers within a backend group. This means that rate limits on the public endpoint are artifically high.
This PR adds a Redis-based rate limiter to fix this problem. While our current rate limiting library (github.com/sethvargo/go-limiter) does support Redis, the client library it uses is not type safe, is less performant, and would require us to update the other places we use Redis. To avoid these issues, I created a simple rate limiting interface with both Redis and memory backend.
Note that this PR only adds the new implementations - it does not integrate them with the rest of the codebase. I'll do that in a separate PR to make review easier.