From bf4b42a551b486524cc1c6a014f258c1773ee363 Mon Sep 17 00:00:00 2001 From: Daniel Liu Date: Fri, 13 Dec 2024 14:00:12 +0800 Subject: [PATCH] metrics: zero temp variable in updateMeter (#21470) * metrics: zero temp variable in updateMeter Previously the temp variable was not updated properly after summing it to count. This meant we had astronomically high metrics, now we zero out the temp whenever we sum it onto the snapshot count * metrics: move temp variable to be aligned, unit tests Moves the temp variable in MeterSnapshot to be 64-bit aligned because of the atomic bug. Adds a unit test, that catches the previous bug. --- metrics/meter.go | 8 ++++++-- metrics/meter_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/metrics/meter.go b/metrics/meter.go index 7d2a2f5307..60ae919d04 100644 --- a/metrics/meter.go +++ b/metrics/meter.go @@ -101,8 +101,12 @@ func NewRegisteredMeterForced(name string, r Registry) Meter { // MeterSnapshot is a read-only copy of another Meter. type MeterSnapshot struct { - count int64 + // WARNING: The `temp` field is accessed atomically. + // On 32 bit platforms, only 64-bit aligned fields can be atomic. The struct is + // guaranteed to be so aligned, so take advantage of that. For more information, + // see https://golang.org/pkg/sync/atomic/#pkg-note-BUG. temp int64 + count int64 rate1, rate5, rate15, rateMean float64 } @@ -253,7 +257,7 @@ func (m *StandardMeter) updateSnapshot() { func (m *StandardMeter) updateMeter() { // should only run with write lock held on m.lock - n := atomic.LoadInt64(&m.snapshot.temp) + n := atomic.SwapInt64(&m.snapshot.temp, 0) m.snapshot.count += n m.a1.Update(n) m.a5.Update(n) diff --git a/metrics/meter_test.go b/metrics/meter_test.go index c44286a035..99bbd6d989 100644 --- a/metrics/meter_test.go +++ b/metrics/meter_test.go @@ -73,3 +73,19 @@ func TestMeterZero(t *testing.T) { t.Errorf("m.Count(): 0 != %v\n", count) } } + +func TestMeterRepeat(t *testing.T) { + m := NewMeter() + for i := 0; i < 101; i++ { + m.Mark(int64(i)) + } + if count := m.Count(); count != 5050 { + t.Errorf("m.Count(): 5050 != %v\n", count) + } + for i := 0; i < 101; i++ { + m.Mark(int64(i)) + } + if count := m.Count(); count != 10100 { + t.Errorf("m.Count(): 10100 != %v\n", count) + } +}