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

feat: refactor the export traversal order as pre-order #662

Closed
wants to merge 10 commits into from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- [#636](https://github.com/cosmos/iavl/pull/636) Speed up rollback method: `LoadVersionForOverwriting`.
- [#654](https://github.com/cosmos/iavl/pull/654) Add API `TraverseStateChanges` to extract state changes from iavl versions.
- [#638](https://github.com/cosmos/iavl/pull/638) Make LazyLoadVersion check the opts.InitialVersion, add API `LazyLoadVersionForOverwriting`.
- [#662](https://github.com/cosmos/iavl/pull/662) Refactor `Export` and `Import` to provide both `post-order` and `pre-order`.

## 0.19.4 (October 28, 2022)

Expand Down
2 changes: 1 addition & 1 deletion benchmarks/cosmos-exim/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func runImport(version int64, exports map[string][]*iavl.ExportNode) error {
if err != nil {
return err
}
importer, err := newTree.Import(version)
importer, err := newTree.ImportPreOrder(version)
if err != nil {
return err
}
Expand Down
81 changes: 55 additions & 26 deletions export.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@ import (
"fmt"
)

type OrderType int

// OrderTraverse is the type of traversal order to use when exporting and importing.
cool-develope marked this conversation as resolved.
Show resolved Hide resolved
// PreOrder is needed for the new node-key refactoring. The default is PostOrder.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought new node-key refactoring works with both orderings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, the pre-order is needed for node-key refactoring

Copy link
Member

Choose a reason for hiding this comment

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

slightly confused now. This pr adds support for both pre and post order, but node key refactor require pre-order. If a node exports using post-order then we cant import it into the new version correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, that's why provides both orders

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

even it is the original version, we can request a pre-order snapshot, then import it into the new version

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the reason post-order is chosen in the first place is the node hash is updated in a post-order way, you need to update the children first to update the parent node, will pre-order import need more temporary memory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, literally post-order more makes sense, both way requires stack to keep the current path, you are right pre-order will require 2 times memory, but the stack length is at most the height of the tree, it is so trivial

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tac0turtle , the old version can use any order, the new version should use pre-order.

Copy link
Collaborator

Choose a reason for hiding this comment

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

but the stack length is at most the height of the tree, it is so trivial

yeah, that should be trivial then, if this is indeed necessary, I think the chains can do a coordinated upgrade in advance to switch to pre-order snapshots, before doing node key format migration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, that should be trivial then, if this is indeed necessary, I think the chains can do a coordinated upgrade in advance to switch to pre-order snapshots, before doing node key format migration.

I think this is what we should discuss, tbh I have no clear idea which way is more efficient

const (
PreOrderTraverse OrderType = iota
PostOrderTraverse
)

// exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by
// processing multiple nodes per context switch, but take care to avoid excessive memory usage,
// especially since callers may export several IAVL stores in parallel (e.g. the Cosmos SDK).
Expand All @@ -27,17 +36,17 @@ type ExportNode struct {

// Exporter exports nodes from an ImmutableTree. It is created by ImmutableTree.Export().
//
// Exported nodes can be imported into an empty tree with MutableTree.Import(). Nodes are exported
// depth-first post-order (LRN), this order must be preserved when importing in order to recreate
// the same tree structure.
// Exported nodes can be imported into an empty tree with MutableTree.Import(). Nodes are exported in traverseOrder,
// this order must be preserved when importing in order to recreate the same tree structure.
type Exporter struct {
tree *ImmutableTree
ch chan *ExportNode
cancel context.CancelFunc
tree *ImmutableTree
traverseOrder OrderType
ch chan *ExportNode
cancel context.CancelFunc
}

// NewExporter creates a new Exporter. Callers must call Close() when done.
func newExporter(tree *ImmutableTree) (*Exporter, error) {
func newExporter(tree *ImmutableTree, traverseOrder OrderType) (*Exporter, error) {
if tree == nil {
return nil, fmt.Errorf("tree is nil: %w", ErrNotInitalizedTree)
}
Expand All @@ -48,9 +57,10 @@ func newExporter(tree *ImmutableTree) (*Exporter, error) {

ctx, cancel := context.WithCancel(context.Background())
exporter := &Exporter{
tree: tree,
ch: make(chan *ExportNode, exportBufferSize),
cancel: cancel,
tree: tree,
traverseOrder: traverseOrder,
ch: make(chan *ExportNode, exportBufferSize),
cancel: cancel,
}

tree.ndb.incrVersionReaders(tree.version)
Expand All @@ -61,22 +71,41 @@ func newExporter(tree *ImmutableTree) (*Exporter, error) {

// export exports nodes
func (e *Exporter) export(ctx context.Context) {
e.tree.root.traversePost(e.tree, true, func(node *Node) bool {
exportNode := &ExportNode{
Key: node.key,
Value: node.value,
Version: node.version,
Height: node.subtreeHeight,
}

select {
case e.ch <- exportNode:
return false
case <-ctx.Done():
return true
}
})
close(e.ch)
defer close(e.ch)
switch e.traverseOrder {
case PreOrderTraverse:
e.tree.root.traverse(e.tree, true, func(node *Node) bool {
exportNode := &ExportNode{
Key: node.key,
Value: node.value,
Version: node.version,
Height: node.subtreeHeight,
}

select {
case e.ch <- exportNode:
return false
case <-ctx.Done():
return true
}
})
case PostOrderTraverse:
e.tree.root.traversePost(e.tree, true, func(node *Node) bool {
exportNode := &ExportNode{
Key: node.key,
Value: node.value,
Version: node.version,
Height: node.subtreeHeight,
}

select {
case e.ch <- exportNode:
return false
case <-ctx.Done():
return true
}
})
}
}

// Next fetches the next exported node, or returns ExportDone when done.
Expand Down
199 changes: 126 additions & 73 deletions export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,32 +160,54 @@ func setupExportTreeSized(t require.TestingT, treeSize int) *ImmutableTree { //n
func TestExporter(t *testing.T) {
tree := setupExportTreeBasic(t)

expect := []*ExportNode{
{Key: []byte("a"), Value: []byte{1}, Version: 1, Height: 0},
{Key: []byte("b"), Value: []byte{2}, Version: 3, Height: 0},
{Key: []byte("b"), Value: nil, Version: 3, Height: 1},
{Key: []byte("c"), Value: []byte{3}, Version: 3, Height: 0},
{Key: []byte("c"), Value: nil, Version: 3, Height: 2},
{Key: []byte("d"), Value: []byte{4}, Version: 2, Height: 0},
{Key: []byte("e"), Value: []byte{5}, Version: 3, Height: 0},
{Key: []byte("e"), Value: nil, Version: 3, Height: 1},
{Key: []byte("d"), Value: nil, Version: 3, Height: 3},
expects := map[OrderType][]*ExportNode{
PreOrderTraverse: {
{Key: []byte("d"), Value: nil, Version: 3, Height: 3},
{Key: []byte("c"), Value: nil, Version: 3, Height: 2},
{Key: []byte("b"), Value: nil, Version: 3, Height: 1},
{Key: []byte("a"), Value: []byte{1}, Version: 1, Height: 0},
{Key: []byte("b"), Value: []byte{2}, Version: 3, Height: 0},
{Key: []byte("c"), Value: []byte{3}, Version: 3, Height: 0},
{Key: []byte("e"), Value: nil, Version: 3, Height: 1},
{Key: []byte("d"), Value: []byte{4}, Version: 2, Height: 0},
{Key: []byte("e"), Value: []byte{5}, Version: 3, Height: 0},
},
PostOrderTraverse: {
{Key: []byte("a"), Value: []byte{1}, Version: 1, Height: 0},
{Key: []byte("b"), Value: []byte{2}, Version: 3, Height: 0},
{Key: []byte("b"), Value: nil, Version: 3, Height: 1},
{Key: []byte("c"), Value: []byte{3}, Version: 3, Height: 0},
{Key: []byte("c"), Value: nil, Version: 3, Height: 2},
{Key: []byte("d"), Value: []byte{4}, Version: 2, Height: 0},
{Key: []byte("e"), Value: []byte{5}, Version: 3, Height: 0},
{Key: []byte("e"), Value: nil, Version: 3, Height: 1},
{Key: []byte("d"), Value: nil, Version: 3, Height: 3},
},
}

actual := make([]*ExportNode, 0, len(expect))
exporter, err := tree.Export()
require.NoError(t, err)
defer exporter.Close()
for {
node, err := exporter.Next()
if err == ErrorExportDone {
break
for orderType, expect := range expects {
actual := make([]*ExportNode, 0, len(expect))
var (
exporter *Exporter
err error
)
if orderType == PreOrderTraverse {
exporter, err = tree.ExportPreOrder()
} else {
exporter, err = tree.Export()
}
require.NoError(t, err)
actual = append(actual, node)
}
defer exporter.Close()
for {
node, err := exporter.Next()
if err == ErrorExportDone {
break
}
require.NoError(t, err)
actual = append(actual, node)
}

assert.Equal(t, expect, actual)
assert.Equal(t, expect, actual)
}
}

func TestExporter_Import(t *testing.T) {
Expand All @@ -197,59 +219,73 @@ func TestExporter_Import(t *testing.T) {
testcases["sized tree"] = setupExportTreeSized(t, 4096)
testcases["random tree"] = setupExportTreeRandom(t)
}
for _, orderType := range []OrderType{PostOrderTraverse, PreOrderTraverse} {
for desc, tree := range testcases {
tree := tree
t.Run(desc, func(t *testing.T) {
t.Parallel()

var (
exporter *Exporter
importer *Importer
err error
)
if orderType == PreOrderTraverse {
exporter, err = tree.ExportPreOrder()
} else {
exporter, err = tree.Export()
}
require.NoError(t, err)
defer exporter.Close()

for desc, tree := range testcases {
tree := tree
t.Run(desc, func(t *testing.T) {
t.Parallel()

exporter, err := tree.Export()
require.NoError(t, err)
defer exporter.Close()

newTree, err := NewMutableTree(db.NewMemDB(), 0, false)
require.NoError(t, err)
importer, err := newTree.Import(tree.Version())
require.NoError(t, err)
defer importer.Close()

for {
item, err := exporter.Next()
if err == ErrorExportDone {
err = importer.Commit()
newTree, err := NewMutableTree(db.NewMemDB(), 0, false)
require.NoError(t, err)
if orderType == PreOrderTraverse {
importer, err = newTree.ImportPreOrder(tree.Version())
} else {
importer, err = newTree.Import(tree.Version())
}
require.NoError(t, err)
defer importer.Close()

for {
item, err := exporter.Next()
if err == ErrorExportDone {
err = importer.Commit()
require.NoError(t, err)
break
}
require.NoError(t, err)
err = importer.Add(item)
require.NoError(t, err)
break
}

treeHash, err := tree.Hash()
require.NoError(t, err)
err = importer.Add(item)
newTreeHash, err := newTree.Hash()
require.NoError(t, err)
}

treeHash, err := tree.Hash()
require.NoError(t, err)
newTreeHash, err := newTree.Hash()
require.NoError(t, err)

require.Equal(t, treeHash, newTreeHash, "Tree hash mismatch")
require.Equal(t, tree.Size(), newTree.Size(), "Tree size mismatch")
require.Equal(t, tree.Version(), newTree.Version(), "Tree version mismatch")
require.Equal(t, treeHash, newTreeHash, "Tree hash mismatch")
require.Equal(t, tree.Size(), newTree.Size(), "Tree size mismatch")
require.Equal(t, tree.Version(), newTree.Version(), "Tree version mismatch")

tree.Iterate(func(key, value []byte) bool { //nolint:errcheck
index, _, err := tree.GetWithIndex(key)
require.NoError(t, err)
newIndex, newValue, err := newTree.GetWithIndex(key)
require.NoError(t, err)
require.Equal(t, index, newIndex, "Index mismatch for key %v", key)
require.Equal(t, value, newValue, "Value mismatch for key %v", key)
return false
tree.Iterate(func(key, value []byte) bool { //nolint:errcheck
index, _, err := tree.GetWithIndex(key)
require.NoError(t, err)
newIndex, newValue, err := newTree.GetWithIndex(key)
require.NoError(t, err)
require.Equal(t, index, newIndex, "Index mismatch for key %v", key)
require.Equal(t, value, newValue, "Value mismatch for key %v", key)
return false
})
})
})
}
}
}

func TestExporter_Close(t *testing.T) {
tree := setupExportTreeSized(t, 4096)
exporter, err := tree.Export()
exporter, err := tree.ExportPreOrder()
require.NoError(t, err)

node, err := exporter.Next()
Expand Down Expand Up @@ -311,17 +347,34 @@ func BenchmarkExport(b *testing.B) {
b.StopTimer()
tree := setupExportTreeSized(b, 4096)
b.StartTimer()
for n := 0; n < b.N; n++ {
exporter, err := tree.Export()
require.NoError(b, err)
for {
_, err := exporter.Next()
if err == ErrorExportDone {
break
} else if err != nil {
b.Error(err)
b.Run("post order export", func(sub *testing.B) {
for n := 0; n < sub.N; n++ {
exporter, err := tree.Export()
require.NoError(sub, err)
for {
_, err := exporter.Next()
if err == ErrorExportDone {
break
} else if err != nil {
sub.Error(err)
}
}
exporter.Close()
}
exporter.Close()
}
})
b.Run("pre order export", func(sub *testing.B) {
for n := 0; n < sub.N; n++ {
exporter, err := tree.ExportPreOrder()
require.NoError(sub, err)
for {
_, err := exporter.Next()
if err == ErrorExportDone {
break
} else if err != nil {
sub.Error(err)
}
}
exporter.Close()
}
})
}
10 changes: 9 additions & 1 deletion immutable_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,16 @@ func (t *ImmutableTree) Hash() ([]byte, error) {

// Export returns an iterator that exports tree nodes as ExportNodes. These nodes can be
// imported with MutableTree.Import() to recreate an identical tree.
// The default export order is PostOrderTraverse.
func (t *ImmutableTree) Export() (*Exporter, error) {
return newExporter(t)
return newExporter(t, PostOrderTraverse)
}

// Export returns an iterator that exports tree nodes as ExportNodes with pre-order traversal.
// This api is for new node-key refactoring since we need to build a new tree with pre-order traversal
// to construct the new node-key (path)
func (t *ImmutableTree) ExportPreOrder() (*Exporter, error) {
return newExporter(t, PreOrderTraverse)
}

// GetWithIndex returns the index and value of the specified key if it exists, or nil and the next index
Expand Down
Loading