tcp_proxy: Add max_downstream_connection_duration_jitter_percentage#40999
tcp_proxy: Add max_downstream_connection_duration_jitter_percentage#40999phlax merged 10 commits intoenvoyproxy:mainfrom
Conversation
|
There are (seemingly) unrelated tests failing in the CI. Can we re-run them? |
|
Please merge main, there was a flakey test added and then reverted. |
af53fd9 to
0d23c9b
Compare
|
Had to force push to add sign-off. |
e85e186 to
a38c718
Compare
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
Had to merge main, could I ret a re-review? |
api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto
Outdated
Show resolved
Hide resolved
|
|
||
| // Percentage-based jitter for max_downstream_connection_duration. The jitter will be added | ||
| // randomly to the max_downstream_connection_duration. | ||
| // This field is ignored if max_downstream_connection_duration is not set. |
There was a problem hiding this comment.
OOC what are the tradeoffs of ignoring the field vs. rejecting the config (invalid configuration)?
There was a problem hiding this comment.
No real huge down or upside really, just wanted to avoid rejecting the config unnecessarily.
9abbd03 to
eb02acc
Compare
|
CC @envoyproxy/runtime-guard-changes: FYI only for changes made to |
Signed-off-by: lucaschimweg <schimweg@uber.com>
Signed-off-by: lucaschimweg <schimweg@uber.com>
Signed-off-by: lucaschimweg <schimweg@uber.com>
Co-authored-by: Adi (Suissa) Peleg <adip@google.com> Signed-off-by: Luca Schimweg <luca@schimweg.net>
Co-authored-by: Adi (Suissa) Peleg <adip@google.com> Signed-off-by: Luca Schimweg <luca@schimweg.net>
Signed-off-by: lucaschimweg <schimweg@uber.com>
Signed-off-by: lucaschimweg <schimweg@uber.com>
eb02acc to
9fb0969
Compare
Signed-off-by: lucaschimweg <schimweg@uber.com>
|
Sorry for force push, had to sign-off a previous commit after merging main which caused issues |
|
I don't see any changes in the deps so singing-off. /lgtm deps |
|
no relevant owners for "deps" |
|
CI for Envoy/Prechecks and Mobile/CC seems to be stuck (running for 24h now), anything we can do about that? |
|
/retest |
|
looks like some github issue - i think you will need to merge main or similar to kick it |
|
Hm. That doesn't seem to have done anything. I'd try adding an empty commit to see if it triggers. |
Signed-off-by: lucaschimweg <schimweg@uber.com>
|
It's ready to merge, but I don't have write access. Can someone with access merge? |
Commit Message: tcp_proxy: Add max_downstream_connection_duration_jitter_percentage
Additional Description: Adds proto for a new field that can be used to add a jitter to the max_downstream_connection_duration. This allows to prevent thundering herd problems.
Risk Level: Low (adds new feature not enabled by default)
Testing: Unit tests & Integration tests added, Functionality manually verified locally
Docs Changes: Added API docs
Release Notes: Added
Platform Specific Features: n/a
Fixes #40686
Optional API Considerations: Considered