Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd, core, metrics: always report expensive metrics #29191

Merged
merged 3 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/geth/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func applyMetricConfig(ctx *cli.Context, cfg *gethConfig) {
cfg.Metrics.Enabled = ctx.Bool(utils.MetricsEnabledFlag.Name)
}
if ctx.IsSet(utils.MetricsEnabledExpensiveFlag.Name) {
cfg.Metrics.EnabledExpensive = ctx.Bool(utils.MetricsEnabledExpensiveFlag.Name)
log.Warn("Expensive metrics are collected by default, please remove this flag", "flag", utils.MetricsEnabledExpensiveFlag.Name)
}
if ctx.IsSet(utils.MetricsHTTPFlag.Name) {
cfg.Metrics.HTTP = ctx.String(utils.MetricsHTTPFlag.Name)
Expand Down
6 changes: 0 additions & 6 deletions cmd/utils/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,12 +862,6 @@ var (
Usage: "Enable metrics collection and reporting",
Category: flags.MetricsCategory,
}
MetricsEnabledExpensiveFlag = &cli.BoolFlag{
Name: "metrics.expensive",
Usage: "Enable expensive metrics collection and reporting",
Category: flags.MetricsCategory,
}

// MetricsHTTPFlag defines the endpoint for a stand-alone metrics HTTP endpoint.
// Since the pprof service enables sensitive/vulnerable behavior, this allows a user
// to enable a public-OK metrics endpoint without having to worry about ALSO exposing
Expand Down
29 changes: 17 additions & 12 deletions cmd/utils/flags_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,35 +93,35 @@ var (
Name: "light.serve",
Usage: "Maximum percentage of time allowed for serving LES requests (deprecated)",
Value: ethconfig.Defaults.LightServ,
Category: flags.LightCategory,
Category: flags.DeprecatedCategory,
}
LightIngressFlag = &cli.IntFlag{
Name: "light.ingress",
Usage: "Incoming bandwidth limit for serving light clients (deprecated)",
Value: ethconfig.Defaults.LightIngress,
Category: flags.LightCategory,
Category: flags.DeprecatedCategory,
}
LightEgressFlag = &cli.IntFlag{
Name: "light.egress",
Usage: "Outgoing bandwidth limit for serving light clients (deprecated)",
Value: ethconfig.Defaults.LightEgress,
Category: flags.LightCategory,
Category: flags.DeprecatedCategory,
}
LightMaxPeersFlag = &cli.IntFlag{
Name: "light.maxpeers",
Usage: "Maximum number of light clients to serve, or light servers to attach to (deprecated)",
Value: ethconfig.Defaults.LightPeers,
Category: flags.LightCategory,
Category: flags.DeprecatedCategory,
}
LightNoPruneFlag = &cli.BoolFlag{
Name: "light.nopruning",
Usage: "Disable ancient light chain data pruning (deprecated)",
Category: flags.LightCategory,
Category: flags.DeprecatedCategory,
}
LightNoSyncServeFlag = &cli.BoolFlag{
Name: "light.nosyncserve",
Usage: "Enables serving light clients before syncing (deprecated)",
Category: flags.LightCategory,
Category: flags.DeprecatedCategory,
}
// Deprecated November 2023
LogBacktraceAtFlag = &cli.StringFlag{
Expand All @@ -138,19 +138,24 @@ var (
// Deprecated February 2024
MinerNewPayloadTimeoutFlag = &cli.DurationFlag{
Name: "miner.newpayload-timeout",
Usage: "Specify the maximum time allowance for creating a new payload",
Usage: "Specify the maximum time allowance for creating a new payload (deprecated)",
Value: ethconfig.Defaults.Miner.Recommit,
Category: flags.MinerCategory,
Category: flags.DeprecatedCategory,
}
MinerEtherbaseFlag = &cli.StringFlag{
Name: "miner.etherbase",
Usage: "0x prefixed public address for block mining rewards",
Category: flags.MinerCategory,
Usage: "0x prefixed public address for block mining rewards (deprecated)",
Category: flags.DeprecatedCategory,
}
MiningEnabledFlag = &cli.BoolFlag{
Name: "mine",
Usage: "Enable mining",
Category: flags.MinerCategory,
Usage: "Enable mining (deprecated)",
Category: flags.DeprecatedCategory,
}
MetricsEnabledExpensiveFlag = &cli.BoolFlag{
karalabe marked this conversation as resolved.
Show resolved Hide resolved
Name: "metrics.expensive",
Usage: "Enable expensive metrics collection and reporting (deprecated)",
Category: flags.DeprecatedCategory,
}
)

Expand Down
40 changes: 20 additions & 20 deletions core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,26 @@ var (

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

accountReadTimer = metrics.NewRegisteredTimer("chain/account/reads", nil)
accountHashTimer = metrics.NewRegisteredTimer("chain/account/hashes", nil)
accountUpdateTimer = metrics.NewRegisteredTimer("chain/account/updates", nil)
accountCommitTimer = metrics.NewRegisteredTimer("chain/account/commits", nil)

storageReadTimer = metrics.NewRegisteredTimer("chain/storage/reads", nil)
storageHashTimer = metrics.NewRegisteredTimer("chain/storage/hashes", nil)
storageUpdateTimer = metrics.NewRegisteredTimer("chain/storage/updates", nil)
storageCommitTimer = metrics.NewRegisteredTimer("chain/storage/commits", nil)

snapshotAccountReadTimer = metrics.NewRegisteredTimer("chain/snapshot/account/reads", nil)
snapshotStorageReadTimer = metrics.NewRegisteredTimer("chain/snapshot/storage/reads", nil)
snapshotCommitTimer = metrics.NewRegisteredTimer("chain/snapshot/commits", nil)

triedbCommitTimer = metrics.NewRegisteredTimer("chain/triedb/commits", nil)

blockInsertTimer = metrics.NewRegisteredTimer("chain/inserts", nil)
blockValidationTimer = metrics.NewRegisteredTimer("chain/validation", nil)
blockExecutionTimer = metrics.NewRegisteredTimer("chain/execution", nil)
blockWriteTimer = metrics.NewRegisteredTimer("chain/write", nil)
accountReadTimer = metrics.NewRegisteredResettingTimer("chain/account/reads", nil)
accountHashTimer = metrics.NewRegisteredResettingTimer("chain/account/hashes", nil)
accountUpdateTimer = metrics.NewRegisteredResettingTimer("chain/account/updates", nil)
accountCommitTimer = metrics.NewRegisteredResettingTimer("chain/account/commits", nil)

storageReadTimer = metrics.NewRegisteredResettingTimer("chain/storage/reads", nil)
storageHashTimer = metrics.NewRegisteredResettingTimer("chain/storage/hashes", nil)
storageUpdateTimer = metrics.NewRegisteredResettingTimer("chain/storage/updates", nil)
storageCommitTimer = metrics.NewRegisteredResettingTimer("chain/storage/commits", nil)

snapshotAccountReadTimer = metrics.NewRegisteredResettingTimer("chain/snapshot/account/reads", nil)
snapshotStorageReadTimer = metrics.NewRegisteredResettingTimer("chain/snapshot/storage/reads", nil)
snapshotCommitTimer = metrics.NewRegisteredResettingTimer("chain/snapshot/commits", nil)

triedbCommitTimer = metrics.NewRegisteredResettingTimer("chain/triedb/commits", nil)

blockInsertTimer = metrics.NewRegisteredResettingTimer("chain/inserts", nil)
blockValidationTimer = metrics.NewRegisteredResettingTimer("chain/validation", nil)
blockExecutionTimer = metrics.NewRegisteredResettingTimer("chain/execution", nil)
blockWriteTimer = metrics.NewRegisteredResettingTimer("chain/write", nil)

blockReorgMeter = metrics.NewRegisteredMeter("chain/reorg/executes", nil)
blockReorgAddMeter = metrics.NewRegisteredMeter("chain/reorg/add", nil)
Expand Down
26 changes: 10 additions & 16 deletions core/state/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/metrics"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/trie/trienode"
"github.com/holiman/uint256"
Expand Down Expand Up @@ -197,9 +196,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
if s.db.snap != nil {
start := time.Now()
enc, err = s.db.snap.Storage(s.addrHash, crypto.Keccak256Hash(key.Bytes()))
if metrics.EnabledExpensive {
s.db.SnapshotStorageReads += time.Since(start)
}
s.db.SnapshotStorageReads += time.Since(start)

if len(enc) > 0 {
_, content, _, err := rlp.Split(enc)
if err != nil {
Expand All @@ -217,9 +215,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
return common.Hash{}
}
val, err := tr.GetStorage(s.address, key.Bytes())
if metrics.EnabledExpensive {
s.db.StorageReads += time.Since(start)
}
s.db.StorageReads += time.Since(start)

if err != nil {
s.db.setError(err)
return common.Hash{}
Expand Down Expand Up @@ -283,9 +280,8 @@ func (s *stateObject) updateTrie() (Trie, error) {
return s.trie, nil
}
// Track the amount of time wasted on updating the storage trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())
}
defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())

// The snapshot storage map for the object
var (
storage map[common.Hash][]byte
Expand Down Expand Up @@ -370,9 +366,8 @@ func (s *stateObject) updateRoot() {
return
}
// Track the amount of time wasted on hashing the storage trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
}
defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())

s.data.Root = tr.Hash()
}

Expand All @@ -386,9 +381,8 @@ func (s *stateObject) commit() (*trienode.NodeSet, error) {
return nil, nil
}
// Track the amount of time wasted on committing the storage trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
}
defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())

