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

net: FIN_ACK takes a long time with Go #66072

Closed
withinboredom opened this issue Mar 2, 2024 · 5 comments
Closed

net: FIN_ACK takes a long time with Go #66072

withinboredom opened this issue Mar 2, 2024 · 5 comments
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@withinboredom
Copy link

withinboredom commented Mar 2, 2024

Go version

<=1.22

Output of go env in your module/workspace:

Generic Linux

What did you do?

Depending on how load tests calculate "time to complete request", Go tends to either perform well, poorly or somewhere in the middle. It took quite a while to figure out what is happening, but I have been unable to determine WHY it is happening, and I'm beyond my expertise here, so I'm handing this off to you guys in the hopes that someone smarter than me can figure it out.

What did you see happen?

When under load, Go tends to delay FIN_ACK packets by up to ~600ms. Other languages (C was mainly compared, though also Rust) do not seem to have this issue. This can skew load tests if they are waiting for the FIN_ACK packet before sending another request, which some load testing tools appear to do (hey, was the tool this was noticed with).

This was measured via performing load tests and taking a packet capture, then examining the time between FIN and FIN_ACK packets (connections were not reused).

What did you expect to see?

I expected to see FIN_ACK packets be emitted in a more reasonable amount of time (0-50ms).

@seankhliao seankhliao changed the title FIN_ACK takes a long time with Go net: FIN_ACK takes a long time with Go Mar 2, 2024
@seankhliao
Copy link
Member

please show a reproducer for the issue.

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 2, 2024
@withinboredom
Copy link
Author

I mean, if you're serious:

package main

import (
	"fmt"
	"net/http"
)

func handler(w http.ResponseWriter, r *http.Request) {
	fmt.Fprint(w, "Hello World")
}

func main() {
	http.HandleFunc("/", handler)
	http.ListenAndServe(":8080", nil)
}

should reproduce the issue just fine.

@Jorropo
Copy link
Member

Jorropo commented Mar 4, 2024

Can you please post a packet capture of what you are seeing (.pcap, .pcapng or anything that can be opened in wireshark are ideal) and or the benchmark tool that reports 600ms so someone can see the 600ms results you are seeing on their machine ?
The code you sent implements HTTP 1.1 and net/http tries really hard to not rely on FIN for signaling the end of requests (ideally we use Content-Length to signal end of requests, else chunked encoding is used, and then if neither are possible then we close the TCP connection).

It is important to eliminate that we are not waiting ~600ms because the HTTP server wants to allow the client to reuse the TCP stream to send an other HTTP request which a non HTTP 1.1 compliant benchmarking tool would incorrectly understand as the server being slow.

@withinboredom
Copy link
Author

@Jorropo:

Can you please post a packet capture of what you are seeing

I can get some more packet captures, I didn't keep them around.

The code you sent implements HTTP 1.1 and net/http tries really hard to not rely on FIN for signaling the end of requests (ideally we use Content-Length to signal end of requests, else chunked encoding is used, and then if neither are possible then we close the TCP connection).

It appears to be just about any socket, h2, grpc, etc (though I haven't tested raw tcp sockets specifically).

Thanks for pointing out content-length, I'll give that a test as well to see if it improves things. It's also a bit worrisome since once a stream is closed you aren't getting any more data out of it, even if content-length says there is more data.

It is important to eliminate that we are not waiting ~600ms because the HTTP server wants to allow the client to reuse the TCP stream to send an other HTTP request which a non HTTP 1.1 compliant benchmarking tool would incorrectly understand as the server being slow.

I may be wrong, as I'm not a network engineer, by any means, but IIRC, once a FIN packet is sent, the client cannot send any more data even if it wanted to; at least on that connection.

I'll write some low-level clients in C so we can better test this as well. I'd also like to rule out network hardware, if at all possible.

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Mar 22, 2024
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants