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

Problem: memiavl don't support CacheMultiStoreWithVersion #1114

Merged
merged 4 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- [#1100](https://github.com/crypto-org-chain/cronos/pull/1100) memiavl support read-only mode, and grab exclusive lock for write mode.
- [#1103](https://github.com/crypto-org-chain/cronos/pull/1103) Add EventQueryTxFor cmd to subscribe and wait for transaction.
- [#1108](https://github.com/crypto-org-chain/cronos/pull/1108) versiondb support restore from local snapshot.
- [#1114](https://github.com/crypto-org-chain/cronos/pull/1114) memiavl support `CacheMultiStoreWithVersion`.

### Improvements

Expand Down
4 changes: 2 additions & 2 deletions memiavl/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ func (db *DB) Snapshot(height uint64, protoWriter protoio.Writer) (returnErr err
if err := protoWriter.WriteMsg(&snapshottypes.SnapshotItem{
Item: &snapshottypes.SnapshotItem_Store{
Store: &snapshottypes.SnapshotStoreItem{
Name: tree.name,
Name: tree.Name,
},
},
}); err != nil {
return err
}

exporter := tree.tree.Export()
exporter := tree.Tree.Export()
for {
node, err := exporter.Next()
if err == iavl.ExportDone {
Expand Down
69 changes: 37 additions & 32 deletions memiavl/multitree.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import (

const MetadataFileName = "__metadata"

type namedTree struct {
tree *Tree
name string
type NamedTree struct {
*Tree
Name string
}

// MultiTree manages multiple memiavl tree together,
Expand All @@ -47,8 +47,8 @@ type MultiTree struct {
zeroCopy bool
cacheSize int

trees []namedTree
treesByName map[string]int // reversed index of the trees
trees []NamedTree // always ordered by tree name
treesByName map[string]int // index of the trees by name
lastCommitInfo storetypes.CommitInfo

// the initial metadata loaded from disk snapshot
Expand Down Expand Up @@ -92,11 +92,11 @@ func LoadMultiTree(dir string, zeroCopy bool, cacheSize int) (*MultiTree, error)

sort.Strings(treeNames)

trees := make([]namedTree, len(treeNames))
trees := make([]NamedTree, len(treeNames))
treesByName := make(map[string]int, len(trees))
for i, name := range treeNames {
tree := treeMap[name]
trees[i] = namedTree{tree: tree, name: name}
trees[i] = NamedTree{Tree: tree, Name: name}
treesByName[name] = i
}

Expand All @@ -116,11 +116,16 @@ func LoadMultiTree(dir string, zeroCopy bool, cacheSize int) (*MultiTree, error)
// TreeByName returns the tree by name, returns nil if not found
func (t *MultiTree) TreeByName(name string) *Tree {
if i, ok := t.treesByName[name]; ok {
return t.trees[i].tree
return t.trees[i].Tree
}
return nil
}

// Trees returns all the trees together with the name, ordered by name.
func (t *MultiTree) Trees() []NamedTree {
return t.trees
}

func (t *MultiTree) SetInitialVersion(initialVersion int64) error {
if initialVersion >= math.MaxUint32 {
return fmt.Errorf("version overflows uint32: %d", initialVersion)
Expand All @@ -131,8 +136,8 @@ func (t *MultiTree) SetInitialVersion(initialVersion int64) error {
}

for _, entry := range t.trees {
if !entry.tree.IsEmpty() {
return fmt.Errorf("tree is not empty: %s", entry.name)
if !entry.Tree.IsEmpty() {
return fmt.Errorf("tree is not empty: %s", entry.Name)
}
}

Expand All @@ -143,25 +148,25 @@ func (t *MultiTree) SetInitialVersion(initialVersion int64) error {
func (t *MultiTree) setInitialVersion(initialVersion int64) {
t.initialVersion = uint32(initialVersion)
for _, entry := range t.trees {
entry.tree.initialVersion = t.initialVersion
entry.Tree.initialVersion = t.initialVersion
}
}

func (t *MultiTree) SetZeroCopy(zeroCopy bool) {
t.zeroCopy = zeroCopy
for _, entry := range t.trees {
entry.tree.SetZeroCopy(zeroCopy)
entry.Tree.SetZeroCopy(zeroCopy)
}
}

// Copy returns a snapshot of the tree which won't be corrupted by further modifications on the main tree.
func (t *MultiTree) Copy(cacheSize int) *MultiTree {
trees := make([]namedTree, len(t.trees))
trees := make([]NamedTree, len(t.trees))
treesByName := make(map[string]int, len(t.trees))
for i, entry := range t.trees {
tree := entry.tree.Copy(cacheSize)
trees[i] = namedTree{tree: tree, name: entry.name}
treesByName[entry.name] = i
tree := entry.Tree.Copy(cacheSize)
trees[i] = NamedTree{Tree: tree, Name: entry.Name}
treesByName[entry.Name] = i
}

clone := *t
Expand Down Expand Up @@ -197,8 +202,8 @@ func (t *MultiTree) ApplyUpgrades(upgrades []*TreeNameUpgrade) error {
for _, upgrade := range upgrades {
switch {
case upgrade.Delete:
i := slices.IndexFunc(t.trees, func(entry namedTree) bool {
return entry.name == upgrade.Name
i := slices.IndexFunc(t.trees, func(entry NamedTree) bool {
return entry.Name == upgrade.Name
})
if i < 0 {
return fmt.Errorf("unknown tree name %s", upgrade.Name)
Expand All @@ -208,29 +213,29 @@ func (t *MultiTree) ApplyUpgrades(upgrades []*TreeNameUpgrade) error {
t.trees = t.trees[:len(t.trees)-1]
case upgrade.RenameFrom != "":
// rename tree
i := slices.IndexFunc(t.trees, func(entry namedTree) bool {
return entry.name == upgrade.RenameFrom
i := slices.IndexFunc(t.trees, func(entry NamedTree) bool {
return entry.Name == upgrade.RenameFrom
})
if i < 0 {
return fmt.Errorf("unknown tree name %s", upgrade.RenameFrom)
}
t.trees[i].name = upgrade.Name
t.trees[i].Name = upgrade.Name
default:
// add tree
tree := NewWithInitialVersion(uint32(nextVersion(t.Version(), t.initialVersion)), t.cacheSize)
t.trees = append(t.trees, namedTree{tree: tree, name: upgrade.Name})
t.trees = append(t.trees, NamedTree{Tree: tree, Name: upgrade.Name})
}
}

sort.SliceStable(t.trees, func(i, j int) bool {
return t.trees[i].name < t.trees[j].name
return t.trees[i].Name < t.trees[j].Name
})
t.treesByName = make(map[string]int, len(t.trees))
for i, tree := range t.trees {
if _, ok := t.treesByName[tree.name]; ok {
return fmt.Errorf("memiavl tree name conflicts: %s", tree.name)
if _, ok := t.treesByName[tree.Name]; ok {
return fmt.Errorf("memiavl tree name conflicts: %s", tree.Name)
}
t.treesByName[tree.name] = i
t.treesByName[tree.Name] = i
}

return nil
Expand All @@ -242,7 +247,7 @@ func (t *MultiTree) ApplyChangeSet(changeSets []*NamedChangeSet, updateCommitInf
version := nextVersion(t.lastCommitInfo.Version, t.initialVersion)

for _, cs := range changeSets {
tree := t.trees[t.treesByName[cs.Name]].tree
tree := t.trees[t.treesByName[cs.Name]].Tree

_, v, err := tree.ApplyChangeSet(cs.Changeset, updateCommitInfo)
if err != nil {
Expand Down Expand Up @@ -271,10 +276,10 @@ func (t *MultiTree) UpdateCommitInfo() []byte {
var infos []storetypes.StoreInfo
for _, entry := range t.trees {
infos = append(infos, storetypes.StoreInfo{
Name: entry.name,
Name: entry.Name,
CommitId: storetypes.CommitID{
Version: entry.tree.Version(),
Hash: entry.tree.RootHash(),
Version: entry.Tree.Version(),
Hash: entry.Tree.RootHash(),
},
})
}
Expand Down Expand Up @@ -337,7 +342,7 @@ func (t *MultiTree) WriteSnapshot(dir string) error {
// write the snapshots in parallel
g, _ := errgroup.WithContext(context.Background())
for _, entry := range t.trees {
tree, name := entry.tree, entry.name // https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables
tree, name := entry.Tree, entry.Name // https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables
g.Go(func() error {
return tree.WriteSnapshot(filepath.Join(dir, name))
})
Expand Down Expand Up @@ -377,7 +382,7 @@ func WriteFileSync(name string, data []byte) error {
func (t *MultiTree) Close() error {
errs := make([]error, 0, len(t.trees))
for _, entry := range t.trees {
errs = append(errs, entry.tree.Close())
errs = append(errs, entry.Tree.Close())
}
t.trees = nil
t.treesByName = nil
Expand Down
28 changes: 26 additions & 2 deletions store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ func (rs *Store) CacheWrapWithTrace(_ io.Writer, _ types.TraceContext) types.Cac

// Implements interface MultiStore
func (rs *Store) CacheMultiStore() types.CacheMultiStore {
// TODO custom cache store
stores := make(map[types.StoreKey]types.CacheWrapper)
for k, v := range rs.stores {
store := types.KVStore(v)
Expand All @@ -167,7 +166,32 @@ func (rs *Store) CacheMultiStore() types.CacheMultiStore {
// Implements interface MultiStore
// used to createQueryContext, abci_query or grpc query service.
func (rs *Store) CacheMultiStoreWithVersion(version int64) (types.CacheMultiStore, error) {
panic("rootmulti Store don't support historical query service")
if version == 0 || (rs.lastCommitInfo != nil && version == rs.lastCommitInfo.Version) {
mmsqe marked this conversation as resolved.
Show resolved Hide resolved
return rs.CacheMultiStore(), nil
}
opts := rs.opts
opts.TargetVersion = uint32(version)

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion

Potential integer overflow by integer type conversion
opts.ReadOnly = true
db, err := memiavl.Load(rs.dir, opts)
yihuang marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

stores := make(map[types.StoreKey]types.CacheWrapper)

// add the transient/mem stores registered in current app.
for k, store := range rs.stores {
if store.GetStoreType() != types.StoreTypeIAVL {
stores[k] = store
}
}
Comment on lines +183 to +187

Check failure

Code scanning / gosec

the value in the range statement should be _ unless copying a map: want: for key := range m

the value in the range statement should be _ unless copying a map: want: for key := range m
Comment on lines +183 to +187

Check warning

Code scanning / CodeQL

Iteration over map

Iteration over map may be a possible source of non-determinism

// add all the iavl stores at the target version.
for _, tree := range db.Trees() {
stores[rs.keysByName[tree.Name]] = memiavlstore.New(tree.Tree, rs.logger)
}

return cachemulti.NewStore(nil, stores, rs.keysByName, nil, nil), nil
}

// Implements interface MultiStore
Expand Down