// The trie is currently in an open state and could potentially contain
// cached mutations. Call commit to acquire a set of nodes that have been
// modified, the set can be nil if nothing to commit.
Expand Down
87 changes: 38 additions & 49 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/metrics"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/trie"
"github.com/ethereum/go-ethereum/trie/trienode"
Expand Down Expand Up @@ -495,9 +494,8 @@ func (s *StateDB) GetTransientState(addr common.Address, key common.Hash) common
// updateStateObject writes the given object to the trie.
func (s *StateDB) updateStateObject(obj *stateObject) {
// Track the amount of time wasted on updating the account from the trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
}
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())

// Encode the account and update the account trie
addr := obj.Address()
if err := s.trie.UpdateAccount(addr, &obj.data); err != nil {
Expand Down Expand Up @@ -527,9 +525,8 @@ func (s *StateDB) updateStateObject(obj *stateObject) {
// deleteStateObject removes the given object from the state trie.
func (s *StateDB) deleteStateObject(obj *stateObject) {
// Track the amount of time wasted on deleting the account from the trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
}
defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())

// Delete the account from the trie
addr := obj.Address()
if err := s.trie.DeleteAccount(addr); err != nil {
Expand Down Expand Up @@ -561,9 +558,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
if s.snap != nil {
start := time.Now()
acc, err := s.snap.Account(crypto.HashData(s.hasher, addr.Bytes()))
if metrics.EnabledExpensive {
s.SnapshotAccountReads += time.Since(start)
}
s.SnapshotAccountReads += time.Since(start)

if err == nil {
if acc == nil {
return nil
Expand All @@ -587,9 +583,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
start := time.Now()
var err error
data, err = s.trie.GetAccount(addr)
if metrics.EnabledExpensive {
s.AccountReads += time.Since(start)
}
s.AccountReads += time.Since(start)

if err != nil {
s.setError(fmt.Errorf("getDeleteStateObject (%x) error: %w", addr.Bytes(), err))
return nil
Expand Down Expand Up @@ -917,9 +912,8 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
s.stateObjectsPending = make(map[common.Address]struct{})
}
// Track the amount of time wasted on hashing the account trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now())
}
defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now())

return s.trie.Hash()
}

Expand Down Expand Up @@ -1042,16 +1036,16 @@ func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root
if err != nil {
return nil, nil, err
}
if metrics.EnabledExpensive {
n := int64(len(slots))
// Report the metrics
n := int64(len(slots))

slotDeletionMaxCount.UpdateIfGt(int64(len(slots)))
slotDeletionMaxSize.UpdateIfGt(int64(size))
slotDeletionMaxCount.UpdateIfGt(int64(len(slots)))
slotDeletionMaxSize.UpdateIfGt(int64(size))

slotDeletionTimer.UpdateSince(start)
slotDeletionCount.Mark(n)
slotDeletionSize.Mark(int64(size))

slotDeletionTimer.UpdateSince(start)
slotDeletionCount.Mark(n)
slotDeletionSize.Mark(int64(size))
}
return slots, nodes, nil
}

Expand Down Expand Up @@ -1190,10 +1184,8 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
}
}
// Write the account trie changes, measuring the amount of wasted time
var start time.Time
if metrics.EnabledExpensive {
start = time.Now()
}
start := time.Now()

