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.

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

fix envoy config to avoid overflows #309

Merged
merged 4 commits into from
Apr 3, 2018
Merged

fix envoy config to avoid overflows #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
134bc58
fix envoy config to avoid overflows
Lookyan Mar 31, 2018
77c46bd
fix indentation in config
Lookyan Mar 31, 2018
47f96ef
fix config test
Lookyan Mar 31, 2018
5437017
Merge branch 'master' into fix-overflow
davecheney Apr 3, 2018
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
12 changes: 12 additions & 0 deletions internal/envoy/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ static_resources:
port_value: {{ if .XDSGRPCPort }}{{ .XDSGRPCPort }}{{ else }}8001{{ end }}
lb_policy: ROUND_ROBIN
http2_protocol_options: {}
circuit_breakers:
thresholds:
- priority: high
max_connections: 100000
max_pending_requests: 100000
max_requests: 60000000
max_retries: 50
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this value. What I see in testing is

contour::127.0.0.1:8001::cx_active::1
contour::127.0.0.1:8001::cx_connect_fail::2
contour::127.0.0.1:8001::cx_total::3
contour::127.0.0.1:8001::rq_active::514
contour::127.0.0.1:8001::rq_error::4
contour::127.0.0.1:8001::rq_success::0
contour::127.0.0.1:8001::rq_timeout::0
contour::127.0.0.1:8001::rq_total::518
contour::127.0.0.1:8001::health_flags::healthy
contour::127.0.0.1:8001::weight::1
contour::127.0.0.1:8001::region::
contour::127.0.0.1:8001::zone::
contour::127.0.0.1:8001::sub_zone::
contour::127.0.0.1:8001::canary::false
contour::127.0.0.1:8001::success_rate::-1

518 in progress connections, but nothing has timed out yet. Retrying is fine, even retrying indefinitely is fine, but I think a reasonably short connection timeout, say < 5 seconds, should be applied to each attempt.

It doesn't matter so much for this change as we're going to embiggen the max incoming connections on the server side in #308, but we should revisit it in #313

- priority: default
max_connections: 100000
max_pending_requests: 100000
max_requests: 60000000
max_retries: 50
admin:
access_log_path: {{ if .AdminAccessLogPath }}{{ .AdminAccessLogPath }}{{ else }}/dev/null{{ end }}
address:
Expand Down
12 changes: 12 additions & 0 deletions internal/envoy/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@ static_resources:
port_value: 8001
lb_policy: ROUND_ROBIN
http2_protocol_options: {}
circuit_breakers:
thresholds:
- priority: high
max_connections: 100000
max_pending_requests: 100000
max_requests: 60000000
max_retries: 50
- priority: default
max_connections: 100000
max_pending_requests: 100000
max_requests: 60000000
max_retries: 50
admin:
access_log_path: /dev/null
address:
Expand Down