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

Add comment about NoDelay=true in systemd docs #3281

Merged
merged 2 commits into from Jan 15, 2024
Merged

Conversation

evenreven
Copy link
Contributor

@evenreven evenreven commented Nov 20, 2023

Description

Since NoDelay apparently is no longer the default, the example in the systemd docs should also reflect this. This follows up #2631.

If the line should be removed instead of commented out (or if the added comment is unclear or unhelpful), please let me know!

(This is my first PR here, and I hope I've ticked the right boxes. Sorry if I overlooked anything!)

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.

Since NoDelay is no longer the default, the example in the systemd docs should reflect this.
docs/systemd.md Outdated Show resolved Hide resolved
@dentarg
Copy link
Member

dentarg commented Nov 26, 2023

Another option could be to just remove this line and trust that people using the low_latency option and systemd, understands that they should also use the NoDelay option in systemd, as the Puma docs do mention TCP_NODELAY:

puma/lib/puma/dsl.rb

Lines 256 to 258 in 32e011a

# * Set whether to optimize for low latency instead of throughput with
# +low_latency+, default is to not optimize for low latency. This is done
# via +Socket::TCP_NODELAY+.

@nateberkopec nateberkopec added systemd waiting-for-changes Waiting on changes from the requestor labels Jan 2, 2024
@nateberkopec
Copy link
Member

Poke @evenreven

Co-authored-by: Patrik Ragnarsson <patrik@starkast.net>
@evenreven
Copy link
Contributor Author

Sorry for the delay, @dentarg and @nateberkopec! I committed @dentarg's suggestion now, but I'd be open to just remove the lines altogether if you prefer that. That's your call.

@dentarg dentarg changed the title [ci skip] Comment out NoDelay=true in puma.socket example Add comment about NoDelay=true in systemd docs Jan 15, 2024
@dentarg dentarg merged commit 6f6a850 into puma:master Jan 15, 2024
10 checks passed
@dentarg dentarg added docs and removed waiting-for-changes Waiting on changes from the requestor labels Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants