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

refactor: remove global metrics in store #14439

Merged
merged 8 commits into from
Dec 30, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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
3 changes: 2 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/store"
storemetrics "github.com/cosmos/cosmos-sdk/store/metrics"
"github.com/cosmos/cosmos-sdk/store/snapshots"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -156,7 +157,7 @@ func NewBaseApp(
logger: logger,
name: name,
db: db,
cms: store.NewCommitMultiStore(db, logger),
cms: store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()), // by default we use a no-op metric gather in store
storeLoader: DefaultStoreLoader,
grpcQueryRouter: NewGRPCQueryRouter(),
msgServiceRouter: NewMsgServiceRouter(),
Expand Down
5 changes: 3 additions & 2 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/store/metrics"
pruningtypes "github.com/cosmos/cosmos-sdk/store/pruning/types"
"github.com/cosmos/cosmos-sdk/store/rootmulti"
"github.com/cosmos/cosmos-sdk/store/snapshots"
Expand Down Expand Up @@ -217,7 +218,7 @@ func TestSetLoader(t *testing.T) {
}

initStore := func(t *testing.T, db dbm.DB, storeKey string, k, v []byte) {
rs := rootmulti.NewStore(db, log.NewNopLogger())
rs := rootmulti.NewStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
rs.SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningNothing))

key := sdk.NewKVStoreKey(storeKey)
Expand All @@ -237,7 +238,7 @@ func TestSetLoader(t *testing.T) {
}

checkStore := func(t *testing.T, db dbm.DB, ver int64, storeKey string, k, v []byte) {
rs := rootmulti.NewStore(db, log.NewNopLogger())
rs := rootmulti.NewStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
rs.SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningDefault))

key := sdk.NewKVStoreKey(storeKey)
Expand Down
10 changes: 10 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec/types"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
"github.com/cosmos/cosmos-sdk/store"
"github.com/cosmos/cosmos-sdk/store/metrics"
pruningtypes "github.com/cosmos/cosmos-sdk/store/pruning/types"
"github.com/cosmos/cosmos-sdk/store/snapshots"
snapshottypes "github.com/cosmos/cosmos-sdk/store/snapshots/types"
Expand Down Expand Up @@ -297,3 +298,12 @@ func (app *BaseApp) SetPrepareProposal(handler sdk.PrepareProposalHandler) {

app.prepareProposal = handler
}

// SetStoreMetrics sets the prepare proposal function for the BaseApp.
func (app *BaseApp) SetStoreMetrics(gatherer metrics.StoreMetrics) {
if app.sealed {
panic("SetStoreMetrics() on sealed BaseApp")
}

app.cms.SetMetrics(gatherer)
}
5 changes: 5 additions & 0 deletions server/mock/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
dbm "github.com/cosmos/cosmos-db"
protoio "github.com/cosmos/gogoproto/io"

"github.com/cosmos/cosmos-sdk/store/metrics"
pruningtypes "github.com/cosmos/cosmos-sdk/store/pruning/types"
snapshottypes "github.com/cosmos/cosmos-sdk/store/snapshots/types"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
Expand Down Expand Up @@ -54,6 +55,10 @@ func (ms multiStore) AddListeners(key storetypes.StoreKey, listeners []storetype
panic("not implemented")
}

func (ms multiStore) SetMetrics(metrics.StoreMetrics) {
panic("not implemented")
}

func (ms multiStore) ListeningEnabled(key storetypes.StoreKey) bool {
panic("not implemented")
}
Expand Down
5 changes: 3 additions & 2 deletions store/cachekv/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import (

dbm "github.com/cosmos/cosmos-db"
"github.com/cosmos/cosmos-sdk/store"
"github.com/cosmos/cosmos-sdk/store/metrics"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdk "github.com/cosmos/cosmos-sdk/types" // TODO figure out how to remove
"github.com/tendermint/tendermint/libs/log"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
)
Expand All @@ -18,7 +19,7 @@ func DoBenchmarkDeepContextStack(b *testing.B, depth int) {
key := storetypes.NewKVStoreKey("test")

db := dbm.NewMemDB()
cms := store.NewCommitMultiStore(db, log.NewNopLogger())
cms := store.NewCommitMultiStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
cms.MountStoreWithDB(key, storetypes.StoreTypeIAVL, db)
cms.LoadLatestVersion()
ctx := sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger())
Expand Down
35 changes: 19 additions & 16 deletions store/iavl/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"fmt"
"io"
"time"

ics23 "github.com/confio/ics23/go"
dbm "github.com/cosmos/cosmos-db"
Expand All @@ -16,10 +15,10 @@ import (
sdkerrors "cosmossdk.io/errors"
"github.com/cosmos/cosmos-sdk/store/cachekv"
"github.com/cosmos/cosmos-sdk/store/internal/kv"
"github.com/cosmos/cosmos-sdk/store/metrics"
pruningtypes "github.com/cosmos/cosmos-sdk/store/pruning/types"
"github.com/cosmos/cosmos-sdk/store/tracekv"
"github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/telemetry"
)

const (
Expand All @@ -36,22 +35,23 @@ var (

// Store Implements types.KVStore and CommitKVStore.
type Store struct {
tree Tree
logger log.Logger
tree Tree
logger log.Logger
metrics metrics.StoreMetrics
}

// LoadStore returns an IAVL Store as a CommitKVStore. Internally, it will load the
// store's version (id) from the provided DB. An error is returned if the version
// fails to load, or if called with a positive version on an empty tree.
func LoadStore(db dbm.DB, logger log.Logger, key types.StoreKey, id types.CommitID, lazyLoading bool, cacheSize int, disableFastNode bool) (types.CommitKVStore, error) {
return LoadStoreWithInitialVersion(db, logger, key, id, lazyLoading, 0, cacheSize, disableFastNode)
func LoadStore(db dbm.DB, logger log.Logger, key types.StoreKey, id types.CommitID, lazyLoading bool, cacheSize int, disableFastNode bool, metrics metrics.StoreMetrics) (types.CommitKVStore, error) {
return LoadStoreWithInitialVersion(db, logger, key, id, lazyLoading, 0, cacheSize, disableFastNode, metrics)
}

// LoadStoreWithInitialVersion returns an IAVL Store as a CommitKVStore setting its initialVersion
// to the one given. Internally, it will load the store's version (id) from the
// provided DB. An error is returned if the version fails to load, or if called with a positive
// version on an empty tree.
func LoadStoreWithInitialVersion(db dbm.DB, logger log.Logger, key types.StoreKey, id types.CommitID, lazyLoading bool, initialVersion uint64, cacheSize int, disableFastNode bool) (types.CommitKVStore, error) {
func LoadStoreWithInitialVersion(db dbm.DB, logger log.Logger, key types.StoreKey, id types.CommitID, lazyLoading bool, initialVersion uint64, cacheSize int, disableFastNode bool, metrics metrics.StoreMetrics) (types.CommitKVStore, error) {
tree, err := iavl.NewMutableTreeWithOpts(db, cacheSize, &iavl.Options{InitialVersion: initialVersion}, disableFastNode)
if err != nil {
return nil, err
Expand Down Expand Up @@ -87,8 +87,9 @@ func LoadStoreWithInitialVersion(db dbm.DB, logger log.Logger, key types.StoreKe
}

return &Store{
tree: tree,
logger: logger,
tree: tree,
logger: logger,
metrics: metrics,
}, nil
}

Expand All @@ -100,7 +101,8 @@ func LoadStoreWithInitialVersion(db dbm.DB, logger log.Logger, key types.StoreKe
// passed into iavl.MutableTree
func UnsafeNewStore(tree *iavl.MutableTree) *Store {
return &Store{
tree: tree,
tree: tree,
metrics: metrics.NewNoOpMetrics(),
}
}

Expand All @@ -120,14 +122,15 @@ func (st *Store) GetImmutable(version int64) (*Store, error) {
}

return &Store{
tree: &immutableTree{iTree},
tree: &immutableTree{iTree},
metrics: st.metrics,
}, nil
}

// Commit commits the current store state and returns a CommitID with the new
// version and hash.
func (st *Store) Commit() types.CommitID {
defer telemetry.MeasureSince(time.Now(), "store", "iavl", "commit")
defer st.metrics.MeasureSince("store", "iavl", "commit")

hash, version, err := st.tree.SaveVersion()
if err != nil {
Expand Down Expand Up @@ -202,7 +205,7 @@ func (st *Store) Set(key, value []byte) {

// Implements types.KVStore.
func (st *Store) Get(key []byte) []byte {
defer telemetry.MeasureSince(time.Now(), "store", "iavl", "get")
defer st.metrics.MeasureSince("store", "iavl", "get")
value, err := st.tree.Get(key)
if err != nil {
panic(err)
Expand All @@ -212,7 +215,7 @@ func (st *Store) Get(key []byte) []byte {

// Implements types.KVStore.
func (st *Store) Has(key []byte) (exists bool) {
defer telemetry.MeasureSince(time.Now(), "store", "iavl", "has")
defer st.metrics.MeasureSince("store", "iavl", "has")
has, err := st.tree.Has(key)
if err != nil {
panic(err)
Expand All @@ -222,7 +225,7 @@ func (st *Store) Has(key []byte) (exists bool) {

// Implements types.KVStore.
func (st *Store) Delete(key []byte) {
defer telemetry.MeasureSince(time.Now(), "store", "iavl", "delete")
defer st.metrics.MeasureSince("store", "iavl", "delete")
st.tree.Remove(key)
}

Expand Down Expand Up @@ -311,7 +314,7 @@ func getHeight(tree Tree, req abci.RequestQuery) int64 {
// if you care to have the latest data to see a tx results, you must
// explicitly set the height you want to see
func (st *Store) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
defer telemetry.MeasureSince(time.Now(), "store", "iavl", "query")
defer st.metrics.MeasureSince("store", "iavl", "query")

if len(req.Data) == 0 {
return types.QueryResult(sdkerrors.Wrap(types.ErrTxDecode, "query cannot be zero length"), false)
Expand Down
7 changes: 4 additions & 3 deletions store/iavl/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/cosmos/cosmos-sdk/store/cachekv"
"github.com/cosmos/cosmos-sdk/store/internal/kv"
"github.com/cosmos/cosmos-sdk/store/metrics"
"github.com/cosmos/cosmos-sdk/store/types"
)

Expand Down Expand Up @@ -99,17 +100,17 @@ func TestLoadStore(t *testing.T) {
require.Equal(t, string(hcStore.Get([]byte("hello"))), "ciao")

// Querying a new store at some previous non-pruned height H
newHStore, err := LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), cIDH, false, DefaultIAVLCacheSize, false)
newHStore, err := LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), cIDH, false, DefaultIAVLCacheSize, false, metrics.NewNoOpMetrics())
require.NoError(t, err)
require.Equal(t, string(newHStore.Get([]byte("hello"))), "hallo")

// Querying a new store at some previous pruned height Hp
newHpStore, err := LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), cIDHp, false, DefaultIAVLCacheSize, false)
newHpStore, err := LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), cIDHp, false, DefaultIAVLCacheSize, false, metrics.NewNoOpMetrics())
require.NoError(t, err)
require.Equal(t, string(newHpStore.Get([]byte("hello"))), "hola")

// Querying a new store at current height H
newHcStore, err := LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), cIDHc, false, DefaultIAVLCacheSize, false)
newHcStore, err := LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), cIDHc, false, DefaultIAVLCacheSize, false, metrics.NewNoOpMetrics())
require.NoError(t, err)
require.Equal(t, string(newHcStore.Get([]byte("hello"))), "ciao")
}
Expand Down
54 changes: 54 additions & 0 deletions store/metrics/telemetry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package metrics

import (
"time"

"github.com/armon/go-metrics"
)

// StoreMetrics defines the set of metrics for the store package
type StoreMetrics interface {
MeasureSince(keys ...string)
}

var (
_ StoreMetrics = Metrics{}
_ StoreMetrics = NoOpMetrics{}
)

// Metrics defines the metrics wrapper for the store package
type Metrics struct {
Labels []metrics.Label
}

func NewMetrics(labels [][]string) Metrics {
gatherer := Metrics{}

if numGlobalLables := len(labels); numGlobalLables > 0 {
parsedGlobalLabels := make([]metrics.Label, numGlobalLables)
for i, gl := range labels {
parsedGlobalLabels[i] = metrics.Label{Name: gl[0], Value: gl[1]}
}

gatherer.Labels = parsedGlobalLabels
}

return gatherer
}

// MeasureSince provides a wrapper functionality for emitting a a time measure
// metric with global labels (if any).
func (m Metrics) MeasureSince(keys ...string) {
start := time.Now()

Check warning

Code scanning / CodeQL

Calling the system time

Calling the system time may be a possible source of non-determinism
metrics.MeasureSinceWithLabels(keys, start.UTC(), m.Labels)
}

// NoOpMetrics is a no-op implementation of the StoreMetrics interface
type NoOpMetrics struct{}

func NewNoOpMetrics() NoOpMetrics {
return NoOpMetrics{}
}

// MeasureSince is a no-op implementation of the StoreMetrics interface to avoid time.Now() calls
func (m NoOpMetrics) MeasureSince(keys ...string) {}
7 changes: 4 additions & 3 deletions store/rootmulti/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import (
"github.com/tendermint/tendermint/libs/log"

"github.com/cosmos/cosmos-sdk/store/iavl"
"github.com/cosmos/cosmos-sdk/store/metrics"
"github.com/cosmos/cosmos-sdk/store/types"
)

func TestVerifyIAVLStoreQueryProof(t *testing.T) {
// Create main tree for testing.
db := dbm.NewMemDB()
iStore, err := iavl.LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), types.CommitID{}, false, iavl.DefaultIAVLCacheSize, false)
iStore, err := iavl.LoadStore(db, log.NewNopLogger(), types.NewKVStoreKey("test"), types.CommitID{}, false, iavl.DefaultIAVLCacheSize, false, metrics.NewNoOpMetrics())
store := iStore.(*iavl.Store)
require.Nil(t, err)
store.Set([]byte("MYKEY"), []byte("MYVALUE"))
Expand Down Expand Up @@ -58,7 +59,7 @@ func TestVerifyIAVLStoreQueryProof(t *testing.T) {
func TestVerifyMultiStoreQueryProof(t *testing.T) {
// Create main tree for testing.
db := dbm.NewMemDB()
store := NewStore(db, log.NewNopLogger())
store := NewStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
iavlStoreKey := types.NewKVStoreKey("iavlStoreKey")

store.MountStoreWithDB(iavlStoreKey, types.StoreTypeIAVL, nil)
Expand Down Expand Up @@ -113,7 +114,7 @@ func TestVerifyMultiStoreQueryProof(t *testing.T) {
func TestVerifyMultiStoreQueryProofAbsence(t *testing.T) {
// Create main tree for testing.
db := dbm.NewMemDB()
store := NewStore(db, log.NewNopLogger())
store := NewStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
iavlStoreKey := types.NewKVStoreKey("iavlStoreKey")

store.MountStoreWithDB(iavlStoreKey, types.StoreTypeIAVL, nil)
Expand Down
9 changes: 5 additions & 4 deletions store/rootmulti/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

dbm "github.com/cosmos/cosmos-db"
"github.com/cosmos/cosmos-sdk/store/iavl"
"github.com/cosmos/cosmos-sdk/store/metrics"
"github.com/cosmos/cosmos-sdk/store/rootmulti"
"github.com/cosmos/cosmos-sdk/store/snapshots"
snapshottypes "github.com/cosmos/cosmos-sdk/store/snapshots/types"
Expand All @@ -23,7 +24,7 @@ import (
)

func newMultiStoreWithGeneratedData(db dbm.DB, stores uint8, storeKeys uint64) *rootmulti.Store {
multiStore := rootmulti.NewStore(db, log.NewNopLogger())
multiStore := rootmulti.NewStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
r := rand.New(rand.NewSource(49872768940)) // Fixed seed for deterministic tests

keys := []*types.KVStoreKey{}
Expand Down Expand Up @@ -55,7 +56,7 @@ func newMultiStoreWithGeneratedData(db dbm.DB, stores uint8, storeKeys uint64) *
}

func newMultiStoreWithMixedMounts(db dbm.DB) *rootmulti.Store {
store := rootmulti.NewStore(db, log.NewNopLogger())
store := rootmulti.NewStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
store.MountStoreWithDB(types.NewKVStoreKey("iavl1"), types.StoreTypeIAVL, nil)
store.MountStoreWithDB(types.NewKVStoreKey("iavl2"), types.StoreTypeIAVL, nil)
store.MountStoreWithDB(types.NewKVStoreKey("iavl3"), types.StoreTypeIAVL, nil)
Expand Down Expand Up @@ -235,7 +236,7 @@ func benchmarkMultistoreSnapshot(b *testing.B, stores uint8, storeKeys uint64) {
b.StartTimer()

for i := 0; i < b.N; i++ {
target := rootmulti.NewStore(dbm.NewMemDB(), log.NewNopLogger())
target := rootmulti.NewStore(dbm.NewMemDB(), log.NewNopLogger(), metrics.NewNoOpMetrics())
for _, key := range source.StoreKeysByName() {
target.MountStoreWithDB(key, types.StoreTypeIAVL, nil)
}
Expand Down Expand Up @@ -270,7 +271,7 @@ func benchmarkMultistoreSnapshotRestore(b *testing.B, stores uint8, storeKeys ui
b.StartTimer()

for i := 0; i < b.N; i++ {
target := rootmulti.NewStore(dbm.NewMemDB(), log.NewNopLogger())
target := rootmulti.NewStore(dbm.NewMemDB(), log.NewNopLogger(), metrics.NewNoOpMetrics())
for _, key := range source.StoreKeysByName() {
target.MountStoreWithDB(key, types.StoreTypeIAVL, nil)
}
Expand Down
Loading