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 keep alive to tcp plugin #3961

Closed

Conversation

dimitarKiryakov
Copy link

Which issue(s) this PR fixes:
Fixes #3960

What this PR does / why we need it:
TCP keepalive should be supported by in_tcp to avoid dead connection, as same as in_forward

Docs Changes:
send_keepalive_packet should be added to https://docs.fluentd.org/input/tcp
Release Note:
Same with the title

@fujimotos fujimotos self-requested a review November 15, 2022 11:44
@fujimotos fujimotos added the enhancement Feature request or improve operations label Nov 15, 2022
@dimitarKiryakov
Copy link
Author

@fujimotos could you please approve running the workflows?

@fujimotos
Copy link
Member

fujimotos commented Nov 15, 2022

@dimitarKiryakov Thank you for contribution!

TCP keepalive should be supported by in_tcp to avoid dead connection, as same as in_forward

I eyeballed your patch. It seems good, but I want to do a bit of more
testing on it tomorrow. Wait for me.

Could you please approve running the workflows?

BTW, I approved the workflow run. The tests have just started running.

Signed-off-by: I326463 <[email protected]>
@dimitarKiryakov
Copy link
Author

@fujimotos Thanks a lot for your help on this. This feature is very crucial for us as it will mitigate the issue of dead connections. I think that the currently failing tests are not related to my change. Did you manage to test it?

Thank you a lot!

Copy link
Member

@fujimotos fujimotos left a comment

Choose a reason for hiding this comment

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

Approved. I can confirm this patch works.

@fujimotos
Copy link
Member

Here is how I tested this patch. First, I created a simple TCP server
using the configuration attached below:

$ fluentd -c tcp.conf

I initiated a TCP connection to the server in another terminal, and kept
it open for a while:

$ telnet localhost 9000

Then I monitored the network stream using tcpdump:

$ tcpdump
...
14:20:21.470600 IP 127.0.0.1.40446 > 127.0.0.1.9000: Flags [.], ack 1, win 512, options [nop,nop,TS val 1282790546 ecr 1282790546], length 0
14:21:22.182178 IP 127.0.0.1.9000 > 127.0.0.1.40446: Flags [.], ack 1, win 512, options [nop,nop,TS val 1282851258 ecr 1282790546], length 0
14:21:22.182196 IP 127.0.0.1.40446 > 127.0.0.1.9000: Flags [.], ack 1, win 512, options [nop,nop,TS val 1282851258 ecr 1282790546], length 0
14:22:23.622148 IP 127.0.0.1.9000 > 127.0.0.1.40446: Flags [.], ack 1, win 512, options [nop,nop,TS val 1282912698 ecr 1282851258], length 0

As shown above, keepalive packets are being transmitted after some
period of inactivity. So I believe the feature is working as expected.

tcp.conf

<source>
  @type tcp
  port 9000
  format none
  tag "test.log"
  send_keepalive_packet true
</source>
<match test.**>
  @type stdout
</match>

@fujimotos
Copy link
Member

Merged via 66c4a8b.

Note: I described more details in the commit message and also
added a little tweak to the test case.

@fujimotos fujimotos closed this Nov 22, 2022
fujimotos pushed a commit to fujimotos/fluentd-docs-gitbook that referenced this pull request Nov 22, 2022
fujimotos pushed a commit to fluent/fluentd-docs-gitbook that referenced this pull request Nov 22, 2022
@fujimotos
Copy link
Member

Documentation added in fluent/fluentd-docs-gitbook#438

@ashie ashie added this to the v1.16.0 milestone Mar 29, 2023
@ashie ashie mentioned this pull request Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants