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

socket_listener & socket_writer should be able to control tcp keep alive #2635

Closed
phemmer opened this issue Apr 7, 2017 · 0 comments
Closed

Comments

@phemmer
Copy link
Contributor

phemmer commented Apr 7, 2017

Feature Request

Proposal:

Users should be able to control TCP keep alive for the socket_listener and socket_writer plugins.

Current behavior:

Telegraf inherits the kernel TCP keep alive settings.

Desired behavior:

Telegraf should be able to set its own TCP keep alive settings.

Use case: [Why is this important (helps with prioritizing requests)]

TCP keep alive is very important on socket_listener, as TCP keep alives are the only way to detect when a connection which is read-only is ungracefully terminated (remote host crash, network loss, etc). Without TCP keep alive, and the ability to detect such issues, telegraf may accumulate a large number of dead connections, and exhaust its available file descriptors.
By default TCP keep alive is off (in the kernel). Even if turned on, it's possible telegraf may need to be configured with different settings.

I imagine configuration would be done with a simple tcp_keepalive_interval setting. When set > 0, TCP keep alive is set on at the given interval value, when set = 0, TCP keep alive is turned off, and then when not set at all, we use the OS default.
In order to detect all 3 states, in the code the struct member will have to be a pointer. We could simplify it and just say that when 0, or unset, we inherit the OS default. I can't think of any reason why one would have TCP keep alive enabled on the kernel, and then want to turn it off in the application. But I also don't like making assumptions about use cases I can't think of.

I'll work on this, as it'll be a very easy change. Just creating the issue as a placeholder.

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

No branches or pull requests

1 participant