derp: remove stats goroutine, use a timer

Without changing behaviour, don't create a goroutine per connection that
sits and sleeps, but rather use a timer that wakes up and gathers
statistics on a regular basis.

Fixes #12127

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ibc486447e403070bdc3c2cd8ae340e7d02854f21
pull/12134/head
Andrew Dunham 6 months ago
parent 7ef2f72135
commit c6d42b1093

@ -776,7 +776,6 @@ func (c *sclient) run(ctx context.Context) error {
var grp errgroup.Group var grp errgroup.Group
sendCtx, cancelSender := context.WithCancel(ctx) sendCtx, cancelSender := context.WithCancel(ctx)
grp.Go(func() error { return c.sendLoop(sendCtx) }) grp.Go(func() error { return c.sendLoop(sendCtx) })
grp.Go(func() error { return c.statsLoop(sendCtx) })
defer func() { defer func() {
cancelSender() cancelSender()
if err := grp.Wait(); err != nil && !c.s.isClosed() { if err := grp.Wait(); err != nil && !c.s.isClosed() {
@ -788,6 +787,8 @@ func (c *sclient) run(ctx context.Context) error {
} }
}() }()
c.startStatsLoop(sendCtx)
for { for {
ft, fl, err := readFrameHeader(c.br) ft, fl, err := readFrameHeader(c.br)
c.debugLogf("read frame type %d len %d err %v", ft, fl, err) c.debugLogf("read frame type %d len %d err %v", ft, fl, err)

@ -7,6 +7,7 @@ package derp
import "context" import "context"
func (c *sclient) statsLoop(ctx context.Context) error { func (c *sclient) startStatsLoop(ctx context.Context) {
return nil // Nothing to do
return
} }

@ -12,40 +12,43 @@ import (
"tailscale.com/net/tcpinfo" "tailscale.com/net/tcpinfo"
) )
func (c *sclient) statsLoop(ctx context.Context) error { func (c *sclient) startStatsLoop(ctx context.Context) {
// Get the RTT initially to verify it's supported. // Get the RTT initially to verify it's supported.
conn := c.tcpConn() conn := c.tcpConn()
if conn == nil { if conn == nil {
c.s.tcpRtt.Add("non-tcp", 1) c.s.tcpRtt.Add("non-tcp", 1)
return nil return
} }
if _, err := tcpinfo.RTT(conn); err != nil { if _, err := tcpinfo.RTT(conn); err != nil {
c.logf("error fetching initial RTT: %v", err) c.logf("error fetching initial RTT: %v", err)
c.s.tcpRtt.Add("error", 1) c.s.tcpRtt.Add("error", 1)
return nil return
} }
const statsInterval = 10 * time.Second const statsInterval = 10 * time.Second
ticker, tickerChannel := c.s.clock.NewTicker(statsInterval) // Don't launch a goroutine; use a timer instead.
defer ticker.Stop() var gatherStats func()
gatherStats = func() {
// Do nothing if the context is finished.
if ctx.Err() != nil {
return
}
statsLoop: // Reschedule ourselves when this stats gathering is finished.
for { defer c.s.clock.AfterFunc(statsInterval, gatherStats)
select {
case <-tickerChannel: // Gather TCP RTT information.
rtt, err := tcpinfo.RTT(conn) rtt, err := tcpinfo.RTT(conn)
if err != nil { if err == nil {
continue statsLoop c.s.tcpRtt.Add(durationToLabel(rtt), 1)
} }
// TODO(andrew): more metrics? // TODO(andrew): more metrics?
c.s.tcpRtt.Add(durationToLabel(rtt), 1)
case <-ctx.Done():
return ctx.Err()
}
} }
// Kick off the initial timer.
c.s.clock.AfterFunc(statsInterval, gatherStats)
} }
// tcpConn attempts to get the underlying *net.TCPConn from this client's // tcpConn attempts to get the underlying *net.TCPConn from this client's

Loading…
Cancel
Save