Skip to content

clarify destination rule docs #2428

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.

Sign up for GitHub

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

Merged
istio-testing merged 3 commits into from
Jul 29, 2022
Merged

clarify destination rule docs #2428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eba8217
clarify destination rule docs
ramaraochavali Jul 23, 2022
915cfd1
review comments
ramaraochavali Jul 25, 2022
a81990b
update docs
ramaraochavali Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 14 additions & 26 deletions kubernetes/customresourcedefinitions.gen.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions networking/v1alpha3/destination_rule.gen.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions networking/v1alpha3/destination_rule.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions networking/v1alpha3/destination_rule.pb.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions networking/v1alpha3/destination_rule.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,10 +709,15 @@ message ConnectionPoolSettings {

// Settings applicable to HTTP1.1/HTTP2/GRPC connections.
message HTTPSettings {
// Maximum number of pending HTTP requests to a destination. Default 2^32-1.
// Maximum number of requests that will be queued while waiting for
// a ready connection pool connection. Default 1024.
// Refer to https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/circuit_breaking
// under which conditions a new connection is created for HTTP2.
// Please note that this is applicable to both HTTP/1.1 and HTTP2.
int32 http1_max_pending_requests = 1;

// Maximum number of requests to a backend. Default 2^32-1.
// Maximum number of active requests to a destination. Default 1024.
// Please note that this is applicable to both HTTP/1.1 and HTTP2.
int32 http2_max_requests = 2;
Copy link
Member

@howardjohn howardjohn Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so is it identical to http1_max_pending_requests then

Copy link
Contributor Author

@ramaraochavali ramaraochavali Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not exactly same. http2_max_requests is how many concurrent active requests can be there. max_pending_requests is how many requests can be in pending queue waiting for connection.

Specifically for pending requests,
"For HTTP/2 connections, if max concurrent streams and max requests per connection are not configured, all requests will be multiplexed over the same connection so this circuit breaker will only be hit when no connection is already established"

So there is subtle difference on how http2 protocol options are setup.

Copy link
Member

@howardjohn howardjohn Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we clarify this difference in our api comments?

Copy link
Contributor Author

@ramaraochavali ramaraochavali Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. good point. Let me update it.

Copy link
Contributor Author

@ramaraochavali ramaraochavali Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the docs. Defaults are also incorrect. Fixed them


// Maximum number of requests per connection to a backend. Setting this
Expand Down
4 changes: 2 additions & 2 deletions networking/v1beta1/destination_rule.gen.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions networking/v1beta1/destination_rule.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions networking/v1beta1/destination_rule.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,15 @@ message ConnectionPoolSettings {

// Settings applicable to HTTP1.1/HTTP2/GRPC connections.
message HTTPSettings {
// Maximum number of pending HTTP requests to a destination. Default 2^32-1.
// Maximum number of requests that will be queued while waiting for
// a ready connection pool connection. Default 1024.
// Refer to https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/circuit_breaking
// under which conditions a new connection is created for HTTP2.
// Please note that this is applicable to both HTTP/1.1 and HTTP2.
int32 http1_max_pending_requests = 1;

// Maximum number of requests to a backend. Default 2^32-1.
// Maximum number of active requests to a destination. Default 1024.
// Please note that this is applicable to both HTTP/1.1 and HTTP2.
int32 http2_max_requests = 2;

// Maximum number of requests per connection to a backend. Setting this
Expand Down