[HOTFIX] Add timeouts to telemetry streams and close/reset streams from write side #7979
Conversation
nholland94
added
the
ci-build-me
|
|
||
| <-time.After(400 * time.Millisecond) | ||
|
|
||
| err = s.Reset() |
There was a problem hiding this comment.
From my understanding of the documentation, reset tells the other side of the stream to hangup and go away. Closing a stream closes our side of the stream, but the connection doesn't get GC'd until the other side closes as well. A reset forcibly tells the other side to close and directly drops the connection.
| @@ -1225,6 +1225,8 @@ type getPeerTelemetryDataMsg struct { | |||
| } | |||
|
|
|||
| func (m *getPeerTelemetryDataMsg) run(app *app) (interface{}, error) { | |||
| ctx, _ := context.WithTimeout(app.Ctx, 400*time.Millisecond) | |||
There was a problem hiding this comment.
Probably this is long enough but just noting on other streams the timeout is 30 seconds (maybe it should be lower like this on those too though).
There was a problem hiding this comment.
I can definitely bump this if we don't think 400ms is enough. I was figuring that we expect these to respond quickly and we don't want to keep these connections open as long.
|
!approved-for-mainnet |
nholland94
force-pushed
the
fix/telemetry-streams
branch
from
5728b5f
to
2eed78b
Compare
nholland94
changed the title
No description provided.