From 98a08e1644858d25cbc5c8613c02aebd3f558b8a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 1 Oct 2020 09:50:08 -0700 Subject: [PATCH] fix: protect updating latencies/throughput slices in obd (#10611) Additionally close the transferChan upon function exit. --- cmd/peer-rest-client.go | 8 +++++++- pkg/disk/obd.go | 7 +------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index 3c098bb39..7b6f0dd85 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -134,6 +134,7 @@ func (r *nullReader) Read(b []byte) (int, error) { } func (client *peerRESTClient) doNetOBDTest(ctx context.Context, dataSize int64, threadCount uint) (info madmin.NetOBDInfo, err error) { + var mu sync.Mutex // mutex used to protect these slices in go-routines latencies := []float64{} throughputs := []float64{} @@ -142,6 +143,8 @@ func (client *peerRESTClient) doNetOBDTest(ctx context.Context, dataSize int64, totalTransferred := int64(0) transferChan := make(chan int64, threadCount) + defer close(transferChan) + go func() { for v := range transferChan { atomic.AddInt64(&totalTransferred, v) @@ -226,13 +229,16 @@ func (client *peerRESTClient) doNetOBDTest(ctx context.Context, dataSize int64, /* Throughput = (total data transferred across all threads / time taken) */ throughput := float64((after - before)) / latency + // Protect updating latencies and throughputs slices from + // multiple go-routines. + mu.Lock() latencies = append(latencies, latency) throughputs = append(throughputs, throughput) + mu.Unlock() }(i) } } wg.Wait() - close(transferChan) if err != nil { return info, err diff --git a/pkg/disk/obd.go b/pkg/disk/obd.go index d439db1c6..c48511c00 100644 --- a/pkg/disk/obd.go +++ b/pkg/disk/obd.go @@ -27,9 +27,6 @@ import ( "github.com/montanaflynn/stats" ) -var globalLatency = map[string]Latency{} -var globalThroughput = map[string]Throughput{} - // Latency holds latency information for write operations to the drive type Latency struct { Avg float64 `json:"avg_secs,omitempty"` @@ -154,6 +151,7 @@ func GetOBDInfo(ctx context.Context, drive, fsPath string) (Latency, Throughput, if minThroughput, err = stats.Min(throughputs); err != nil { return Latency{}, Throughput{}, err } + t := Throughput{ Avg: avgThroughput, Percentile50: percentile50Throughput, @@ -163,8 +161,5 @@ func GetOBDInfo(ctx context.Context, drive, fsPath string) (Latency, Throughput, Max: maxThroughput, } - globalLatency[drive] = l - globalThroughput[drive] = t - return l, t, nil }