Skip to content

feat: Add support for configuring maxConnectionsToAcceptPerSocketEvent in ClientTrafficPolicy#6233

Merged
rudrakhp merged 23 commits intoenvoyproxy:mainfrom
jukie:api-connections-per-socket-event
Jun 21, 2025
Merged

feat: Add support for configuring maxConnectionsToAcceptPerSocketEvent in ClientTrafficPolicy#6233
rudrakhp merged 23 commits intoenvoyproxy:mainfrom
jukie:api-connections-per-socket-event

Conversation

@jukie
Copy link
Contributor

@jukie jukie commented May 29, 2025

What type of PR is this?

feat: Add support for configuring maxConnectionsToAcceptPerSocketEvent in Envoy listener and set default to 1

What this PR does / why we need it:
Under high load the default maxConnectionsToAcceptPerSocketEvent could cause performance issues (ref envoyproxy/envoy#19103 (comment)). It's recommended to configure a limit vs the default unlimited and a value of 1 can be safely applied without impact.

Which issue(s) this PR fixes:

Fixes #6232

Release Notes: Yes

jukie and others added 7 commits May 29, 2025 15:59
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie jukie marked this pull request as ready for review May 29, 2025 22:47
@jukie jukie requested a review from a team as a code owner May 29, 2025 22:47
@jukie jukie changed the title wip feat: Add support for configuring maxConnectionsToAcceptPerSocketEvent in ClientTrafficPolicy May 29, 2025
@codecov
Copy link

codecov bot commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.76%. Comparing base (e1c0474) to head (1841550).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6233      +/-   ##
==========================================
+ Coverage   70.72%   70.76%   +0.03%     
==========================================
  Files         220      220              
  Lines       37096    37103       +7     
==========================================
+ Hits        26236    26255      +19     
+ Misses       9320     9310      -10     
+ Partials     1540     1538       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jukie
Copy link
Contributor Author

jukie commented May 29, 2025

/retest

jukie and others added 2 commits May 30, 2025 09:39
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie jukie requested a review from rudrakhp May 30, 2025 15:40
rudrakhp
rudrakhp previously approved these changes May 30, 2025
@jukie
Copy link
Contributor Author

jukie commented May 30, 2025

/retest

@jukie
Copy link
Contributor Author

jukie commented May 31, 2025

/retest

Signed-off-by: Isaac <10012479+jukie@users.noreply.github.com>
Copy link
Member

@rudrakhp rudrakhp left a comment

Choose a reason for hiding this comment

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

Minor suggestion, rest LGTM!

jukie and others added 2 commits June 3, 2025 09:24
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie jukie requested a review from rudrakhp June 3, 2025 15:39
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie
Copy link
Contributor Author

jukie commented Jun 3, 2025

/retest

rudrakhp
rudrakhp previously approved these changes Jun 3, 2025
@rudrakhp rudrakhp requested review from a team June 3, 2025 18:42
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
@rudrakhp rudrakhp requested review from a team June 13, 2025 15:09
@zirain
Copy link
Member

zirain commented Jun 13, 2025

/retest

@zirain
Copy link
Member

zirain commented Jun 17, 2025

/retest

@jukie
Copy link
Contributor Author

jukie commented Jun 20, 2025

/retest

@zirain
Copy link
Member

zirain commented Jun 21, 2025

/retest

@rudrakhp
Copy link
Member

/retest

@rudrakhp rudrakhp merged commit 3e14c49 into envoyproxy:main Jun 21, 2025
49 of 53 checks passed
@jukie jukie deleted the api-connections-per-socket-event branch June 22, 2025 22:38
@arkodg
Copy link
Contributor

arkodg commented Jun 23, 2025

@jukie is the use case to set it to 1 ?
imo in that case its fine to set the default value to 1 w/o exposing an API which is also the recommendation from upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configure default max_connections_to_accept_per_socket_event=1 or expose configuration

4 participants