Skip to content

Commit

Permalink
metrics: zero temp variable in updateMeter (ethereum#21470)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
MariusVanDerWijden authored and enriquefynn committed Feb 15, 2021
1 parent 60df30a commit 30e22ae
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
8 changes: 6 additions & 2 deletions metrics/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions metrics/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 30e22ae

Please sign in to comment.