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.

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

http server: fix regression set backlog to 1024 #718

Merged
merged 1 commit into from
Mar 28, 2022
Merged

Conversation

niklasad1
Copy link
Member

@niklasad1 niklasad1 commented Mar 26, 2022

Closing #716

@niklasad1 niklasad1 requested a review from a team as a code owner March 26, 2022 11:28
Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

LGTM!

Any idea what the negative effects of a higher backlog is? As a followup I wonder whether it's worth making this configurable?

@niklasad1
Copy link
Member Author

niklasad1 commented Mar 27, 2022

Any idea what the negative effects of a higher backlog is?

I don't think there any downsides with a bigger backlog rather than that the kernel might use more memory and potential DOS for big values (also the OS might truncate this value).

For instance the max backlog size for Linux is 4096 and it is highly platform dependent (BSD uses one queue for both "ESTABLISHED" and "SYN waiting to be ACK:ed"; Linux uses two different queues for "ESTABLISHED" and "SYN waiting to be ACK:ed") so yes we should make this configurable for use but 1024 seems like more reasonable default value for instance tokio/mio and paritytech/jsonrpc uses 1024 already for the backlog. Perhaps even 4096 is reasonable here considering the BSD implementation.....

As a followup I wonder whether it's worth making this configurable?

Yes, let's take care of that in another PR.

@lexnv
Copy link
Contributor

lexnv commented Mar 28, 2022

I wonder if this is capped by kern.ipc.somaxconn, currently the MacOS default is kern.ipc.somaxconn=128.

@niklasad1 niklasad1 merged commit 054c0e3 into master Mar 28, 2022
@niklasad1 niklasad1 deleted the na-fix-712 branch March 28, 2022 18:48
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.

4 participants