Skip to content

Commit 30e22ae

Browse files
MariusVanDerWijdenenriquefynn
authored andcommitted
metrics: zero temp variable in updateMeter (ethereum#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.
1 parent 60df30a commit 30e22ae

File tree

2 files changed

+22
-2
lines changed

2 files changed

+22
-2
lines changed

metrics/meter.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,12 @@ func NewRegisteredMeterForced(name string, r Registry) Meter {
101101

102102
// MeterSnapshot is a read-only copy of another Meter.
103103
type MeterSnapshot struct {
104-
count int64
104+
// WARNING: The `temp` field is accessed atomically.
105+
// On 32 bit platforms, only 64-bit aligned fields can be atomic. The struct is
106+
// guaranteed to be so aligned, so take advantage of that. For more information,
107+
// see https://golang.org/pkg/sync/atomic/#pkg-note-BUG.
105108
temp int64
109+
count int64
106110
rate1, rate5, rate15, rateMean float64
107111
}
108112

@@ -253,7 +257,7 @@ func (m *StandardMeter) updateSnapshot() {
253257

254258
func (m *StandardMeter) updateMeter() {
255259
// should only run with write lock held on m.lock
256-
n := atomic.LoadInt64(&m.snapshot.temp)
260+
n := atomic.SwapInt64(&m.snapshot.temp, 0)
257261
m.snapshot.count += n
258262
m.a1.Update(n)
259263
m.a5.Update(n)

metrics/meter_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,19 @@ func TestMeterZero(t *testing.T) {
7373
t.Errorf("m.Count(): 0 != %v\n", count)
7474
}
7575
}
76+
77+
func TestMeterRepeat(t *testing.T) {
78+
m := NewMeter()
79+
for i := 0; i < 101; i++ {
80+
m.Mark(int64(i))
81+
}
82+
if count := m.Count(); count != 5050 {
83+
t.Errorf("m.Count(): 5050 != %v\n", count)
84+
}
85+
for i := 0; i < 101; i++ {
86+
m.Mark(int64(i))
87+
}
88+
if count := m.Count(); count != 10100 {
89+
t.Errorf("m.Count(): 10100 != %v\n", count)
90+
}
91+
}

0 commit comments

Comments
 (0)