Skip to content

Commit ed101d1

Browse files
karalabejorgemmsilva
authored andcommitted
cmd, core, metrics: always report expensive metrics (ethereum#29191)
* cmd, core, metrics: always report expensive metrics * core, metrics: report block processing metrics as resetting timer * metrics: update reporter tests
1 parent 0676276 commit ed101d1

File tree

14 files changed

+112
-149
lines changed

14 files changed

+112
-149
lines changed

cmd/geth/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ func applyMetricConfig(ctx *cli.Context, cfg *gethConfig) {
267267
cfg.Metrics.Enabled = ctx.Bool(utils.MetricsEnabledFlag.Name)
268268
}
269269
if ctx.IsSet(utils.MetricsEnabledExpensiveFlag.Name) {
270-
cfg.Metrics.EnabledExpensive = ctx.Bool(utils.MetricsEnabledExpensiveFlag.Name)
270+
log.Warn("Expensive metrics are collected by default, please remove this flag", "flag", utils.MetricsEnabledExpensiveFlag.Name)
271271
}
272272
if ctx.IsSet(utils.MetricsHTTPFlag.Name) {
273273
cfg.Metrics.HTTP = ctx.String(utils.MetricsHTTPFlag.Name)

cmd/utils/flags.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -862,12 +862,6 @@ var (
862862
Usage: "Enable metrics collection and reporting",
863863
Category: flags.MetricsCategory,
864864
}
865-
MetricsEnabledExpensiveFlag = &cli.BoolFlag{
866-
Name: "metrics.expensive",
867-
Usage: "Enable expensive metrics collection and reporting",
868-
Category: flags.MetricsCategory,
869-
}
870-
871865
// MetricsHTTPFlag defines the endpoint for a stand-alone metrics HTTP endpoint.
872866
// Since the pprof service enables sensitive/vulnerable behavior, this allows a user
873867
// to enable a public-OK metrics endpoint without having to worry about ALSO exposing

cmd/utils/flags_legacy.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,35 +93,35 @@ var (
9393
Name: "light.serve",
9494
Usage: "Maximum percentage of time allowed for serving LES requests (deprecated)",
9595
Value: ethconfig.Defaults.LightServ,
96-
Category: flags.LightCategory,
96+
Category: flags.DeprecatedCategory,
9797
}
9898
LightIngressFlag = &cli.IntFlag{
9999
Name: "light.ingress",
100100
Usage: "Incoming bandwidth limit for serving light clients (deprecated)",
101101
Value: ethconfig.Defaults.LightIngress,
102-
Category: flags.LightCategory,
102+
Category: flags.DeprecatedCategory,
103103
}
104104
LightEgressFlag = &cli.IntFlag{
105105
Name: "light.egress",
106106
Usage: "Outgoing bandwidth limit for serving light clients (deprecated)",
107107
Value: ethconfig.Defaults.LightEgress,
108-
Category: flags.LightCategory,
108+
Category: flags.DeprecatedCategory,
109109
}
110110
LightMaxPeersFlag = &cli.IntFlag{
111111
Name: "light.maxpeers",
112112
Usage: "Maximum number of light clients to serve, or light servers to attach to (deprecated)",
113113
Value: ethconfig.Defaults.LightPeers,
114-
Category: flags.LightCategory,
114+
Category: flags.DeprecatedCategory,
115115
}
116116
LightNoPruneFlag = &cli.BoolFlag{
117117
Name: "light.nopruning",
118118
Usage: "Disable ancient light chain data pruning (deprecated)",
119-
Category: flags.LightCategory,
119+
Category: flags.DeprecatedCategory,
120120
}
121121
LightNoSyncServeFlag = &cli.BoolFlag{
122122
Name: "light.nosyncserve",
123123
Usage: "Enables serving light clients before syncing (deprecated)",
124-
Category: flags.LightCategory,
124+
Category: flags.DeprecatedCategory,
125125
}
126126
// Deprecated November 2023
127127
LogBacktraceAtFlag = &cli.StringFlag{
@@ -138,19 +138,24 @@ var (
138138
// Deprecated February 2024
139139
MinerNewPayloadTimeoutFlag = &cli.DurationFlag{
140140
Name: "miner.newpayload-timeout",
141-
Usage: "Specify the maximum time allowance for creating a new payload",
141+
Usage: "Specify the maximum time allowance for creating a new payload (deprecated)",
142142
Value: ethconfig.Defaults.Miner.Recommit,
143-
Category: flags.MinerCategory,
143+
Category: flags.DeprecatedCategory,
144144
}
145145
MinerEtherbaseFlag = &cli.StringFlag{
146146
Name: "miner.etherbase",
147-
Usage: "0x prefixed public address for block mining rewards",
148-
Category: flags.MinerCategory,
147+
Usage: "0x prefixed public address for block mining rewards (deprecated)",
148+
Category: flags.DeprecatedCategory,
149149
}
150150
MiningEnabledFlag = &cli.BoolFlag{
151151
Name: "mine",
152-
Usage: "Enable mining",
153-
Category: flags.MinerCategory,
152+
Usage: "Enable mining (deprecated)",
153+
Category: flags.DeprecatedCategory,
154+
}
155+
MetricsEnabledExpensiveFlag = &cli.BoolFlag{
156+
Name: "metrics.expensive",
157+
Usage: "Enable expensive metrics collection and reporting (deprecated)",
158+
Category: flags.DeprecatedCategory,
154159
}
155160
)
156161

core/blockchain.go

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,26 @@ var (
6161

6262
chainInfoGauge = metrics.NewRegisteredGaugeInfo("chain/info", nil)
6363

64-
accountReadTimer = metrics.NewRegisteredTimer("chain/account/reads", nil)
65-
accountHashTimer = metrics.NewRegisteredTimer("chain/account/hashes", nil)
66-
accountUpdateTimer = metrics.NewRegisteredTimer("chain/account/updates", nil)
67-
accountCommitTimer = metrics.NewRegisteredTimer("chain/account/commits", nil)
68-
69-
storageReadTimer = metrics.NewRegisteredTimer("chain/storage/reads", nil)
70-
storageHashTimer = metrics.NewRegisteredTimer("chain/storage/hashes", nil)
71-
storageUpdateTimer = metrics.NewRegisteredTimer("chain/storage/updates", nil)
72-
storageCommitTimer = metrics.NewRegisteredTimer("chain/storage/commits", nil)
73-
74-
snapshotAccountReadTimer = metrics.NewRegisteredTimer("chain/snapshot/account/reads", nil)
75-
snapshotStorageReadTimer = metrics.NewRegisteredTimer("chain/snapshot/storage/reads", nil)
76-
snapshotCommitTimer = metrics.NewRegisteredTimer("chain/snapshot/commits", nil)
77-
78-
triedbCommitTimer = metrics.NewRegisteredTimer("chain/triedb/commits", nil)
79-
80-
blockInsertTimer = metrics.NewRegisteredTimer("chain/inserts", nil)
81-
blockValidationTimer = metrics.NewRegisteredTimer("chain/validation", nil)
82-
blockExecutionTimer = metrics.NewRegisteredTimer("chain/execution", nil)
83-
blockWriteTimer = metrics.NewRegisteredTimer("chain/write", nil)
64+
accountReadTimer = metrics.NewRegisteredResettingTimer("chain/account/reads", nil)
65+
accountHashTimer = metrics.NewRegisteredResettingTimer("chain/account/hashes", nil)
66+
accountUpdateTimer = metrics.NewRegisteredResettingTimer("chain/account/updates", nil)
67+
accountCommitTimer = metrics.NewRegisteredResettingTimer("chain/account/commits", nil)
68+
69+
storageReadTimer = metrics.NewRegisteredResettingTimer("chain/storage/reads", nil)
70+
storageHashTimer = metrics.NewRegisteredResettingTimer("chain/storage/hashes", nil)
71+
storageUpdateTimer = metrics.NewRegisteredResettingTimer("chain/storage/updates", nil)
72+
storageCommitTimer = metrics.NewRegisteredResettingTimer("chain/storage/commits", nil)
73+
74+
snapshotAccountReadTimer = metrics.NewRegisteredResettingTimer("chain/snapshot/account/reads", nil)
75+
snapshotStorageReadTimer = metrics.NewRegisteredResettingTimer("chain/snapshot/storage/reads", nil)
76+
snapshotCommitTimer = metrics.NewRegisteredResettingTimer("chain/snapshot/commits", nil)
77+
78+
triedbCommitTimer = metrics.NewRegisteredResettingTimer("chain/triedb/commits", nil)
79+
80+
blockInsertTimer = metrics.NewRegisteredResettingTimer("chain/inserts", nil)
81+
blockValidationTimer = metrics.NewRegisteredResettingTimer("chain/validation", nil)
82+
blockExecutionTimer = metrics.NewRegisteredResettingTimer("chain/execution", nil)
83+
blockWriteTimer = metrics.NewRegisteredResettingTimer("chain/write", nil)
8484

8585
blockReorgMeter = metrics.NewRegisteredMeter("chain/reorg/executes", nil)
8686
blockReorgAddMeter = metrics.NewRegisteredMeter("chain/reorg/add", nil)

core/state/state_object.go

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"github.com/ethereum/go-ethereum/common"
2626
"github.com/ethereum/go-ethereum/core/types"
2727
"github.com/ethereum/go-ethereum/crypto"
28-
"github.com/ethereum/go-ethereum/metrics"
2928
"github.com/ethereum/go-ethereum/rlp"
3029
"github.com/ethereum/go-ethereum/trie/trienode"
3130
"github.com/holiman/uint256"
@@ -197,9 +196,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
197196
if s.db.snap != nil {
198197
start := time.Now()
199198
enc, err = s.db.snap.Storage(s.addrHash, crypto.Keccak256Hash(key.Bytes()))
200-
if metrics.EnabledExpensive {
201-
s.db.SnapshotStorageReads += time.Since(start)
202-
}
199+
s.db.SnapshotStorageReads += time.Since(start)
200+
203201
if len(enc) > 0 {
204202
_, content, _, err := rlp.Split(enc)
205203
if err != nil {
@@ -217,9 +215,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
217215
return common.Hash{}
218216
}
219217
val, err := tr.GetStorage(s.address, key.Bytes())
220-
if metrics.EnabledExpensive {
221-
s.db.StorageReads += time.Since(start)
222-
}
218+
s.db.StorageReads += time.Since(start)
219+
223220
if err != nil {
224221
s.db.setError(err)
225222
return common.Hash{}
@@ -283,9 +280,8 @@ func (s *stateObject) updateTrie() (Trie, error) {
283280
return s.trie, nil
284281
}
285282
// Track the amount of time wasted on updating the storage trie
286-
if metrics.EnabledExpensive {
287-
defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())
288-
}
283+
defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())
284+
289285
// The snapshot storage map for the object
290286
var (
291287
storage map[common.Hash][]byte
@@ -370,9 +366,8 @@ func (s *stateObject) updateRoot() {
370366
return
371367
}
372368
// Track the amount of time wasted on hashing the storage trie
373-
if metrics.EnabledExpensive {
374-
defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
375-
}
369+
defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
370+
376371
s.data.Root = tr.Hash()
377372
}
378373

@@ -386,9 +381,8 @@ func (s *stateObject) commit() (*trienode.NodeSet, error) {
386381
return nil, nil
387382
}
388383
// Track the amount of time wasted on committing the storage trie
389-
if metrics.EnabledExpensive {
390-
defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
391-
}
384+
defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
385+
392386
// The trie is currently in an open state and could potentially contain
393387
// cached mutations. Call commit to acquire a set of nodes that have been
394388
// modified, the set can be nil if nothing to commit.

core/state/statedb.go

Lines changed: 38 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"github.com/ethereum/go-ethereum/core/types"
2929
"github.com/ethereum/go-ethereum/crypto"
3030
"github.com/ethereum/go-ethereum/log"
31-
"github.com/ethereum/go-ethereum/metrics"
3231
"github.com/ethereum/go-ethereum/params"
3332
"github.com/ethereum/go-ethereum/trie"
3433
"github.com/ethereum/go-ethereum/trie/trienode"
@@ -495,9 +494,8 @@ func (s *StateDB) GetTransientState(addr common.Address, key common.Hash) common
495494
// updateStateObject writes the given object to the trie.
496495
func (s *StateDB) updateStateObject(obj *stateObject) {
497496
// Track the amount of time wasted on updating the account from the trie
498-
if metrics.EnabledExpensive {
499-
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
500-
}
497+
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
498+
501499
// Encode the account and update the account trie
502500
addr := obj.Address()
503501
if err := s.trie.UpdateAccount(addr, &obj.data); err != nil {
@@ -527,9 +525,8 @@ func (s *StateDB) updateStateObject(obj *stateObject) {
527525
// deleteStateObject removes the given object from the state trie.
528526
func (s *StateDB) deleteStateObject(obj *stateObject) {
529527
// Track the amount of time wasted on deleting the account from the trie
530-
if metrics.EnabledExpensive {
531-
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
532-
}
528+
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
529+
533530
// Delete the account from the trie
534531
addr := obj.Address()
535532
if err := s.trie.DeleteAccount(addr); err != nil {
@@ -561,9 +558,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
561558
if s.snap != nil {
562559
start := time.Now()
563560
acc, err := s.snap.Account(crypto.HashData(s.hasher, addr.Bytes()))
564-
if metrics.EnabledExpensive {
565-
s.SnapshotAccountReads += time.Since(start)
566-
}
561+
s.SnapshotAccountReads += time.Since(start)
562+
567563
if err == nil {
568564
if acc == nil {
569565
return nil
@@ -587,9 +583,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
587583
start := time.Now()
588584
var err error
589585
data, err = s.trie.GetAccount(addr)
590-
if metrics.EnabledExpensive {
591-
s.AccountReads += time.Since(start)
592-
}
586+
s.AccountReads += time.Since(start)
587+
593588
if err != nil {
594589
s.setError(fmt.Errorf("getDeleteStateObject (%x) error: %w", addr.Bytes(), err))
595590
return nil
@@ -917,9 +912,8 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
917912
s.stateObjectsPending = make(map[common.Address]struct{})
918913
}
919914
// Track the amount of time wasted on hashing the account trie
920-
if metrics.EnabledExpensive {
921-
defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now())
922-
}
915+
defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now())
916+
923917
return s.trie.Hash()
924918
}
925919

@@ -1042,16 +1036,16 @@ func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root
10421036
if err != nil {
10431037
return nil, nil, err
10441038
}
1045-
if metrics.EnabledExpensive {
1046-
n := int64(len(slots))
1039+
// Report the metrics
1040+
n := int64(len(slots))
10471041

1048-
slotDeletionMaxCount.UpdateIfGt(int64(len(slots)))
1049-
slotDeletionMaxSize.UpdateIfGt(int64(size))
1042+
slotDeletionMaxCount.UpdateIfGt(int64(len(slots)))
1043+
slotDeletionMaxSize.UpdateIfGt(int64(size))
1044+
1045+
slotDeletionTimer.UpdateSince(start)
1046+
slotDeletionCount.Mark(n)
1047+
slotDeletionSize.Mark(int64(size))
10501048

1051-
slotDeletionTimer.UpdateSince(start)
1052-
slotDeletionCount.Mark(n)
1053-
slotDeletionSize.Mark(int64(size))
1054-
}
10551049
return slots, nodes, nil
10561050
}
10571051

@@ -1190,10 +1184,8 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
11901184
}
11911185
}
11921186
// Write the account trie changes, measuring the amount of wasted time
1193-
var start time.Time
1194-
if metrics.EnabledExpensive {
1195-
start = time.Now()
1196-
}
1187+
start := time.Now()
1188+
11971189
root, set, err := s.trie.Commit(true)
11981190
if err != nil {
11991191
return common.Hash{}, err
@@ -1205,23 +1197,23 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
12051197
}
12061198
accountTrieNodesUpdated, accountTrieNodesDeleted = set.Size()
12071199
}
1208-
if metrics.EnabledExpensive {
1209-
s.AccountCommits += time.Since(start)
1200+
// Report the commit metrics
1201+
s.AccountCommits += time.Since(start)
1202+
1203+
accountUpdatedMeter.Mark(int64(s.AccountUpdated))
1204+
storageUpdatedMeter.Mark(int64(s.StorageUpdated))
1205+
accountDeletedMeter.Mark(int64(s.AccountDeleted))
1206+
storageDeletedMeter.Mark(int64(s.StorageDeleted))
1207+
accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
1208+
accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
1209+
storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
1210+
storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
1211+
s.AccountUpdated, s.AccountDeleted = 0, 0
1212+
s.StorageUpdated, s.StorageDeleted = 0, 0
12101213

1211-
accountUpdatedMeter.Mark(int64(s.AccountUpdated))
1212-
storageUpdatedMeter.Mark(int64(s.StorageUpdated))
1213-
accountDeletedMeter.Mark(int64(s.AccountDeleted))
1214-
storageDeletedMeter.Mark(int64(s.StorageDeleted))
1215-
accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
1216-
accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
1217-
storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
1218-
storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
1219-
s.AccountUpdated, s.AccountDeleted = 0, 0
1220-
s.StorageUpdated, s.StorageDeleted = 0, 0
1221-
}
12221214
// If snapshotting is enabled, update the snapshot tree with this new version
12231215
if s.snap != nil {
1224-
start := time.Now()
1216+
start = time.Now()
12251217
// Only update if there's a state transition (skip empty Clique blocks)
12261218
if parent := s.snap.Root(); parent != root {
12271219
if err := s.snaps.Update(root, parent, s.convertAccountSet(s.stateObjectsDestruct), s.accounts, s.storages); err != nil {
@@ -1235,9 +1227,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
12351227
log.Warn("Failed to cap snapshot tree", "root", root, "layers", 128, "err", err)
12361228
}
12371229
}
1238-
if metrics.EnabledExpensive {
1239-
s.SnapshotCommits += time.Since(start)
1240-
}
1230+
s.SnapshotCommits += time.Since(start)
12411231
s.snap = nil
12421232
}
12431233
if root == (common.Hash{}) {
@@ -1248,15 +1238,14 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
12481238
origin = types.EmptyRootHash
12491239
}
12501240
if root != origin {
1251-
start := time.Now()
1241+
start = time.Now()
12521242
set := triestate.New(s.accountsOrigin, s.storagesOrigin)
12531243
if err := s.db.TrieDB().Update(root, origin, block, nodes, set); err != nil {
12541244
return common.Hash{}, err
12551245
}
12561246
s.originalRoot = root
1257-
if metrics.EnabledExpensive {
1258-
s.TrieDBCommits += time.Since(start)
1259-
}
1247+
s.TrieDBCommits += time.Since(start)
1248+
12601249
if s.onCommit != nil {
12611250
s.onCommit(set)
12621251
}

internal/flags/categories.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import "github.com/urfave/cli/v2"
2121
const (
2222
EthCategory = "ETHEREUM"
2323
BeaconCategory = "BEACON CHAIN"
24-
LightCategory = "LIGHT CLIENT"
2524
DevCategory = "DEVELOPER CHAIN"
2625
StateCategory = "STATE HISTORY MANAGEMENT"
2726
TxPoolCategory = "TRANSACTION POOL (EVM)"

0 commit comments

Comments
 (0)