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

disable connection linger timeout by default #3576

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

fwbrasil
Copy link
Contributor

@fwbrasil fwbrasil commented Mar 7, 2024

Problem

Tapir sets the SO_LINGER socket option by default. This config holds the connection for some time after it's closed and Netty uses a global thread pool (GlobalEventExecutor) with a single ephemeral thread to delay the connection close. Netty doesn't set this config by default and I couldn't identify a mechanism to make it use another thread pool.

I noticed this issue when benchmarking with a high rate of short-lived connections, which ended up generating a backlog in this thread pool and memory pressure. The GlobalEventExecutor also keeps constantly tearing down its thread and creating new ones, which introduces additional overhead.

Solution

Don't set the configuration by default.

Notes

  • There can be cases where the config makes sense but I'd say it's important that it is done by an expert considering the characteristics of specific systems.

@adamw
Copy link
Member

adamw commented Mar 7, 2024

I'm trying to understand more about the setting, and it seems that by default SO_LINGER should be set to a non-zero value. What they recommend on SO is having the client initiate connection close - which, I guess, happens in HTTP (unless there's some error server-side)? But then I don't understand why your benchmark would behave the way it does. Anybody knows more about sockets & linger? ;)

@kciesielski
Copy link
Member

@adamw I read the same thread and this response seems interesting. It suggests that there's a distinction between using netty as a client and as a server, and in the latter case disabling linger seems ok?

@adamw
Copy link
Member

adamw commented Mar 7, 2024

@kciesielski Ah makes sense, thanks! :)

@adamw adamw merged commit 6613efd into softwaremill:master Mar 7, 2024
11 checks passed
@fwbrasil fwbrasil deleted the linger branch March 7, 2024 15:38
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.

3 participants