mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: use atomic.Int64 for workspace traffic metrics (#24844)
connMetrics.total was written to via atomic.AddInt64 and read as a plain int64, producing a data race. Fix by switching the field to atomic.Int64 and using its typed Add/Load methods. The testMetrics mock had a similar issue where mutex use was missing where GetTotalBytes read the total bytes, which is also fixed in this change.
This commit is contained in:
@@ -86,7 +86,7 @@ type connMetrics struct {
|
||||
addError func(float64)
|
||||
observeLatency func(float64)
|
||||
addTotal func(float64)
|
||||
total int64
|
||||
total atomic.Int64
|
||||
}
|
||||
|
||||
func (c *connMetrics) AddError(f float64) {
|
||||
@@ -98,10 +98,10 @@ func (c *connMetrics) ObserveLatency(f float64) {
|
||||
}
|
||||
|
||||
func (c *connMetrics) AddTotal(f float64) {
|
||||
atomic.AddInt64(&c.total, int64(f))
|
||||
c.total.Add(int64(f))
|
||||
c.addTotal(f)
|
||||
}
|
||||
|
||||
func (c *connMetrics) GetTotalBytes() int64 {
|
||||
return c.total
|
||||
return c.total.Load()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
package workspacetraffic_test
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/scaletest/workspacetraffic"
|
||||
)
|
||||
|
||||
func TestConnMetrics_Concurrent(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
reg := prometheus.NewRegistry()
|
||||
m := workspacetraffic.NewMetrics(reg, "username", "workspace_name", "agent_name")
|
||||
cm := m.ReadMetrics("username", "workspace_name", "agent_name")
|
||||
|
||||
const (
|
||||
writers = 8
|
||||
readers = 8
|
||||
opsPerWriter = 1000
|
||||
bytesPerWrite = 1
|
||||
)
|
||||
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(writers + readers)
|
||||
for i := 0; i < writers; i++ {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
for j := 0; j < opsPerWriter; j++ {
|
||||
cm.AddTotal(float64(bytesPerWrite))
|
||||
}
|
||||
}()
|
||||
}
|
||||
for i := 0; i < readers; i++ {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
for j := 0; j < opsPerWriter; j++ {
|
||||
_ = cm.GetTotalBytes()
|
||||
}
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
require.Equal(t, int64(writers*opsPerWriter*bytesPerWrite), cm.GetTotalBytes())
|
||||
}
|
||||
@@ -423,5 +423,7 @@ func (m *testMetrics) Latencies() []float64 {
|
||||
}
|
||||
|
||||
func (m *testMetrics) GetTotalBytes() int64 {
|
||||
m.Lock()
|
||||
defer m.Unlock()
|
||||
return int64(m.total)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user