Skip to content

default maxconnectionstoacceptpersocketevent to a sensible value#55817

Merged
istio-testing merged 3 commits intoistio:masterfrom
ramaraochavali:fix/event_loop
Apr 15, 2025
Merged

default maxconnectionstoacceptpersocketevent to a sensible value#55817
istio-testing merged 3 commits intoistio:masterfrom
ramaraochavali:fix/event_loop

Conversation

@ramaraochavali
Copy link
Copy Markdown
Contributor

@ramaraochavali ramaraochavali commented Apr 5, 2025

See envoyproxy/envoy#38999 and envoyproxy/envoy#19103 (comment)

I think compatibility version is not needed as it is most sensible default but if there are concerns I can add

  • Ambient
  • Configuration Infrastructure
  • Docs
  • Dual Stack
  • Installation
  • Networking
  • Performance and Scalability
  • Extensions and Telemetry
  • Security
  • Test and Release
  • User Experience
  • Developer Infrastructure
  • Upgrade
  • Multi Cluster
  • Virtual Machine
  • Control Plane Revisions

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@ramaraochavali ramaraochavali requested review from a team as code owners April 5, 2025 07:54
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 5, 2025
@ramaraochavali ramaraochavali added the release-notes-none Indicates a PR that does not require release notes. label Apr 5, 2025
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Copy link
Copy Markdown
Contributor

@dgn dgn left a comment

Choose a reason for hiding this comment

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

LGTM and it appears to me (without knowing the details) that this is a sensible default but IMO we will need at least a release note to capture that this default value was changed.

EnableGatewayAPIManualDeployment = env.Register("ENABLE_GATEWAY_API_MANUAL_DEPLOYMENT", true,
"If true, allows users to bind Gateway API resources to existing gateway deployments.").Get()

MaxConnectionsToAcceptPerSocketEvent = func() *wrapperspb.UInt32Value {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: maybe move the proto-specific part to pilot/pkg/networking/core/listener.go. will keep the imports in features pkg tidy


MaxConnectionsToAcceptPerSocketEvent = func() *wrapperspb.UInt32Value {
v := env.Register("MAX_CONNECTIONS_PER_SOCKET_EVENT_LOOP", 1,
"The maximum number of connections to accept from the kernel per socket event.").Get()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"The maximum number of connections to accept from the kernel per socket event.").Get()
"The maximum number of connections to accept from the kernel per socket event. Set this to '0' to accept unlimited connections.").Get()

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@ramaraochavali
Copy link
Copy Markdown
Contributor Author

Good call on release notes. Added.

@ramaraochavali
Copy link
Copy Markdown
Contributor Author

@howardjohn @hzxuzhonghu can you please review as well?

@istio-testing istio-testing merged commit 7164e00 into istio:master Apr 15, 2025
29 checks passed
@ramaraochavali ramaraochavali deleted the fix/event_loop branch April 15, 2025 13:33
fjglira pushed a commit to fjglira/istio that referenced this pull request Sep 26, 2025
* upstream/master:
  Automator: update ztunnel@master in istio/istio@master (istio#55950)
  Automator: update proxy@master in istio/istio@master (istio#55938)
  gateway: refactor event handling (istio#55758)
  initial ClusterTrustBundle v1alpha1 support (istio#55592)
  gateway: add support for using workload certificate (istio#55899)
  krt: implement folder based collections (istio#55337)
  support envVarFrom in istiod chart (istio#55872)
  pilot: fix verified certificate when mtls and referencegrants (istio#55859)
  default maxconnectionstoacceptpersocketevent to a sensible value (istio#55817)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes-none Indicates a PR that does not require release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants