Skip to content

Commit

Permalink
metrics: refactor metrics (ethereum#28035)
Browse files Browse the repository at this point in the history
This change includes a lot of things, listed below.

The interfaces have been split up into one write-interface and one read-interface, with `Snapshot` being the gateway from write to read. This simplifies the semantics _a lot_.

Example of splitting up an interface into one readonly 'snapshot' part, and one updatable writeonly part:

```golang
type MeterSnapshot interface {
	Count() int64
	Rate1() float64
	Rate5() float64
	Rate15() float64
	RateMean() float64
}

// Meters count events to produce exponentially-weighted moving average rates
// at one-, five-, and fifteen-minutes and a mean rate.
type Meter interface {
	Mark(int64)
	Snapshot() MeterSnapshot
	Stop()
}
```

This PR makes the concurrency model clearer. We have actual meters and snapshot of meters. The `meter` is the thing which can be accessed from the registry, and updates can be made to it.

- For all `meters`, (`Gauge`, `Timer` etc), it is assumed that they are accessed by different threads, making updates. Therefore, all `meters` update-methods (`Inc`, `Add`, `Update`, `Clear` etc) need to be concurrency-safe.
- All `meters` have a `Snapshot()` method. This method is _usually_ called from one thread, a backend-exporter. But it's fully possible to have several exporters simultaneously: therefore this method should also be concurrency-safe.

TLDR: `meter`s are accessible via registry, all their methods must be concurrency-safe.

For all `Snapshot`s, it is assumed that an individual exporter-thread has obtained a `meter` from the registry, and called the `Snapshot` method to obtain a readonly snapshot. This snapshot is _not_ guaranteed to be concurrency-safe. There's no need for a snapshot to be concurrency-safe, since exporters should not share snapshots.

Note, though: that by happenstance a lot of the snapshots _are_ concurrency-safe, being unmutable minimal representations of a value. Only the more complex ones are _not_ threadsafe, those that lazily calculate things like `Variance()`, `Mean()`.

Example of how a background exporter typically works, obtaining the snapshot and sequentially accessing the non-threadsafe methods in it:
```golang
		ms := metric.Snapshot()
                ...
		fields := map[string]interface{}{
			"count":    ms.Count(),
			"max":      ms.Max(),
			"mean":     ms.Mean(),
			"min":      ms.Min(),
			"stddev":   ms.StdDev(),
			"variance": ms.Variance(),
```

TLDR: `snapshots` are not guaranteed to be concurrency-safe (but often are).

I also changed the `Sample` type: previously, it iterated the samples fully every time `Mean()`,`Sum()`, `Min()` or `Max()` was invoked. Since we now have readonly base data, we can just iterate it once, in the constructor, and set all four values at once.

The same thing has been done for runtimehistogram.

Back when ResettingTImer was implemented, as part of ethereum#15910, Anton implemented a `Percentiles` on the new type. However, the method did not conform to the other existing types which also had a `Percentiles`.

1. The existing ones, on input, took `0.5` to mean `50%`. Anton used `50` to mean `50%`.
2. The existing ones returned `float64` outputs, thus interpolating between values. A value-set of `0, 10`, at `50%` would return `5`, whereas Anton's would return either `0` or `10`.

This PR removes the 'new' version, and uses only the 'legacy' percentiles, also for the ResettingTimer type.

The resetting timer snapshot was also defined so that it would expose the internal values. This has been removed, and getters for `Max, Min, Mean` have been added instead.

A lot of types were exported, but do not need to be. This PR unexports quite a lot of them.

metrics: refactor metrics (28035)
  • Loading branch information
gzliudan committed Dec 10, 2024
1 parent 9f22af5 commit e61b9c1
Show file tree
Hide file tree
Showing 50 changed files with 1,107 additions and 1,988 deletions.
2 changes: 1 addition & 1 deletion consensus/ethash/ethash.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ func (ethash *Ethash) SetThreads(threads int) {
// Hashrate implements PoW, returning the measured rate of the search invocations
// per second over the last minute.
func (ethash *Ethash) Hashrate() float64 {
return ethash.hashrate.Rate1()
return ethash.hashrate.Snapshot().Rate1()
}

// APIs implements consensus.Engine, returning the user facing RPC APIs. Currently
Expand Down
72 changes: 20 additions & 52 deletions metrics/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import (
"sync/atomic"
)

type CounterSnapshot interface {
Count() int64
}

// Counters hold an int64 value that can be incremented and decremented.
type Counter interface {
Clear()
Count() int64
Dec(int64)
Inc(int64)
Snapshot() Counter
Snapshot() CounterSnapshot
}

// GetOrRegisterCounter returns an existing Counter or constructs and registers
Expand Down Expand Up @@ -38,13 +41,13 @@ func NewCounter() Counter {
if !Enabled {
return NilCounter{}
}
return &StandardCounter{}
return new(StandardCounter)
}

// NewCounterForced constructs a new StandardCounter and returns it no matter if
// the global switch is enabled or not.
func NewCounterForced() Counter {
return &StandardCounter{}
return new(StandardCounter)
}

// NewRegisteredCounter constructs and registers a new StandardCounter.
Expand All @@ -70,75 +73,40 @@ func NewRegisteredCounterForced(name string, r Registry) Counter {
return c
}

// CounterSnapshot is a read-only copy of another Counter.
type CounterSnapshot int64

// Clear panics.
func (CounterSnapshot) Clear() {
panic("Clear called on a CounterSnapshot")
}
// counterSnapshot is a read-only copy of another Counter.
type counterSnapshot int64

// Count returns the count at the time the snapshot was taken.
func (c CounterSnapshot) Count() int64 { return int64(c) }

// Dec panics.
func (CounterSnapshot) Dec(int64) {
panic("Dec called on a CounterSnapshot")
}

// Inc panics.
func (CounterSnapshot) Inc(int64) {
panic("Inc called on a CounterSnapshot")
}

// Snapshot returns the snapshot.
func (c CounterSnapshot) Snapshot() Counter { return c }
func (c counterSnapshot) Count() int64 { return int64(c) }

// NilCounter is a no-op Counter.
type NilCounter struct{}

// Clear is a no-op.
func (NilCounter) Clear() {}

// Count is a no-op.
func (NilCounter) Count() int64 { return 0 }

// Dec is a no-op.
func (NilCounter) Dec(i int64) {}

// Inc is a no-op.
func (NilCounter) Inc(i int64) {}

// Snapshot is a no-op.
func (NilCounter) Snapshot() Counter { return NilCounter{} }
func (NilCounter) Clear() {}
func (NilCounter) Dec(i int64) {}
func (NilCounter) Inc(i int64) {}
func (NilCounter) Snapshot() CounterSnapshot { return (*emptySnapshot)(nil) }

// StandardCounter is the standard implementation of a Counter and uses the
// sync/atomic package to manage a single int64 value.
type StandardCounter struct {
count atomic.Int64
}
type StandardCounter atomic.Int64

// Clear sets the counter to zero.
func (c *StandardCounter) Clear() {
c.count.Store(0)
}

// Count returns the current count.
func (c *StandardCounter) Count() int64 {
return c.count.Load()
(*atomic.Int64)(c).Store(0)
}

// Dec decrements the counter by the given amount.
func (c *StandardCounter) Dec(i int64) {
c.count.Add(-i)
(*atomic.Int64)(c).Add(-i)
}

// Inc increments the counter by the given amount.
func (c *StandardCounter) Inc(i int64) {
c.count.Add(i)
(*atomic.Int64)(c).Add(i)
}

// Snapshot returns a read-only copy of the counter.
func (c *StandardCounter) Snapshot() Counter {
return CounterSnapshot(c.Count())
func (c *StandardCounter) Snapshot() CounterSnapshot {
return counterSnapshot((*atomic.Int64)(c).Load())
}
61 changes: 16 additions & 45 deletions metrics/counter_float64.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ import (
"sync/atomic"
)

type CounterFloat64Snapshot interface {
Count() float64
}

// CounterFloat64 holds a float64 value that can be incremented and decremented.
type CounterFloat64 interface {
Clear()
Count() float64
Dec(float64)
Inc(float64)
Snapshot() CounterFloat64
Snapshot() CounterFloat64Snapshot
}

// GetOrRegisterCounterFloat64 returns an existing CounterFloat64 or constructs and registers
Expand Down Expand Up @@ -71,47 +74,19 @@ func NewRegisteredCounterFloat64Forced(name string, r Registry) CounterFloat64 {
return c
}

// CounterFloat64Snapshot is a read-only copy of another CounterFloat64.
type CounterFloat64Snapshot float64

// Clear panics.
func (CounterFloat64Snapshot) Clear() {
panic("Clear called on a CounterFloat64Snapshot")
}
// counterFloat64Snapshot is a read-only copy of another CounterFloat64.
type counterFloat64Snapshot float64

// Count returns the value at the time the snapshot was taken.
func (c CounterFloat64Snapshot) Count() float64 { return float64(c) }

// Dec panics.
func (CounterFloat64Snapshot) Dec(float64) {
panic("Dec called on a CounterFloat64Snapshot")
}
func (c counterFloat64Snapshot) Count() float64 { return float64(c) }

// Inc panics.
func (CounterFloat64Snapshot) Inc(float64) {
panic("Inc called on a CounterFloat64Snapshot")
}

// Snapshot returns the snapshot.
func (c CounterFloat64Snapshot) Snapshot() CounterFloat64 { return c }

// NilCounterFloat64 is a no-op CounterFloat64.
type NilCounterFloat64 struct{}

// Clear is a no-op.
func (NilCounterFloat64) Clear() {}

// Count is a no-op.
func (NilCounterFloat64) Count() float64 { return 0.0 }

// Dec is a no-op.
func (NilCounterFloat64) Dec(i float64) {}

// Inc is a no-op.
func (NilCounterFloat64) Inc(i float64) {}

// Snapshot is a no-op.
func (NilCounterFloat64) Snapshot() CounterFloat64 { return NilCounterFloat64{} }
func (NilCounterFloat64) Clear() {}
func (NilCounterFloat64) Count() float64 { return 0.0 }
func (NilCounterFloat64) Dec(i float64) {}
func (NilCounterFloat64) Inc(i float64) {}
func (NilCounterFloat64) Snapshot() CounterFloat64Snapshot { return NilCounterFloat64{} }

// StandardCounterFloat64 is the standard implementation of a CounterFloat64 and uses the
// atomic to manage a single float64 value.
Expand All @@ -124,11 +99,6 @@ func (c *StandardCounterFloat64) Clear() {
c.floatBits.Store(0)
}

// Count returns the current value.
func (c *StandardCounterFloat64) Count() float64 {
return math.Float64frombits(c.floatBits.Load())
}

// Dec decrements the counter by the given amount.
func (c *StandardCounterFloat64) Dec(v float64) {
atomicAddFloat(&c.floatBits, -v)
Expand All @@ -140,8 +110,9 @@ func (c *StandardCounterFloat64) Inc(v float64) {
}

// Snapshot returns a read-only copy of the counter.
func (c *StandardCounterFloat64) Snapshot() CounterFloat64 {
return CounterFloat64Snapshot(c.Count())
func (c *StandardCounterFloat64) Snapshot() CounterFloat64Snapshot {
v := math.Float64frombits(c.floatBits.Load())
return counterFloat64Snapshot(v)
}

func atomicAddFloat(fbits *atomic.Uint64, v float64) {
Expand Down
16 changes: 8 additions & 8 deletions metrics/counter_float_64_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func BenchmarkCounterFloat64Parallel(b *testing.B) {
}()
}
wg.Wait()
if have, want := c.Count(), 10.0*float64(b.N); have != want {
if have, want := c.Snapshot().Count(), 10.0*float64(b.N); have != want {
b.Fatalf("have %f want %f", have, want)
}
}
Expand All @@ -36,39 +36,39 @@ func TestCounterFloat64Clear(t *testing.T) {
c := NewCounterFloat64()
c.Inc(1.0)
c.Clear()
if count := c.Count(); count != 0 {
if count := c.Snapshot().Count(); count != 0 {
t.Errorf("c.Count(): 0 != %v\n", count)
}
}

func TestCounterFloat64Dec1(t *testing.T) {
c := NewCounterFloat64()
c.Dec(1.0)
if count := c.Count(); count != -1.0 {
if count := c.Snapshot().Count(); count != -1.0 {
t.Errorf("c.Count(): -1.0 != %v\n", count)
}
}

func TestCounterFloat64Dec2(t *testing.T) {
c := NewCounterFloat64()
c.Dec(2.0)
if count := c.Count(); count != -2.0 {
if count := c.Snapshot().Count(); count != -2.0 {
t.Errorf("c.Count(): -2.0 != %v\n", count)
}
}

func TestCounterFloat64Inc1(t *testing.T) {
c := NewCounterFloat64()
c.Inc(1.0)
if count := c.Count(); count != 1.0 {
if count := c.Snapshot().Count(); count != 1.0 {
t.Errorf("c.Count(): 1.0 != %v\n", count)
}
}

func TestCounterFloat64Inc2(t *testing.T) {
c := NewCounterFloat64()
c.Inc(2.0)
if count := c.Count(); count != 2.0 {
if count := c.Snapshot().Count(); count != 2.0 {
t.Errorf("c.Count(): 2.0 != %v\n", count)
}
}
Expand All @@ -85,15 +85,15 @@ func TestCounterFloat64Snapshot(t *testing.T) {

func TestCounterFloat64Zero(t *testing.T) {
c := NewCounterFloat64()
if count := c.Count(); count != 0 {
if count := c.Snapshot().Count(); count != 0 {
t.Errorf("c.Count(): 0 != %v\n", count)
}
}

func TestGetOrRegisterCounterFloat64(t *testing.T) {
r := NewRegistry()
NewRegisteredCounterFloat64("foo", r).Inc(47.0)
if c := GetOrRegisterCounterFloat64("foo", r); c.Count() != 47.0 {
if c := GetOrRegisterCounterFloat64("foo", r).Snapshot(); c.Count() != 47.0 {
t.Fatal(c)
}
}
14 changes: 7 additions & 7 deletions metrics/counter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,39 +14,39 @@ func TestCounterClear(t *testing.T) {
c := NewCounter()
c.Inc(1)
c.Clear()
if count := c.Count(); count != 0 {
if count := c.Snapshot().Count(); count != 0 {
t.Errorf("c.Count(): 0 != %v\n", count)
}
}

func TestCounterDec1(t *testing.T) {
c := NewCounter()
c.Dec(1)
if count := c.Count(); count != -1 {
if count := c.Snapshot().Count(); count != -1 {
t.Errorf("c.Count(): -1 != %v\n", count)
}
}

func TestCounterDec2(t *testing.T) {
c := NewCounter()
c.Dec(2)
if count := c.Count(); count != -2 {
if count := c.Snapshot().Count(); count != -2 {
t.Errorf("c.Count(): -2 != %v\n", count)
}
}

func TestCounterInc1(t *testing.T) {
c := NewCounter()
c.Inc(1)
if count := c.Count(); count != 1 {
if count := c.Snapshot().Count(); count != 1 {
t.Errorf("c.Count(): 1 != %v\n", count)
}
}

func TestCounterInc2(t *testing.T) {
c := NewCounter()
c.Inc(2)
if count := c.Count(); count != 2 {
if count := c.Snapshot().Count(); count != 2 {
t.Errorf("c.Count(): 2 != %v\n", count)
}
}
Expand All @@ -63,15 +63,15 @@ func TestCounterSnapshot(t *testing.T) {

func TestCounterZero(t *testing.T) {
c := NewCounter()
if count := c.Count(); count != 0 {
if count := c.Snapshot().Count(); count != 0 {
t.Errorf("c.Count(): 0 != %v\n", count)
}
}

func TestGetOrRegisterCounter(t *testing.T) {
r := NewRegistry()
NewRegisteredCounter("foo", r).Inc(47)
if c := GetOrRegisterCounter("foo", r); c.Count() != 47 {
if c := GetOrRegisterCounter("foo", r).Snapshot(); c.Count() != 47 {
t.Fatal(c)
}
}
4 changes: 0 additions & 4 deletions metrics/doc.go

This file was deleted.

Loading

0 comments on commit e61b9c1

Please sign in to comment.