Skip to content

Commit f1a9b65

Browse files
ARR4Nqdm12
ARR4N
authored and
qdm12
committed
fix(metrics): avoid conflicts between subnet-evm and libevm
`geth` (and hence `libevm`) global metrics tend to be created with `NewRegistered*()`, which assumes that there are no name conflicts on the same `Registry`. If there are then the second call fails silently to register the new metric. Except for in `warp` (where there won't be any conflicts) metric registration is changed globally to use `GetOrRegister*()` instead of `NewRegistered*()`. With GNU tools (not the Mac budget replacements): ```shell find -iname '*.go' -not -iwholename '*/warp/*' | xargs sed -i 's|metrics.NewRegistered|metrics.GetOrRegister|g' ``` All modified files that don't already import their `libevm` counterparts now `_` import them to force the initialisation order. The `libevm` construction with `NewRegistered*()` therefore always comes before the respective `GetOrRegister*()` so both call sites have the same metric instance. Metrics that we expect to be counters instead of timers are unregistered and newly created with the correct type, borrowing from ava-labs/libevm#159. See original PR ava-labs/coreth#860
1 parent 66278e9 commit f1a9b65

File tree

11 files changed

+309
-208
lines changed

11 files changed

+309
-208
lines changed

‎core/blockchain.go‎

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -59,47 +59,61 @@ import (
5959
"github.com/ava-labs/subnet-evm/params"
6060
"github.com/ava-labs/subnet-evm/triedb/hashdb"
6161
"github.com/ava-labs/subnet-evm/triedb/pathdb"
62+
63+
// Force libevm metrics of the same name to be registered first.
64+
_ "github.com/ava-labs/libevm/core"
6265
)
6366

67+
// ====== If resolving merge conflicts ======
68+
//
69+
// All calls to metrics.NewRegistered*() for metrics also defined in libevm/core have been
70+
// replaced either with:
71+
// - metrics.GetOrRegister*() to get a metric already registered in libevm/core, or register it
72+
// here otherwise
73+
// - [getOrOverrideAsRegisteredCounter] to get a metric already registered in libevm/core
74+
// only if it is a [metrics.Counter]. If it is not, the metric is unregistered and registered
75+
// as a [metrics.Counter] here.
76+
//
77+
// These replacements ensure the same metrics are shared between the two packages.
6478
var (
65-
accountReadTimer = metrics.NewRegisteredCounter("chain/account/reads", nil)
66-
accountHashTimer = metrics.NewRegisteredCounter("chain/account/hashes", nil)
67-
accountUpdateTimer = metrics.NewRegisteredCounter("chain/account/updates", nil)
68-
accountCommitTimer = metrics.NewRegisteredCounter("chain/account/commits", nil)
69-
storageReadTimer = metrics.NewRegisteredCounter("chain/storage/reads", nil)
70-
storageHashTimer = metrics.NewRegisteredCounter("chain/storage/hashes", nil)
71-
storageUpdateTimer = metrics.NewRegisteredCounter("chain/storage/updates", nil)
72-
storageCommitTimer = metrics.NewRegisteredCounter("chain/storage/commits", nil)
73-
snapshotAccountReadTimer = metrics.NewRegisteredCounter("chain/snapshot/account/reads", nil)
74-
snapshotStorageReadTimer = metrics.NewRegisteredCounter("chain/snapshot/storage/reads", nil)
75-
snapshotCommitTimer = metrics.NewRegisteredCounter("chain/snapshot/commits", nil)
76-
77-
triedbCommitTimer = metrics.NewRegisteredCounter("chain/triedb/commits", nil)
78-
79-
blockInsertTimer = metrics.NewRegisteredCounter("chain/block/inserts", nil)
80-
blockInsertCount = metrics.NewRegisteredCounter("chain/block/inserts/count", nil)
81-
blockContentValidationTimer = metrics.NewRegisteredCounter("chain/block/validations/content", nil)
82-
blockStateInitTimer = metrics.NewRegisteredCounter("chain/block/inits/state", nil)
83-
blockExecutionTimer = metrics.NewRegisteredCounter("chain/block/executions", nil)
84-
blockTrieOpsTimer = metrics.NewRegisteredCounter("chain/block/trie", nil)
85-
blockValidationTimer = metrics.NewRegisteredCounter("chain/block/validations/state", nil)
86-
blockWriteTimer = metrics.NewRegisteredCounter("chain/block/writes", nil)
87-
88-
acceptorQueueGauge = metrics.NewRegisteredGauge("chain/acceptor/queue/size", nil)
89-
acceptorWorkTimer = metrics.NewRegisteredCounter("chain/acceptor/work", nil)
90-
acceptorWorkCount = metrics.NewRegisteredCounter("chain/acceptor/work/count", nil)
79+
accountReadTimer = getOrOverrideAsRegisteredCounter("chain/account/reads", nil)
80+
accountHashTimer = getOrOverrideAsRegisteredCounter("chain/account/hashes", nil)
81+
accountUpdateTimer = getOrOverrideAsRegisteredCounter("chain/account/updates", nil)
82+
accountCommitTimer = getOrOverrideAsRegisteredCounter("chain/account/commits", nil)
83+
storageReadTimer = getOrOverrideAsRegisteredCounter("chain/storage/reads", nil)
84+
storageHashTimer = getOrOverrideAsRegisteredCounter("chain/storage/hashes", nil)
85+
storageUpdateTimer = getOrOverrideAsRegisteredCounter("chain/storage/updates", nil)
86+
storageCommitTimer = getOrOverrideAsRegisteredCounter("chain/storage/commits", nil)
87+
snapshotAccountReadTimer = getOrOverrideAsRegisteredCounter("chain/snapshot/account/reads", nil)
88+
snapshotStorageReadTimer = getOrOverrideAsRegisteredCounter("chain/snapshot/storage/reads", nil)
89+
snapshotCommitTimer = getOrOverrideAsRegisteredCounter("chain/snapshot/commits", nil)
90+
91+
triedbCommitTimer = getOrOverrideAsRegisteredCounter("chain/triedb/commits", nil)
92+
93+
blockInsertTimer = metrics.GetOrRegisterCounter("chain/block/inserts", nil)
94+
blockInsertCount = metrics.GetOrRegisterCounter("chain/block/inserts/count", nil)
95+
blockContentValidationTimer = metrics.GetOrRegisterCounter("chain/block/validations/content", nil)
96+
blockStateInitTimer = metrics.GetOrRegisterCounter("chain/block/inits/state", nil)
97+
blockExecutionTimer = metrics.GetOrRegisterCounter("chain/block/executions", nil)
98+
blockTrieOpsTimer = metrics.GetOrRegisterCounter("chain/block/trie", nil)
99+
blockValidationTimer = metrics.GetOrRegisterCounter("chain/block/validations/state", nil)
100+
blockWriteTimer = metrics.GetOrRegisterCounter("chain/block/writes", nil)
101+
102+
acceptorQueueGauge = metrics.GetOrRegisterGauge("chain/acceptor/queue/size", nil)
103+
acceptorWorkTimer = metrics.GetOrRegisterCounter("chain/acceptor/work", nil)
104+
acceptorWorkCount = metrics.GetOrRegisterCounter("chain/acceptor/work/count", nil)
105+
processedBlockGasUsedCounter = metrics.GetOrRegisterCounter("chain/block/gas/used/processed", nil)
91106
lastAcceptedBlockBaseFeeGauge = metrics.NewRegisteredGauge("chain/block/fee/basefee", nil)
92107
blockTotalFeesGauge = metrics.NewRegisteredGauge("chain/block/fee/total", nil)
93-
processedBlockGasUsedCounter = metrics.NewRegisteredCounter("chain/block/gas/used/processed", nil)
94-
acceptedBlockGasUsedCounter = metrics.NewRegisteredCounter("chain/block/gas/used/accepted", nil)
95-
badBlockCounter = metrics.NewRegisteredCounter("chain/block/bad/count", nil)
108+
acceptedBlockGasUsedCounter = metrics.GetOrRegisterCounter("chain/block/gas/used/accepted", nil)
109+
badBlockCounter = metrics.GetOrRegisterCounter("chain/block/bad/count", nil)
96110

97-
txUnindexTimer = metrics.NewRegisteredCounter("chain/txs/unindex", nil)
98-
acceptedTxsCounter = metrics.NewRegisteredCounter("chain/txs/accepted", nil)
99-
processedTxsCounter = metrics.NewRegisteredCounter("chain/txs/processed", nil)
111+
txUnindexTimer = metrics.GetOrRegisterCounter("chain/txs/unindex", nil)
112+
acceptedTxsCounter = metrics.GetOrRegisterCounter("chain/txs/accepted", nil)
113+
processedTxsCounter = metrics.GetOrRegisterCounter("chain/txs/processed", nil)
100114

101-
acceptedLogsCounter = metrics.NewRegisteredCounter("chain/logs/accepted", nil)
102-
processedLogsCounter = metrics.NewRegisteredCounter("chain/logs/processed", nil)
115+
acceptedLogsCounter = metrics.GetOrRegisterCounter("chain/logs/accepted", nil)
116+
processedLogsCounter = metrics.GetOrRegisterCounter("chain/logs/processed", nil)
103117

104118
ErrRefuseToCorruptArchiver = errors.New("node has operated with pruning disabled, shutting down to prevent missing tries")
105119

‎core/blockchain_ext.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// (c) 2024 Ava Labs, Inc. All rights reserved.
2+
// See the file LICENSE for licensing terms.
3+
package core
4+
5+
import "github.com/ava-labs/libevm/metrics"
6+
7+
// getOrOverrideAsRegisteredCounter searches for a metric already registered
8+
// with`name`. If a metric is found and it is a Counter, it is returned. If a
9+
// metric is found and it is not a Counter, it is unregistered and replaced with
10+
// a new registered Counter. If no metric is found, a new Counter is constructed
11+
// and registered.
12+
//
13+
// This is necessary for a metric defined in libevm with the same name but a
14+
// different type to what we expect.
15+
func getOrOverrideAsRegisteredCounter(name string, r metrics.Registry) metrics.Counter {
16+
if r == nil {
17+
r = metrics.DefaultRegistry
18+
}
19+
20+
switch c := r.GetOrRegister(name, metrics.NewCounter).(type) {
21+
case metrics.Counter:
22+
return c
23+
default: // `name` must have already been registered to be any other type
24+
r.Unregister(name)
25+
return metrics.NewRegisteredCounter(name, r)
26+
}
27+
}

‎core/main_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ func TestMain(m *testing.M) {
1515
opts := []goleak.Option{
1616
// No good way to shut down these goroutines:
1717
goleak.IgnoreTopFunction("github.com/ava-labs/subnet-evm/core/state/snapshot.(*diskLayer).generate"),
18+
goleak.IgnoreTopFunction("github.com/ava-labs/libevm/core.(*txSenderCacher).cache"),
1819
goleak.IgnoreTopFunction("github.com/ava-labs/subnet-evm/metrics.(*meterArbiter).tick"),
1920
goleak.IgnoreTopFunction("github.com/syndtr/goleveldb/leveldb.(*DB).mpoolDrain"),
2021
}

‎core/state/snapshot/snapshot.go‎

Lines changed: 48 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -55,49 +55,55 @@ const (
5555
skipGenThreshold = 500 * time.Millisecond
5656
)
5757

58+
// ====== If resolving merge conflicts ======
59+
//
60+
// All calls to metrics.NewRegistered*() for metrics also defined in libevm/core/state/snapshot
61+
// have been replaced with metrics.GetOrRegister*() to get metrics already registered in
62+
// libevm/core/state/snapshot or register them here otherwise. These replacements ensure the
63+
// same metrics are shared between the two packages.
5864
var (
59-
snapshotCleanAccountHitMeter = metrics.NewRegisteredMeter("state/snapshot/clean/account/hit", nil)
60-
snapshotCleanAccountMissMeter = metrics.NewRegisteredMeter("state/snapshot/clean/account/miss", nil)
61-
snapshotCleanAccountInexMeter = metrics.NewRegisteredMeter("state/snapshot/clean/account/inex", nil)
62-
snapshotCleanAccountReadMeter = metrics.NewRegisteredMeter("state/snapshot/clean/account/read", nil)
63-
snapshotCleanAccountWriteMeter = metrics.NewRegisteredMeter("state/snapshot/clean/account/write", nil)
64-
65-
snapshotCleanStorageHitMeter = metrics.NewRegisteredMeter("state/snapshot/clean/storage/hit", nil)
66-
snapshotCleanStorageMissMeter = metrics.NewRegisteredMeter("state/snapshot/clean/storage/miss", nil)
67-
snapshotCleanStorageInexMeter = metrics.NewRegisteredMeter("state/snapshot/clean/storage/inex", nil)
68-
snapshotCleanStorageReadMeter = metrics.NewRegisteredMeter("state/snapshot/clean/storage/read", nil)
69-
snapshotCleanStorageWriteMeter = metrics.NewRegisteredMeter("state/snapshot/clean/storage/write", nil)
70-
71-
snapshotDirtyAccountHitMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/account/hit", nil)
72-
snapshotDirtyAccountMissMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/account/miss", nil)
73-
snapshotDirtyAccountInexMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/account/inex", nil)
74-
snapshotDirtyAccountReadMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/account/read", nil)
75-
snapshotDirtyAccountWriteMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/account/write", nil)
76-
77-
snapshotDirtyStorageHitMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/storage/hit", nil)
78-
snapshotDirtyStorageMissMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/storage/miss", nil)
79-
snapshotDirtyStorageInexMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/storage/inex", nil)
80-
snapshotDirtyStorageReadMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/storage/read", nil)
81-
snapshotDirtyStorageWriteMeter = metrics.NewRegisteredMeter("state/snapshot/dirty/storage/write", nil)
82-
83-
snapshotDirtyAccountHitDepthHist = metrics.NewRegisteredHistogram("state/snapshot/dirty/account/hit/depth", nil, metrics.NewExpDecaySample(1028, 0.015))
84-
snapshotDirtyStorageHitDepthHist = metrics.NewRegisteredHistogram("state/snapshot/dirty/storage/hit/depth", nil, metrics.NewExpDecaySample(1028, 0.015))
85-
86-
snapshotFlushAccountItemMeter = metrics.NewRegisteredMeter("state/snapshot/flush/account/item", nil)
87-
snapshotFlushAccountSizeMeter = metrics.NewRegisteredMeter("state/snapshot/flush/account/size", nil)
88-
snapshotFlushStorageItemMeter = metrics.NewRegisteredMeter("state/snapshot/flush/storage/item", nil)
89-
snapshotFlushStorageSizeMeter = metrics.NewRegisteredMeter("state/snapshot/flush/storage/size", nil)
90-
91-
snapshotBloomIndexTimer = metrics.NewRegisteredResettingTimer("state/snapshot/bloom/index", nil)
92-
snapshotBloomErrorGauge = metrics.NewRegisteredGaugeFloat64("state/snapshot/bloom/error", nil)
93-
94-
snapshotBloomAccountTrueHitMeter = metrics.NewRegisteredMeter("state/snapshot/bloom/account/truehit", nil)
95-
snapshotBloomAccountFalseHitMeter = metrics.NewRegisteredMeter("state/snapshot/bloom/account/falsehit", nil)
96-
snapshotBloomAccountMissMeter = metrics.NewRegisteredMeter("state/snapshot/bloom/account/miss", nil)
97-
98-
snapshotBloomStorageTrueHitMeter = metrics.NewRegisteredMeter("state/snapshot/bloom/storage/truehit", nil)
99-
snapshotBloomStorageFalseHitMeter = metrics.NewRegisteredMeter("state/snapshot/bloom/storage/falsehit", nil)
100-
snapshotBloomStorageMissMeter = metrics.NewRegisteredMeter("state/snapshot/bloom/storage/miss", nil)
65+
snapshotCleanAccountHitMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/account/hit", nil)
66+
snapshotCleanAccountMissMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/account/miss", nil)
67+
snapshotCleanAccountInexMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/account/inex", nil)
68+
snapshotCleanAccountReadMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/account/read", nil)
69+
snapshotCleanAccountWriteMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/account/write", nil)
70+
71+
snapshotCleanStorageHitMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/storage/hit", nil)
72+
snapshotCleanStorageMissMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/storage/miss", nil)
73+
snapshotCleanStorageInexMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/storage/inex", nil)
74+
snapshotCleanStorageReadMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/storage/read", nil)
75+
snapshotCleanStorageWriteMeter = metrics.GetOrRegisterMeter("state/snapshot/clean/storage/write", nil)
76+
77+
snapshotDirtyAccountHitMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/account/hit", nil)
78+
snapshotDirtyAccountMissMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/account/miss", nil)
79+
snapshotDirtyAccountInexMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/account/inex", nil)
80+
snapshotDirtyAccountReadMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/account/read", nil)
81+
snapshotDirtyAccountWriteMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/account/write", nil)
82+
83+
snapshotDirtyStorageHitMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/storage/hit", nil)
84+
snapshotDirtyStorageMissMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/storage/miss", nil)
85+
snapshotDirtyStorageInexMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/storage/inex", nil)
86+
snapshotDirtyStorageReadMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/storage/read", nil)
87+
snapshotDirtyStorageWriteMeter = metrics.GetOrRegisterMeter("state/snapshot/dirty/storage/write", nil)
88+
89+
snapshotDirtyAccountHitDepthHist = metrics.GetOrRegisterHistogram("state/snapshot/dirty/account/hit/depth", nil, metrics.NewExpDecaySample(1028, 0.015))
90+
snapshotDirtyStorageHitDepthHist = metrics.GetOrRegisterHistogram("state/snapshot/dirty/storage/hit/depth", nil, metrics.NewExpDecaySample(1028, 0.015))
91+
92+
snapshotFlushAccountItemMeter = metrics.GetOrRegisterMeter("state/snapshot/flush/account/item", nil)
93+
snapshotFlushAccountSizeMeter = metrics.GetOrRegisterMeter("state/snapshot/flush/account/size", nil)
94+
snapshotFlushStorageItemMeter = metrics.GetOrRegisterMeter("state/snapshot/flush/storage/item", nil)
95+
snapshotFlushStorageSizeMeter = metrics.GetOrRegisterMeter("state/snapshot/flush/storage/size", nil)
96+
97+
snapshotBloomIndexTimer = metrics.GetOrRegisterResettingTimer("state/snapshot/bloom/index", nil)
98+
snapshotBloomErrorGauge = metrics.GetOrRegisterGaugeFloat64("state/snapshot/bloom/error", nil)
99+
100+
snapshotBloomAccountTrueHitMeter = metrics.GetOrRegisterMeter("state/snapshot/bloom/account/truehit", nil)
101+
snapshotBloomAccountFalseHitMeter = metrics.GetOrRegisterMeter("state/snapshot/bloom/account/falsehit", nil)
102+
snapshotBloomAccountMissMeter = metrics.GetOrRegisterMeter("state/snapshot/bloom/account/miss", nil)
103+
104+
snapshotBloomStorageTrueHitMeter = metrics.GetOrRegisterMeter("state/snapshot/bloom/storage/truehit", nil)
105+
snapshotBloomStorageFalseHitMeter = metrics.GetOrRegisterMeter("state/snapshot/bloom/storage/falsehit", nil)
106+
snapshotBloomStorageMissMeter = metrics.GetOrRegisterMeter("state/snapshot/bloom/storage/miss", nil)
101107

102108
// ErrSnapshotStale is returned from data accessors if the underlying snapshot
103109
// layer had been invalidated due to the chain progressing forward far enough

0 commit comments

Comments
 (0)