TUN-8297: Improve write timeout logging on safe_stream.go

## Summary:
In order to properly monitor what is happening with the new write timeouts that we introduced
in TUN-8244 we need proper logging. Right now we were logging write timeouts when the safe
stream was being closed which didn't make sense because it was miss leading, so this commit
prevents that by adding a flag that allows us to know whether we are closing the stream or not.
This commit is contained in:
João "Pisco" Fernandes 2024-03-13 13:30:38 +00:00
parent 47ad3238dd
commit da6fac4133
1 changed files with 13 additions and 3 deletions

View File

@ -4,6 +4,7 @@ import (
"errors" "errors"
"net" "net"
"sync" "sync"
"sync/atomic"
"time" "time"
"github.com/quic-go/quic-go" "github.com/quic-go/quic-go"
@ -19,6 +20,7 @@ type SafeStreamCloser struct {
stream quic.Stream stream quic.Stream
writeTimeout time.Duration writeTimeout time.Duration
log *zerolog.Logger log *zerolog.Logger
closing atomic.Bool
} }
func NewSafeStreamCloser(stream quic.Stream, writeTimeout time.Duration, log *zerolog.Logger) *SafeStreamCloser { func NewSafeStreamCloser(stream quic.Stream, writeTimeout time.Duration, log *zerolog.Logger) *SafeStreamCloser {
@ -44,27 +46,35 @@ func (s *SafeStreamCloser) Write(p []byte) (n int, err error) {
} }
nBytes, err := s.stream.Write(p) nBytes, err := s.stream.Write(p)
if err != nil { if err != nil {
s.handleTimeout(err) s.handleWriteError(err)
} }
return nBytes, err return nBytes, err
} }
// Handles the timeout error in case it happened, by canceling the stream write. // Handles the timeout error in case it happened, by canceling the stream write.
func (s *SafeStreamCloser) handleTimeout(err error) { func (s *SafeStreamCloser) handleWriteError(err error) {
// If we are closing the stream we just ignore any write error.
if s.closing.Load() {
return
}
var netErr net.Error var netErr net.Error
if errors.As(err, &netErr) { if errors.As(err, &netErr) {
if netErr.Timeout() { if netErr.Timeout() {
// We don't need to log if what cause the timeout was `no network activity`. // We don't need to log if what cause the timeout was no network activity.
if !errors.Is(netErr, &idleTimeoutError) { if !errors.Is(netErr, &idleTimeoutError) {
s.log.Error().Err(netErr).Msg("Closing quic stream due to timeout while writing") s.log.Error().Err(netErr).Msg("Closing quic stream due to timeout while writing")
} }
// We need to explicitly cancel the write so that it frees all buffers.
s.stream.CancelWrite(0) s.stream.CancelWrite(0)
} }
} }
} }
func (s *SafeStreamCloser) Close() error { func (s *SafeStreamCloser) Close() error {
// Set this stream to a closing state.
s.closing.Store(true)
// Make sure a possible writer does not block the lock forever. We need it, so we can close the writer // Make sure a possible writer does not block the lock forever. We need it, so we can close the writer
// side of the stream safely. // side of the stream safely.
_ = s.stream.SetWriteDeadline(time.Now()) _ = s.stream.SetWriteDeadline(time.Now())