Add timeout to writing to responses #15831
Conversation
In go-gitea#15826 it has become apparent that there are a few occasions when a response can hang during writing, and because there is no timeout go will happily just block interminably. This PR adds a fixed 5 second timeout to all writes to a connection. Fix go-gitea#15826 Signed-off-by: Andrew Thornton <art27@cantab.net>
| @@ -250,6 +253,13 @@ type wrappedConn struct { | |||
| closed *int32 | |||
| } | |||
|
|
|||
| func (w wrappedConn) Write(p []byte) (n int, err error) { | |||
| if PerWriteWriteTimeout > 0 { | |||
There was a problem hiding this comment.
why check? It's a const value, isn't it?
There was a problem hiding this comment.
A const that is labelled so that you can change it at compile time to something else...
There was a problem hiding this comment.
e.g.
LDFLAGS="-X \"code.gitea.io/gitea/modules/graceful.PerWriteWriteTimeout=0\""
or
LDFLAGS="-X \"code.gitea.io/gitea/modules/graceful.PerWriteWriteTimeout=1s\""
GiteaBot
added
the
lgtm/need 2
|
This will break some large repo views, who need long time on first view ..., it could be used temporarly until we can render multithreaded again! |
No it won't This is at the time of writing. |
This is an orthogonal issue to the render multithreaded issue - the singlethreading through the render has revealed that it exists but the problem is still a real issue. |
There was a problem hiding this comment.
For other reviewers: This is not a timeout on TCP connections between devices.
This is (at least on unix systems) a timeout the kernel sets on the connection when writes to the network interface are blocked for whatever reason.
edit: this is a stopgap to #15826, but isn't a fix to the underlying issue. I think we should keep that issue open until we got more feedback
| @@ -28,6 +28,9 @@ var ( | |||
| DefaultMaxHeaderBytes int | |||
| ) | |||
|
|
|||
| // PerWriteWriteTimeout timeout for writes | |||
| const PerWriteWriteTimeout = 5 * time.Second | |||
There was a problem hiding this comment.
5 secs seem a lot; if kernel blocks for that duration the system has bigger problems..
200ms seems to be a lower bound due to write batching with NAGLE (?), I'd go with 500ms?
But as a stopgap to avoid lingering connections a large value seems ok.
There was a problem hiding this comment.
I picked 5s as value everyone could agree meant that the underlying connection is totally blocked and just wasn't coming back. But I have no metric to know what is a good value here as I don't completely understand the situations where this happens.
I'm suspecting that the problem is due to some network reconfiguration that leads to the connection at the go end being disconnected from the client but somehow didn't kill the connection.
GiteaBot
added
lgtm/need 1
|
I suspect it's not actually a stop gap. I think this is a genuine thing - If you don't set a writetimeout go will happily sit there trying to write to a blocked connection indefinitely. Given we have situations like docker, Unix pipes, fcgi, bsds, logrotating proxy servers and so on. I think there are plenty of ways that this could happen. Imagine a situation whereby your network goes down - is the connection closed? It's not really - but the buffer can fill up and then you get write blocking. If you don't have a timeout you never know. It could be that gitea is being told by the system that something is happening - eg through a signal but we're capturing it and ignoring it because we weren't expecting it. I dunno but I think this has been affecting a few people for a while. |
GiteaBot
added
lgtm/done
|
make lgtm work |
Backport go-gitea#15831 In go-gitea#15826 it has become apparent that there are a few occasions when a response can hang during writing, and because there is no timeout go will happily just block interminably. This PR adds a fixed 5 second timeout to all writes to a connection. Fix go-gitea#15826 Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport #15831 In #15826 it has become apparent that there are a few occasions when a response can hang during writing, and because there is no timeout go will happily just block interminably. This PR adds a fixed 5 second timeout to all writes to a connection. Fix #15826 Signed-off-by: Andrew Thornton <art27@cantab.net>
In go-gitea#15826 it has become apparent that there are a few occasions when a response can hang during writing, and because there is no timeout go will happily just block interminably. This PR adds a fixed 5 second timeout to all writes to a connection. Fix go-gitea#15826 Signed-off-by: Andrew Thornton <art27@cantab.net>
In #15826 it has become apparent that there are a few occasions when a response can
hang during writing, and because there is no timeout go will happily just block
interminably. This PR adds a fixed 5 second timeout to all writes to a connection.
Fix #15826
Signed-off-by: Andrew Thornton art27@cantab.net