-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Labels
Area: TransportIncludes HTTP/2 client/server and HTTP server handler transports and advanced transport features.Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features.Status: Help WantedType: Bug
Description
The deleteStream method can be called multiple times due to stream cancellation, timeout expiry and the handler function returning. This can result in the overcounting the number of stream failures/successes.
grpc-go/internal/transport/http2_server.go
Lines 1306 to 1323 in 5ed7cf6
func (t *http2Server) deleteStream(s *ServerStream, eosReceived bool) { | |
t.mu.Lock() | |
if _, ok := t.activeStreams[s.id]; ok { | |
delete(t.activeStreams, s.id) | |
if len(t.activeStreams) == 0 { | |
t.idle = time.Now() | |
} | |
} | |
t.mu.Unlock() | |
if channelz.IsOn() { | |
if eosReceived { | |
t.channelz.SocketMetrics.StreamsSucceeded.Add(1) | |
} else { | |
t.channelz.SocketMetrics.StreamsFailed.Add(1) | |
} | |
} | |
} |
To fix this we should only increment the metrics when the stream is found in the activeStreams
map. We should also add a test to verify this behaviour.
Metadata
Metadata
Assignees
Labels
Area: TransportIncludes HTTP/2 client/server and HTTP server handler transports and advanced transport features.Includes HTTP/2 client/server and HTTP server handler transports and advanced transport features.Status: Help WantedType: Bug