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

R4R: Store Refactor 1 #2985

Merged
merged 36 commits into from
Feb 2, 2019
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
a669428
in progress
mossid Dec 3, 2018
83dac5d
store/* now compiles
mossid Dec 3, 2018
fe1209a
make store/gas independent from types/, now pass ci
mossid Dec 3, 2018
c40055e
remove residue, rename files to match with convention
mossid Dec 3, 2018
ff0e9dc
fix lint
mossid Dec 3, 2018
b08044a
replace dependency types/ -> store/types/
mossid Dec 3, 2018
26e05ae
update pending
mossid Dec 3, 2018
324a849
fix gas.NewStore argument order
mossid Dec 3, 2018
9d806d6
rename {gas, cache, trace} -> {gas, cache, trace}kv
mossid Dec 5, 2018
cb8006b
Merge branch 'develop' into joon/store-refactor-1
cwgoes Dec 5, 2018
9876e30
adress comments in progress
mossid Dec 14, 2018
676e837
Merge branch 'joon/store-refactor-1' of github.com:cosmos/cosmos-sdk …
mossid Dec 14, 2018
ab8143e
Merge branch 'develop' into joon/store-refactor-1
mossid Dec 14, 2018
86b19d4
fix server
mossid Dec 14, 2018
2c316e3
address comments in progress
mossid Dec 14, 2018
43ab0c3
Merge branch 'develop' into joon/store-refactor-1
jackzampolin Dec 20, 2018
30a4a58
addressing comments
mossid Dec 29, 2018
4eda02b
address comments
mossid Jan 5, 2019
04631ad
fix test
mossid Jan 5, 2019
2429ef4
Merge branch 'develop' into joon/store-refactor-1
mossid Jan 21, 2019
cad00ca
address comments
mossid Jan 25, 2019
a1aaba3
Merge branch 'develop' into joon/store-refactor-1
mossid Jan 25, 2019
d2e8044
Merge branch 'develop' of github.com:cosmos/cosmos-sdk into joon/stor…
mossid Jan 25, 2019
fe063b3
mv Cp() back to store
mossid Jan 25, 2019
20e02f7
Update store/cachemulti/store.go
melekes Jan 29, 2019
67c5541
Update store/dbadapter/store.go
melekes Jan 29, 2019
11c202a
address comments in progress
mossid Jan 29, 2019
1e68319
Merge branch 'joon/store-refactor-1' of github.com:cosmos/cosmos-sdk …
mossid Jan 29, 2019
e1f3fec
address comments
mossid Jan 29, 2019
9b16f2f
Merge branch 'develop' into joon/store-refactor-1
Jan 30, 2019
13470c3
add README, fix lint, add AssertValidKey in prefix.Store
mossid Jan 30, 2019
997d88d
Merge branch 'joon/store-refactor-1' of github.com:cosmos/cosmos-sdk …
mossid Jan 30, 2019
af69aeb
modify README
mossid Jan 30, 2019
7cd7880
Update store/README.md
cwgoes Jan 31, 2019
ab82454
Update store/README.md
cwgoes Jan 31, 2019
ae9eeb9
address comments
mossid Jan 31, 2019
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
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ IMPROVEMENTS
* Gaia

* SDK
* [\#2986](https://github.com/cosmos/cosmos-sdk/pull/2986) Store Refactor

* Tendermint

Expand Down
10 changes: 5 additions & 5 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (app *BaseApp) Name() string {
// SetCommitMultiStoreTracer sets the store tracer on the BaseApp's underlying
// CommitMultiStore.
func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) {
app.cms.WithTracer(w)
app.cms.SetTracer(w)
}

// Mount IAVL or DB stores to the provided keys in the BaseApp multistore
Expand Down Expand Up @@ -483,8 +483,8 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
// BeginBlock implements the ABCI application interface.
func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeginBlock) {
if app.cms.TracingEnabled() {
app.cms.ResetTraceContext()
app.cms.WithTracingContext(sdk.TraceContext(
app.cms.SetTracingContext(nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we need to set the context to nil if we set it to sdk.TraceContext the next line?

app.cms.SetTracingContext(sdk.TraceContext(
map[string]interface{}{"blockHeight": req.Header.Height},
))
}
Expand Down Expand Up @@ -679,7 +679,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (
// TODO: https://github.com/cosmos/cosmos-sdk/issues/2824
msCache := ms.CacheMultiStore()
if msCache.TracingEnabled() {
msCache = msCache.WithTracingContext(
msCache = msCache.SetTracingContext(
sdk.TraceContext(
map[string]interface{}{
"txHash": fmt.Sprintf("%X", tmhash.Sum(txBytes)),
Expand Down Expand Up @@ -813,7 +813,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
// EndBlock implements the ABCI application interface.
func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBlock) {
if app.deliverState.ms.TracingEnabled() {
app.deliverState.ms = app.deliverState.ms.ResetTraceContext().(sdk.CacheMultiStore)
app.deliverState.ms = app.deliverState.ms.SetTracingContext(nil).(sdk.CacheMultiStore)
}

if app.endBlocker != nil {
Expand Down
2 changes: 1 addition & 1 deletion baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"os"
"testing"

"github.com/cosmos/cosmos-sdk/store"
store "github.com/cosmos/cosmos-sdk/store/types"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down
7 changes: 3 additions & 4 deletions client/context/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ import (

"strings"

"github.com/cosmos/cosmos-sdk/store/rootmulti"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/crypto/merkle"
cmn "github.com/tendermint/tendermint/libs/common"
tmliteErr "github.com/tendermint/tendermint/lite/errors"
tmliteProxy "github.com/tendermint/tendermint/lite/proxy"
rpcclient "github.com/tendermint/tendermint/rpc/client"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/cosmos/cosmos-sdk/store"
)

// GetNode returns an RPC client. If the context's client is not defined, an
Expand Down Expand Up @@ -211,7 +210,7 @@ func (ctx CLIContext) verifyProof(queryPath string, resp abci.ResponseQuery) err
}

// TODO: Instead of reconstructing, stash on CLIContext field?
prt := store.DefaultProofRuntime()
prt := rootmulti.DefaultProofRuntime()

// TODO: Better convention for path?
storeName, err := parseQueryStorePath(queryPath)
Expand Down Expand Up @@ -251,7 +250,7 @@ func isQueryStoreWithProof(path string) bool {
return false
case paths[0] != "store":
return false
case store.RequireProof("/" + paths[2]):
case rootmulti.RequireProof("/" + paths[2]):
return true
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/gaia/cmd/gaiad/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func main() {
func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer) abci.Application {
return app.NewGaiaApp(
logger, db, traceStore, true,
baseapp.SetPruning(store.NewPruningOptions(viper.GetString("pruning"))),
baseapp.SetPruning(store.NewPruningOptionsFromString(viper.GetString("pruning"))),
baseapp.SetMinGasPrices(viper.GetString(server.FlagMinGasPrices)),
)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/gaia/cmd/gaiadebug/hack.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func runHackCmd(cmd *cobra.Command, args []string) error {
fmt.Println(err)
os.Exit(1)
}
app := NewGaiaApp(logger, db, baseapp.SetPruning(store.NewPruningOptions(viper.GetString("pruning"))))
app := NewGaiaApp(logger, db, baseapp.SetPruning(store.NewPruningOptionsFromString(viper.GetString("pruning"))))

// print some info
id := app.LastCommitID()
Expand Down
8 changes: 2 additions & 6 deletions server/mock/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,15 @@ func (ms multiStore) CacheWrapWithTrace(_ io.Writer, _ sdk.TraceContext) sdk.Cac
panic("not implemented")
}

func (ms multiStore) ResetTraceContext() sdk.MultiStore {
panic("not implemented")
}

func (ms multiStore) TracingEnabled() bool {
panic("not implemented")
}

func (ms multiStore) WithTracingContext(tc sdk.TraceContext) sdk.MultiStore {
func (ms multiStore) SetTracingContext(tc sdk.TraceContext) sdk.MultiStore {
panic("not implemented")
}

func (ms multiStore) WithTracer(w io.Writer) sdk.MultiStore {
func (ms multiStore) SetTracer(w io.Writer) sdk.MultiStore {
panic("not implemented")
}

Expand Down
2 changes: 1 addition & 1 deletion store/memiterator.go → store/cachekv/memiterator.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package store
package cachekv

import (
"bytes"
Expand Down
12 changes: 7 additions & 5 deletions store/cachemergeiterator.go → store/cachekv/mergeiterator.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package store
package cachekv

import (
"bytes"

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

// cacheMergeIterator merges a parent Iterator and a cache Iterator.
Expand All @@ -12,14 +14,14 @@ import (
//
// TODO: Optimize by memoizing.
type cacheMergeIterator struct {
parent Iterator
cache Iterator
parent types.Iterator
cache types.Iterator
ascending bool
}

var _ Iterator = (*cacheMergeIterator)(nil)
var _ types.Iterator = (*cacheMergeIterator)(nil)

func newCacheMergeIterator(parent, cache Iterator, ascending bool) *cacheMergeIterator {
func newCacheMergeIterator(parent, cache types.Iterator, ascending bool) *cacheMergeIterator {
iter := &cacheMergeIterator{
parent: parent,
cache: cache,
Expand Down
80 changes: 37 additions & 43 deletions store/cachekvstore.go → store/cachekv/store.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package store
package cachekv

import (
"bytes"
Expand All @@ -8,6 +8,10 @@ import (

cmn "github.com/tendermint/tendermint/libs/common"
dbm "github.com/tendermint/tendermint/libs/db"

"github.com/cosmos/cosmos-sdk/store/types"

"github.com/cosmos/cosmos-sdk/store/tracekv"
)

// If value is nil but deleted is false, it means the parent doesn't have the
Expand All @@ -18,30 +22,30 @@ type cValue struct {
dirty bool
}

// cacheKVStore wraps an in-memory cache around an underlying KVStore.
type cacheKVStore struct {
// Store wraps an in-memory cache around an underlying types.KVStore.
type Store struct {
mtx sync.Mutex
cache map[string]cValue
parent KVStore
parent types.KVStore
}

var _ CacheKVStore = (*cacheKVStore)(nil)
var _ types.CacheKVStore = (*Store)(nil)

// nolint
func NewCacheKVStore(parent KVStore) *cacheKVStore {
return &cacheKVStore{
func NewStore(parent types.KVStore) *Store {
return &Store{
cache: make(map[string]cValue),
parent: parent,
}
}

// Implements Store.
func (ci *cacheKVStore) GetStoreType() StoreType {
func (ci *Store) GetStoreType() types.StoreType {
mossid marked this conversation as resolved.
Show resolved Hide resolved
return ci.parent.GetStoreType()
}

// Implements KVStore.
func (ci *cacheKVStore) Get(key []byte) (value []byte) {
// Implements types.KVStore.
func (ci *Store) Get(key []byte) (value []byte) {
ci.mtx.Lock()
defer ci.mtx.Unlock()
ci.assertValidKey(key)
Expand All @@ -57,8 +61,8 @@ func (ci *cacheKVStore) Get(key []byte) (value []byte) {
return value
}

// Implements KVStore.
func (ci *cacheKVStore) Set(key []byte, value []byte) {
// Implements types.KVStore.
func (ci *Store) Set(key []byte, value []byte) {
ci.mtx.Lock()
defer ci.mtx.Unlock()
ci.assertValidKey(key)
Expand All @@ -67,33 +71,23 @@ func (ci *cacheKVStore) Set(key []byte, value []byte) {
ci.setCacheValue(key, value, false, true)
}

// Implements KVStore.
func (ci *cacheKVStore) Has(key []byte) bool {
// Implements types.KVStore.
func (ci *Store) Has(key []byte) bool {
value := ci.Get(key)
return value != nil
}

// Implements KVStore.
func (ci *cacheKVStore) Delete(key []byte) {
// Implements types.KVStore.
func (ci *Store) Delete(key []byte) {
ci.mtx.Lock()
defer ci.mtx.Unlock()
ci.assertValidKey(key)

ci.setCacheValue(key, nil, true, true)
}

// Implements KVStore
func (ci *cacheKVStore) Prefix(prefix []byte) KVStore {
return prefixStore{ci, prefix}
}

// Implements KVStore
func (ci *cacheKVStore) Gas(meter GasMeter, config GasConfig) KVStore {
return NewGasKVStore(meter, config, ci)
}

// Implements CacheKVStore.
func (ci *cacheKVStore) Write() {
// Implements Cachetypes.KVStore.
func (ci *Store) Write() {
ci.mtx.Lock()
defer ci.mtx.Unlock()

Expand Down Expand Up @@ -126,33 +120,33 @@ func (ci *cacheKVStore) Write() {
}

//----------------------------------------
// To cache-wrap this cacheKVStore further.
// To cache-wrap this Store further.

// Implements CacheWrapper.
func (ci *cacheKVStore) CacheWrap() CacheWrap {
return NewCacheKVStore(ci)
func (ci *Store) CacheWrap() types.CacheWrap {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to cache-wrap an already cached store? I recall there are places where we do this, but it leads to difficult debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will need multiple layers of cache wrapping in some cases(one of the examples is https://github.com/cosmos/cosmos-sdk/blob/develop/types/context.go#L235), although I'm not advocating it hard.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK, but SDK applications should be careful about repeatedly cache-wrapping.

return NewStore(ci)
}

// CacheWrapWithTrace implements the CacheWrapper interface.
func (ci *cacheKVStore) CacheWrapWithTrace(w io.Writer, tc TraceContext) CacheWrap {
return NewCacheKVStore(NewTraceKVStore(ci, w, tc))
func (ci *Store) CacheWrapWithTrace(w io.Writer, tc types.TraceContext) types.CacheWrap {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

return NewStore(tracekv.NewStore(ci, w, tc))
}

//----------------------------------------
// Iteration

// Implements KVStore.
func (ci *cacheKVStore) Iterator(start, end []byte) Iterator {
// Implements types.KVStore.
func (ci *Store) Iterator(start, end []byte) types.Iterator {
return ci.iterator(start, end, true)
}

// Implements KVStore.
func (ci *cacheKVStore) ReverseIterator(start, end []byte) Iterator {
// Implements types.KVStore.
func (ci *Store) ReverseIterator(start, end []byte) types.Iterator {
return ci.iterator(start, end, false)
}

func (ci *cacheKVStore) iterator(start, end []byte, ascending bool) Iterator {
var parent, cache Iterator
func (ci *Store) iterator(start, end []byte, ascending bool) types.Iterator {
var parent, cache types.Iterator

if ascending {
parent = ci.parent.Iterator(start, end)
Expand All @@ -167,7 +161,7 @@ func (ci *cacheKVStore) iterator(start, end []byte, ascending bool) Iterator {
}

// Constructs a slice of dirty items, to use w/ memIterator.
func (ci *cacheKVStore) dirtyItems(start, end []byte, ascending bool) []cmn.KVPair {
func (ci *Store) dirtyItems(start, end []byte, ascending bool) []cmn.KVPair {
items := make([]cmn.KVPair, 0)

for key, cacheValue := range ci.cache {
Expand All @@ -192,20 +186,20 @@ func (ci *cacheKVStore) dirtyItems(start, end []byte, ascending bool) []cmn.KVPa
//----------------------------------------
// etc

func (ci *cacheKVStore) assertValidKey(key []byte) {
func (ci *Store) assertValidKey(key []byte) {
if key == nil {
panic("key is nil")
}
}

func (ci *cacheKVStore) assertValidValue(value []byte) {
func (ci *Store) assertValidValue(value []byte) {
if value == nil {
panic("value is nil")
}
}

// Only entrypoint to mutate ci.cache.
func (ci *cacheKVStore) setCacheValue(key, value []byte, deleted bool, dirty bool) {
func (ci *Store) setCacheValue(key, value []byte, deleted bool, dirty bool) {
ci.cache[string(key)] = cValue{
value: value,
deleted: deleted,
Expand Down
Loading