Add support for load-shedding. #1616
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the per-connection service is not ready this gives an option of immediately rejecting requests.
|
Setting @abizjak did you get a chance to work on this issue? |
djc
reviewed
Aug 19, 2024
| @@ -73,7 +73,7 @@ h2 = {version = "0.3.17", optional = true} | |||
| hyper = {version = "0.14.26", features = ["full"], optional = true} | |||
| hyper-timeout = {version = "0.4", optional = true} | |||
| tokio-stream = "0.1" | |||
| tower = {version = "0.4.7", default-features = false, features = ["balance", "buffer", "discover", "limit", "load", "make", "timeout", "util"], optional = true} | |||
| tower = {version = "0.4.7", default-features = false, features = ["balance", "buffer", "discover", "limit", "load", "make", "timeout", "util", "load-shed"], optional = true} | |||
There was a problem hiding this comment.
Let's keep the features here in alphabetic order?
| let err = match err.downcast::<tower::load_shed::error::Overloaded>() { | ||
| Ok(_) => { | ||
| return Ok(Status::resource_exhausted( | ||
| "Too many active requests for the connection", |
There was a problem hiding this comment.
I think the majority of our messages are lowercase, so please match that here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add an option to enable load shedding for the service constructed for the connection.
Motivation
In the server we are implementing we want to load-shed when the service is overloaded, and we want to limit the number of concurrent requests per connection. We use tower::load_shed layer for load shedding, but this only applies to the entire service.
If we additionally apply the per-connection concurrency limit the issue is that this limit is typically hit first and so the service responds with "not ready". As a result the load-shed layer of the service is never reached meaning that requests just queue up.
Solution
The solution is to optionally add a load-shed layer to the per-connection service constructed by tonic.