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

Notebooks 2.0 // Backend // add a rate limiting #131

Open
ederign opened this issue Nov 18, 2024 · 8 comments
Open

Notebooks 2.0 // Backend // add a rate limiting #131

ederign opened this issue Nov 18, 2024 · 8 comments
Labels
area/backend area - related to backend components

Comments

@ederign
Copy link
Member

ederign commented Nov 18, 2024

Respond to status with information about "retry after time" so we can implement rate liming in the future.

Copy link

@ederign: The label(s) /label project/notebooks-v2 cannot be applied. These labels are supported: tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, lifecycle/needs-triage. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label project/notebooks-v2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ederign
Copy link
Member Author

ederign commented Jan 16, 2025

/label project/notebooks-v2

Copy link

@ederign: The label(s) /label project/notebooks-v2 cannot be applied. These labels are supported: tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, lifecycle/needs-triage. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label project/notebooks-v2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

@ederign: The label(s) area/project/notebooks-v2 cannot be applied, because the repository doesn't have them.

In response to this:

/area project/notebooks-v2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ederign
Copy link
Member Author

ederign commented Jan 16, 2025

/area backend

@google-oss-prow google-oss-prow bot added the area/backend area - related to backend components label Jan 16, 2025
@milinddethe15
Copy link

Hi, I propose to work on this issue.
My idea is to add a middleware RateLimiter(next http.Handler) http.Handler to wrap the app.Routes() which will track client (based on their IP?) and denies requests exceeding limit within a time frame and responds with 429 Too Many Requests.
Assign me if good to go.

@thesuperzapper
Copy link
Member

thesuperzapper commented Feb 4, 2025

I'm not sure that IP address can identify the client, as it's very common to have a proxy in front of the kubeflow gateway, and we don't really make any requirements about setting any "x forwarded for" headers.

I'm really not sure if we can successfully do this, because we can't identify clients.

The main thing we can do is probably just ensure that we're not overloading the kubernetes cluster, and we're already doing that largely (by using controller-runtime client so get/lists are cached).

@thesuperzapper
Copy link
Member

I think there's already an issue for it, but we should probably consider doing some kind of caching (at multiple layers of the stack), as that will probably be more useful.

That is, if two people make the exact same list request within a few seconds of each other they should just be given the same answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend area - related to backend components
Projects
Status: In Discussion
Development

No branches or pull requests

3 participants