root, set, err := s.trie.Commit(true)
if err != nil {
return common.Hash{}, err
Expand All @@ -1205,23 +1197,23 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
}
accountTrieNodesUpdated, accountTrieNodesDeleted = set.Size()
}
if metrics.EnabledExpensive {
s.AccountCommits += time.Since(start)
// Report the commit metrics
s.AccountCommits += time.Since(start)

accountUpdatedMeter.Mark(int64(s.AccountUpdated))
storageUpdatedMeter.Mark(int64(s.StorageUpdated))
accountDeletedMeter.Mark(int64(s.AccountDeleted))
storageDeletedMeter.Mark(int64(s.StorageDeleted))
accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
s.AccountUpdated, s.AccountDeleted = 0, 0
s.StorageUpdated, s.StorageDeleted = 0, 0

accountUpdatedMeter.Mark(int64(s.AccountUpdated))
storageUpdatedMeter.Mark(int64(s.StorageUpdated))
accountDeletedMeter.Mark(int64(s.AccountDeleted))
storageDeletedMeter.Mark(int64(s.StorageDeleted))
accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
s.AccountUpdated, s.AccountDeleted = 0, 0
s.StorageUpdated, s.StorageDeleted = 0, 0
}
// If snapshotting is enabled, update the snapshot tree with this new version
if s.snap != nil {
start := time.Now()
start = time.Now()
// Only update if there's a state transition (skip empty Clique blocks)
if parent := s.snap.Root(); parent != root {
if err := s.snaps.Update(root, parent, s.convertAccountSet(s.stateObjectsDestruct), s.accounts, s.storages); err != nil {
Expand All @@ -1235,9 +1227,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
log.Warn("Failed to cap snapshot tree", "root", root, "layers", 128, "err", err)
}
}
if metrics.EnabledExpensive {
s.SnapshotCommits += time.Since(start)
}
s.SnapshotCommits += time.Since(start)
s.snap = nil
}
if root == (common.Hash{}) {
Expand All @@ -1248,15 +1238,14 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
origin = types.EmptyRootHash
}
if root != origin {
start := time.Now()
start = time.Now()
set := triestate.New(s.accountsOrigin, s.storagesOrigin)
if err := s.db.TrieDB().Update(root, origin, block, nodes, set); err != nil {
return common.Hash{}, err
}
s.originalRoot = root
if metrics.EnabledExpensive {
s.TrieDBCommits += time.Since(start)
}
s.TrieDBCommits += time.Since(start)

if s.onCommit != nil {
s.onCommit(set)
}
Expand Down
1 change: 0 additions & 1 deletion internal/flags/categories.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import "github.com/urfave/cli/v2"
const (
EthCategory = "ETHEREUM"
BeaconCategory = "BEACON CHAIN"
LightCategory = "LIGHT CLIENT"
DevCategory = "DEVELOPER CHAIN"
StateCategory = "STATE HISTORY MANAGEMENT"
TxPoolCategory = "TRANSACTION POOL (EVM)"
Expand Down
Loading
Loading