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

Feature Request: Allow updating the limit #69

Closed
prathik opened this issue Mar 1, 2021 · 8 comments
Closed

Feature Request: Allow updating the limit #69

prathik opened this issue Mar 1, 2021 · 8 comments

Comments

@prathik
Copy link

prathik commented Mar 1, 2021

Goal: To use this library with an adaptive rate limiter algorithm like Vegas or AIMD

Problem: Right now the library exposes no way to update the rates dynamically.

There are two ways to solve for this

  1. Create a new rate limiter every time I want to update limits
  2. Dynamically update the rate limit on the object

Would like feedback on which is the preferred approach. Or if there is any other way.

@rabbbit
Copy link
Contributor

rabbbit commented Mar 1, 2021

Hi,

Would you elaborate on the use case for this? Do you have a project already, or is it a theoretical interest?

@prathik
Copy link
Author

prathik commented Mar 2, 2021

@rabbbit I am working on it and evaluating various rate limiters. This is inspired by https://github.com/Netflix/concurrency-limits and wanted to reuse any existing libraries if present.

@xiaoyang-chen
Copy link

xiaoyang-chen commented Mar 2, 2021

Goal: To use this library with an adaptive rate limiter algorithm like Vegas or AIMD

Problem: Right now the library exposes no way to update the rates dynamically.

There are two ways to solve for this

  1. Create a new rate limiter every time I want to update limits
  2. Dynamically update the rate limit on the object

Would like feedback on which is the preferred approach. Or if there is any other way.

I read the go files in this repo, first way is the better way for this ratelimit, I think, because limit is not direct influence to this ratelimit

@prathik
Copy link
Author

prathik commented Mar 16, 2021

@rabbbit If we update the perRequest value in limiter_atomic.go:41 we should be able to dynamically change the rate. Can I go ahead and contribute? Or do you prefer that we stick to create a new ratelimit object when we want to change the rate?

@rabbbit
Copy link
Contributor

rabbbit commented Mar 16, 2021

It's not just perRequest. Since we also store slack, we'd need to update (or throw away?) the actual slack value. We'd also need to do it atomically, so there's extra complexity there.

Is creating a new limiter feasible? If so, that would be my preference.

@prathik
Copy link
Author

prathik commented Mar 17, 2021

@rabbbit sure, I don't see any concerns with it, at worst case scenario we'd be creating a new object every couple of seconds till we hit a stable point. You see any concerns with that?

@prathik
Copy link
Author

prathik commented Mar 17, 2021

@rabbbit BTW I'm implementing Vegas congestion control on top of this library.

@rabbbit
Copy link
Contributor

rabbbit commented Mar 20, 2021 via email

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

3 participants