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

Allow backlog parameter to be set with ssl_bind DSL #2780

Merged
merged 6 commits into from Jan 1, 2022

Conversation

dalibor
Copy link
Contributor

@dalibor dalibor commented Dec 24, 2021

Description

We're adding option for setting the socket backlog value with the ssl_bind DSL, and updating the docs with a hint to the net.core.somaxconn sysctl value. What's interesting here is that on older Linux kernels it defaults to 128 (reference), so the actual backlog value is capped to it.

somaxconn - INTEGER
Limit of socket listen() backlog, known in userspace as SOMAXCONN. Defaults to 4096. (Was 128 before linux-5.4) See also tcp_max_syn_backlog for additional tuning for TCP sockets.

Even with the default backlog value of 1024 (when kernel allows it), we are seeing some requests get dropped (504 errors with AWS ALB). That happens with a super fast end-points that respond in < 5ms occasionally when receiving a blast of requests within few seconds and there are other slower end-points that saturate the queue causing some requests to get dropped. So, I'm tweaking slightly the original doc about backlog that were misleading:

Generally, you'll never hit the backlog cap in production.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@nateberkopec
Copy link
Member

@nateberkopec nateberkopec commented Jan 1, 2022

Makes total sense. Not having this set on ssl_bind was an oversight and I agree with your docs changes as well.

@MSP-Greg MSP-Greg merged commit 4ac1448 into puma:master Jan 1, 2022
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants