From 6444ca0fb8dbc16b15f93f0934086d65e92bf4ff Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 17:18:12 -0600 Subject: [PATCH 01/26] make debug logs --- mutable_tree.go | 166 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 119 insertions(+), 47 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index e65dbd109..f47e816bd 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -161,6 +161,7 @@ func (tree *MutableTree) String() (string, error) { // to slices stored within IAVL. It returns true when an existing value was // updated, while false means it was a new key. func (tree *MutableTree) Set(key, value []byte) (updated bool, err error) { + fmt.Println("Set", key) updated, err = tree.set(key, value) if err != nil { return false, err @@ -269,62 +270,130 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( version := tree.version + 1 if node.isLeaf() { - if !tree.skipFastStorageUpgrade { - tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) - } - switch bytes.Compare(key, node.key) { - case -1: // setKey < leafKey - return &Node{ - key: node.key, - subtreeHeight: 1, - size: 2, - nodeKey: nil, - leftNode: NewNode(key, value), - rightNode: node, - }, false, nil - case 1: // setKey > leafKey - return &Node{ - key: key, - subtreeHeight: 1, - size: 2, - nodeKey: nil, - leftNode: node, - rightNode: NewNode(key, value), - }, false, nil - default: - return NewNode(key, value), true, nil + return tree.recursiveSetLeaf(node, key, value) + } + fmt.Println("Recursive Set", node.key) + node, err = node.clone(tree) + if err != nil { + return nil, false, err + } + + if bytes.Compare(key, node.key) < 0 { + if len(node.leftNodeKey) == 32 { + node.leftNode, updated, err = tree.recursiveSetLegacy(node.leftNode, key, value) + } else { + node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) } - } else { - node, err = node.clone(tree) if err != nil { - return nil, false, err + return nil, updated, err } - - if bytes.Compare(key, node.key) < 0 { - node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) - if err != nil { - return nil, updated, err - } + } else { + if len(node.rightNodeKey) == 32 { + node.rightNode, updated, err = tree.recursiveSetLegacy(node.rightNode, key, value) } else { node.rightNode, updated, err = tree.recursiveSet(node.rightNode, key, value) - if err != nil { - return nil, updated, err - } - } - - if updated { - return node, updated, nil } - err = node.calcHeightAndSize(tree.ImmutableTree) if err != nil { - return nil, false, err + return nil, updated, err } - newNode, err := tree.balance(node) - if err != nil { - return nil, false, err - } - return newNode, updated, err } + + if updated { + return node, updated, nil + } + err = node.calcHeightAndSize(tree.ImmutableTree) + if err != nil { + return nil, false, err + } + newNode, err := tree.balance(node) + if err != nil { + return nil, false, err + } + return newNode, updated, err +} + +func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) ( + newSelf *Node, updated bool, err error, +) { + fmt.Println("Recursive Set Leaf", node.key) + version := tree.version + 1 + if !tree.skipFastStorageUpgrade { + tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) + } + switch bytes.Compare(key, node.key) { + case -1: // setKey < leafKey + return &Node{ + key: node.key, + subtreeHeight: 1, + size: 2, + nodeKey: nil, + leftNode: NewNode(key, value), + rightNode: node, + }, false, nil + case 1: // setKey > leafKey + return &Node{ + key: key, + subtreeHeight: 1, + size: 2, + nodeKey: nil, + leftNode: node, + rightNode: NewNode(key, value), + }, false, nil + default: + return NewNode(key, value), true, nil + } +} + +// recursiveSetLegacy is the same as recursiveSet but takes an optimization where +// if you are updating an existing leaf, you do not need to get both children, you only need one child. +// +// This operates on the assumption that when recursiveSet enters a legacy node, +// all of the legacy nodes children will be legacy nodes. +func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte) ( + newSelf *Node, updated bool, err error, +) { + if node.isLeaf() { + return tree.recursiveSetLeaf(node, key, value) + } + fmt.Println("Recursive Set Legacy", string(node.key)) + node, err = node.cloneNoChildFetch(tree) + if err != nil { + return nil, false, err + } + + recurseLeft := false + if bytes.Compare(key, node.key) < 0 { + recurseLeft = true + } + child, err := node.fetchOneChild(tree, recurseLeft) + if err != nil { + return nil, false, err + } + + newChild, updated, err := tree.recursiveSetLegacy(child, key, value) + if err != nil { + return nil, updated, err + } + if recurseLeft { + node.leftNode = newChild + } else { + node.rightNode = newChild + } + + if updated { + return node, updated, nil + } + node.fetchOneChild(tree, !recurseLeft) + + err = node.calcHeightAndSize(tree.ImmutableTree) + if err != nil { + return nil, false, err + } + newNode, err := tree.balance(node) + if err != nil { + return nil, false, err + } + return newNode, updated, err } // Remove removes a key from the working tree. The given key byte slice should not be modified @@ -333,6 +402,7 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { if tree.root == nil { return nil, false, nil } + fmt.Println("Remove", string(key)) newRoot, _, value, removed, err := tree.recursiveRemove(tree.root, key) if err != nil { return nil, false, err @@ -357,7 +427,9 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { // - the removed value func (tree *MutableTree) recursiveRemove(node *Node, key []byte) (newSelf *Node, newKey []byte, newValue []byte, removed bool, err error) { tree.logger.Debug("recursiveRemove", "node", node, "key", key) + fmt.Println("Recursive remove", string(node.key)) if node.isLeaf() { + if bytes.Equal(key, node.key) { return nil, nil, node.value, true, nil } From d868459b6166d20c00f12a93d836b34e52e3467d Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:27:42 -0700 Subject: [PATCH 02/26] Revert "make debug logs" This reverts commit 6444ca0fb8dbc16b15f93f0934086d65e92bf4ff. --- mutable_tree.go | 166 ++++++++++++++---------------------------------- 1 file changed, 47 insertions(+), 119 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index f47e816bd..e65dbd109 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -161,7 +161,6 @@ func (tree *MutableTree) String() (string, error) { // to slices stored within IAVL. It returns true when an existing value was // updated, while false means it was a new key. func (tree *MutableTree) Set(key, value []byte) (updated bool, err error) { - fmt.Println("Set", key) updated, err = tree.set(key, value) if err != nil { return false, err @@ -270,130 +269,62 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( version := tree.version + 1 if node.isLeaf() { - return tree.recursiveSetLeaf(node, key, value) - } - fmt.Println("Recursive Set", node.key) - node, err = node.clone(tree) - if err != nil { - return nil, false, err - } - - if bytes.Compare(key, node.key) < 0 { - if len(node.leftNodeKey) == 32 { - node.leftNode, updated, err = tree.recursiveSetLegacy(node.leftNode, key, value) - } else { - node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) + if !tree.skipFastStorageUpgrade { + tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) } - if err != nil { - return nil, updated, err + switch bytes.Compare(key, node.key) { + case -1: // setKey < leafKey + return &Node{ + key: node.key, + subtreeHeight: 1, + size: 2, + nodeKey: nil, + leftNode: NewNode(key, value), + rightNode: node, + }, false, nil + case 1: // setKey > leafKey + return &Node{ + key: key, + subtreeHeight: 1, + size: 2, + nodeKey: nil, + leftNode: node, + rightNode: NewNode(key, value), + }, false, nil + default: + return NewNode(key, value), true, nil } } else { - if len(node.rightNodeKey) == 32 { - node.rightNode, updated, err = tree.recursiveSetLegacy(node.rightNode, key, value) + node, err = node.clone(tree) + if err != nil { + return nil, false, err + } + + if bytes.Compare(key, node.key) < 0 { + node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) + if err != nil { + return nil, updated, err + } } else { node.rightNode, updated, err = tree.recursiveSet(node.rightNode, key, value) + if err != nil { + return nil, updated, err + } + } + + if updated { + return node, updated, nil } + err = node.calcHeightAndSize(tree.ImmutableTree) if err != nil { - return nil, updated, err + return nil, false, err } + newNode, err := tree.balance(node) + if err != nil { + return nil, false, err + } + return newNode, updated, err } - - if updated { - return node, updated, nil - } - err = node.calcHeightAndSize(tree.ImmutableTree) - if err != nil { - return nil, false, err - } - newNode, err := tree.balance(node) - if err != nil { - return nil, false, err - } - return newNode, updated, err -} - -func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) ( - newSelf *Node, updated bool, err error, -) { - fmt.Println("Recursive Set Leaf", node.key) - version := tree.version + 1 - if !tree.skipFastStorageUpgrade { - tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) - } - switch bytes.Compare(key, node.key) { - case -1: // setKey < leafKey - return &Node{ - key: node.key, - subtreeHeight: 1, - size: 2, - nodeKey: nil, - leftNode: NewNode(key, value), - rightNode: node, - }, false, nil - case 1: // setKey > leafKey - return &Node{ - key: key, - subtreeHeight: 1, - size: 2, - nodeKey: nil, - leftNode: node, - rightNode: NewNode(key, value), - }, false, nil - default: - return NewNode(key, value), true, nil - } -} - -// recursiveSetLegacy is the same as recursiveSet but takes an optimization where -// if you are updating an existing leaf, you do not need to get both children, you only need one child. -// -// This operates on the assumption that when recursiveSet enters a legacy node, -// all of the legacy nodes children will be legacy nodes. -func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte) ( - newSelf *Node, updated bool, err error, -) { - if node.isLeaf() { - return tree.recursiveSetLeaf(node, key, value) - } - fmt.Println("Recursive Set Legacy", string(node.key)) - node, err = node.cloneNoChildFetch(tree) - if err != nil { - return nil, false, err - } - - recurseLeft := false - if bytes.Compare(key, node.key) < 0 { - recurseLeft = true - } - child, err := node.fetchOneChild(tree, recurseLeft) - if err != nil { - return nil, false, err - } - - newChild, updated, err := tree.recursiveSetLegacy(child, key, value) - if err != nil { - return nil, updated, err - } - if recurseLeft { - node.leftNode = newChild - } else { - node.rightNode = newChild - } - - if updated { - return node, updated, nil - } - node.fetchOneChild(tree, !recurseLeft) - - err = node.calcHeightAndSize(tree.ImmutableTree) - if err != nil { - return nil, false, err - } - newNode, err := tree.balance(node) - if err != nil { - return nil, false, err - } - return newNode, updated, err } // Remove removes a key from the working tree. The given key byte slice should not be modified @@ -402,7 +333,6 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { if tree.root == nil { return nil, false, nil } - fmt.Println("Remove", string(key)) newRoot, _, value, removed, err := tree.recursiveRemove(tree.root, key) if err != nil { return nil, false, err @@ -427,9 +357,7 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { // - the removed value func (tree *MutableTree) recursiveRemove(node *Node, key []byte) (newSelf *Node, newKey []byte, newValue []byte, removed bool, err error) { tree.logger.Debug("recursiveRemove", "node", node, "key", key) - fmt.Println("Recursive remove", string(node.key)) if node.isLeaf() { - if bytes.Equal(key, node.key) { return nil, nil, node.value, true, nil } From 6aee1ef9bd746b2fff0c58b679d24c564d2e3581 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Fri, 22 Dec 2023 16:07:50 -0700 Subject: [PATCH 03/26] orphan change --- batch.go | 9 +++++++++ nodedb.go | 37 ++++++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/batch.go b/batch.go index b25e879a2..a7e5e20b0 100644 --- a/batch.go +++ b/batch.go @@ -1,6 +1,8 @@ package iavl import ( + "sync" + dbm "github.com/cosmos/cosmos-db" ) @@ -11,6 +13,7 @@ type BatchWithFlusher struct { db dbm.DB // This is only used to create new batch batch dbm.Batch // Batched writing buffer. + mtx sync.Mutex flushThreshold int // The threshold to flush the batch to disk. } @@ -46,6 +49,9 @@ func (b *BatchWithFlusher) estimateSizeAfterSetting(key []byte, value []byte) (i // the batch is flushed to disk, cleared, and a new one is created with buffer pre-allocated to threshold. // The addition entry is then added to the batch. func (b *BatchWithFlusher) Set(key, value []byte) error { + b.mtx.Lock() + defer b.mtx.Unlock() + batchSizeAfter, err := b.estimateSizeAfterSetting(key, value) if err != nil { return err @@ -67,6 +73,9 @@ func (b *BatchWithFlusher) Set(key, value []byte) error { // the batch is flushed to disk, cleared, and a new one is created with buffer pre-allocated to threshold. // The deletion entry is then added to the batch. func (b *BatchWithFlusher) Delete(key []byte) error { + b.mtx.Lock() + defer b.mtx.Unlock() + batchSizeAfter, err := b.estimateSizeAfterSetting(key, []byte{}) if err != nil { return err diff --git a/nodedb.go b/nodedb.go index d446b7c1d..d7852574f 100644 --- a/nodedb.go +++ b/nodedb.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" "sync" + "time" "cosmossdk.io/log" dbm "github.com/cosmos/cosmos-db" @@ -470,9 +471,39 @@ func (ndb *nodeDB) deleteLegacyVersions() error { ndb.legacyLatestVersion = -1 // Delete all orphan nodes of the legacy versions - return ndb.traversePrefix(legacyOrphanKeyFormat.Key(), func(key, value []byte) error { - return ndb.batch.Delete(key) - }) + go func() { + if err := ndb.deleteOrphans(); err != nil { + ndb.logger.Error("failed to clean legacy orphans", "err", err) + } + }() + + return nil +} + +// deleteOrphans cleans all legacy orphans from the nodeDB. +func (ndb *nodeDB) deleteOrphans() error { + itr, err := dbm.IteratePrefix(ndb.db, legacyOrphanKeyFormat.Key()) + if err != nil { + return err + } + defer itr.Close() + + count := 0 + for ; itr.Valid(); itr.Next() { + if err := ndb.batch.Delete(itr.Key()); err != nil { + return err + } + + // Sleep for a while to avoid blocking the main thread i/o. + count++ + if count > 1000 { + count = 0 + time.Sleep(100 * time.Millisecond) + } + + } + + return nil } // DeleteVersionsFrom permanently deletes all tree versions from the given version upwards. From cb9b2415392cac65861be6aae6a09ecf862cf56e Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 22 Dec 2023 20:59:42 -0600 Subject: [PATCH 04/26] Dev orphan code --- nodedb.go | 99 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 57 insertions(+), 42 deletions(-) diff --git a/nodedb.go b/nodedb.go index d7852574f..386f2162d 100644 --- a/nodedb.go +++ b/nodedb.go @@ -422,58 +422,73 @@ func (ndb *nodeDB) deleteLegacyNodes(version int64, nk []byte) error { // deleteLegacyVersions deletes all legacy versions from disk. func (ndb *nodeDB) deleteLegacyVersions() error { - // Check if we have a legacy version - itr, err := dbm.IteratePrefix(ndb.db, legacyRootKeyFormat.Key()) - if err != nil { - return err - } - defer itr.Close() - - // Delete orphans for all legacy versions - var prevVersion, curVersion int64 - var rootKeys [][]byte - for ; itr.Valid(); itr.Next() { - legacyRootKeyFormat.Scan(itr.Key(), &curVersion) - rootKeys = append(rootKeys, itr.Key()) - if prevVersion > 0 { - if err := ndb.traverseOrphans(prevVersion, curVersion, func(orphan *Node) error { - return ndb.batch.Delete(ndb.nodeKey(orphan.GetKey())) - }); err != nil { - return err - } - } - prevVersion = curVersion - } - // Delete the last version for the legacyLastVersion - if curVersion > 0 { - legacyLatestVersion, err := ndb.getLegacyLatestVersion() + go func() { + // Check if we have a legacy version + itr, err := dbm.IteratePrefix(ndb.db, legacyRootKeyFormat.Key()) if err != nil { - return err + ndb.logger.Error(err.Error()) + return } - if curVersion != legacyLatestVersion { - return fmt.Errorf("expected legacyLatestVersion to be %d, got %d", legacyLatestVersion, curVersion) + defer itr.Close() + + // Delete orphans for all legacy versions + var prevVersion, curVersion int64 + var rootKeys [][]byte + counter := 0 + for ; itr.Valid(); itr.Next() { + legacyRootKeyFormat.Scan(itr.Key(), &curVersion) + rootKeys = append(rootKeys, itr.Key()) + if prevVersion > 0 { + if err := ndb.traverseOrphans(prevVersion, curVersion, func(orphan *Node) error { + counter += 1 + if counter == 1000 { + counter = 0 + time.Sleep(100 * time.Millisecond) + fmt.Println("IAVL sleep happening") + } + return ndb.batch.Delete(ndb.nodeKey(orphan.GetKey())) + }); err != nil { + ndb.logger.Error(err.Error()) + return + } + } + prevVersion = curVersion } - if err := ndb.traverseOrphans(curVersion, curVersion+1, func(orphan *Node) error { - return ndb.batch.Delete(ndb.nodeKey(orphan.GetKey())) - }); err != nil { - return err + // Delete the last version for the legacyLastVersion + if curVersion > 0 { + legacyLatestVersion, err := ndb.getLegacyLatestVersion() + if err != nil { + ndb.logger.Error(err.Error()) + return + } + if curVersion != legacyLatestVersion { + ndb.logger.Error("expected legacyLatestVersion to be %d, got %d", legacyLatestVersion, curVersion) + return + } + if err := ndb.traverseOrphans(curVersion, curVersion+1, func(orphan *Node) error { + return ndb.batch.Delete(ndb.nodeKey(orphan.GetKey())) + }); err != nil { + ndb.logger.Error("failed to clean legacy orphans between versions", "err", err) + return + } } - } - // Delete all roots of the legacy versions - for _, rootKey := range rootKeys { - if err := ndb.batch.Delete(rootKey); err != nil { - return err + // Delete all roots of the legacy versions + for _, rootKey := range rootKeys { + if err := ndb.batch.Delete(rootKey); err != nil { + ndb.logger.Error("failed to clean legacy orphans root keys", "err", err) + return + } } - } - // Initialize the legacy latest version to -1 to demonstrate that all legacy versions have been deleted - ndb.legacyLatestVersion = -1 + // Initialize the legacy latest version to -1 to demonstrate that all legacy versions have been deleted + ndb.legacyLatestVersion = -1 - // Delete all orphan nodes of the legacy versions - go func() { + // Delete all orphan nodes of the legacy versions + // TODO: Is this just deadcode????? if err := ndb.deleteOrphans(); err != nil { ndb.logger.Error("failed to clean legacy orphans", "err", err) + return } }() From 80584b998913f54ee80cf1442815141fa38569e1 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 22 Dec 2023 21:16:10 -0600 Subject: [PATCH 05/26] tune params --- nodedb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nodedb.go b/nodedb.go index 386f2162d..541b1e378 100644 --- a/nodedb.go +++ b/nodedb.go @@ -443,7 +443,7 @@ func (ndb *nodeDB) deleteLegacyVersions() error { counter += 1 if counter == 1000 { counter = 0 - time.Sleep(100 * time.Millisecond) + time.Sleep(1000 * time.Millisecond) fmt.Println("IAVL sleep happening") } return ndb.batch.Delete(ndb.nodeKey(orphan.GetKey())) From a62ea1d33d2f9018f5372d778592b647aa2734cf Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 22 Dec 2023 21:35:29 -0600 Subject: [PATCH 06/26] Fix stacking --- nodedb.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/nodedb.go b/nodedb.go index 541b1e378..843725b7a 100644 --- a/nodedb.go +++ b/nodedb.go @@ -420,9 +420,27 @@ func (ndb *nodeDB) deleteLegacyNodes(version int64, nk []byte) error { return ndb.batch.Delete(ndb.legacyNodeKey(nk)) } +var isDeletingLegacyVersionsMutex *sync.Mutex +var isDeletingLegacyVersions bool = false + // deleteLegacyVersions deletes all legacy versions from disk. func (ndb *nodeDB) deleteLegacyVersions() error { + isDeletingLegacyVersionsMutex.Lock() + if isDeletingLegacyVersions { + isDeletingLegacyVersionsMutex.Unlock() + return nil + } else { + isDeletingLegacyVersions = true + isDeletingLegacyVersionsMutex.Unlock() + } + go func() { + defer func() { + isDeletingLegacyVersionsMutex.Lock() + isDeletingLegacyVersions = false + isDeletingLegacyVersionsMutex.Unlock() + }() + // Check if we have a legacy version itr, err := dbm.IteratePrefix(ndb.db, legacyRootKeyFormat.Key()) if err != nil { From 154ef57c1f9ee4b672c85e3508fa624ca2f893d1 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 22 Dec 2023 21:55:38 -0600 Subject: [PATCH 07/26] Fix --- nodedb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nodedb.go b/nodedb.go index 843725b7a..5194a85e0 100644 --- a/nodedb.go +++ b/nodedb.go @@ -420,7 +420,7 @@ func (ndb *nodeDB) deleteLegacyNodes(version int64, nk []byte) error { return ndb.batch.Delete(ndb.legacyNodeKey(nk)) } -var isDeletingLegacyVersionsMutex *sync.Mutex +var isDeletingLegacyVersionsMutex *sync.Mutex = &sync.Mutex{} var isDeletingLegacyVersions bool = false // deleteLegacyVersions deletes all legacy versions from disk. From f5cce22f2cbfc05a91226206305effc634af1fca Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 12:35:24 -0600 Subject: [PATCH 08/26] Use 32Byte encoding + make legacy encoding improvements --- internal/encoding/encoding.go | 17 ++++++++++ mutable_tree.go | 59 +++++++++++++++++++---------------- node.go | 32 +++++++++++++------ 3 files changed, 72 insertions(+), 36 deletions(-) diff --git a/internal/encoding/encoding.go b/internal/encoding/encoding.go index 17a994bc1..e198a7937 100644 --- a/internal/encoding/encoding.go +++ b/internal/encoding/encoding.go @@ -97,6 +97,23 @@ func EncodeBytes(w io.Writer, bz []byte) error { return err } +var hashLenBz []byte + +func init() { + hashLenBz = make([]byte, 1) + binary.PutUvarint(hashLenBz, 32) +} + +// Encode 32 byte long hash +func Encode32BytesHash(w io.Writer, bz []byte) error { + _, err := w.Write(hashLenBz) + if err != nil { + return err + } + _, err = w.Write(bz) + return err +} + // encodeBytesSlice length-prefixes the byte slice and returns it. func EncodeBytesSlice(bz []byte) ([]byte, error) { buf := bufPool.Get().(*bytes.Buffer) diff --git a/mutable_tree.go b/mutable_tree.go index e65dbd109..f342c04e0 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -266,34 +266,8 @@ func (tree *MutableTree) set(key []byte, value []byte) (updated bool, err error) func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { - version := tree.version + 1 - if node.isLeaf() { - if !tree.skipFastStorageUpgrade { - tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) - } - switch bytes.Compare(key, node.key) { - case -1: // setKey < leafKey - return &Node{ - key: node.key, - subtreeHeight: 1, - size: 2, - nodeKey: nil, - leftNode: NewNode(key, value), - rightNode: node, - }, false, nil - case 1: // setKey > leafKey - return &Node{ - key: key, - subtreeHeight: 1, - size: 2, - nodeKey: nil, - leftNode: node, - rightNode: NewNode(key, value), - }, false, nil - default: - return NewNode(key, value), true, nil - } + return tree.recursiveSetLeaf(node, key, value) } else { node, err = node.clone(tree) if err != nil { @@ -327,6 +301,37 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( } } +func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) ( + newSelf *Node, updated bool, err error, +) { + version := tree.version + 1 + if !tree.skipFastStorageUpgrade { + tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) + } + switch bytes.Compare(key, node.key) { + case -1: // setKey < leafKey + return &Node{ + key: node.key, + subtreeHeight: 1, + size: 2, + nodeKey: nil, + leftNode: NewNode(key, value), + rightNode: node, + }, false, nil + case 1: // setKey > leafKey + return &Node{ + key: key, + subtreeHeight: 1, + size: 2, + nodeKey: nil, + leftNode: node, + rightNode: NewNode(key, value), + }, false, nil + default: + return NewNode(key, value), true, nil + } +} + // Remove removes a key from the working tree. The given key byte slice should not be modified // after this call, since it may point to data stored inside IAVL. func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { diff --git a/node.go b/node.go index 97d54a865..0f1bc0542 100644 --- a/node.go +++ b/node.go @@ -57,11 +57,15 @@ func GetRootKey(version int64) []byte { // Node represents a node in a Tree. type Node struct { - key []byte - value []byte - hash []byte - nodeKey *NodeKey - leftNodeKey []byte + key []byte + value []byte + hash []byte + nodeKey *NodeKey + // Legacy: LeftNodeHash + // v1: Left node ptr via Version/key + leftNodeKey []byte + // Legacy: RightNodeHash + // v1: Right node ptr via Version/key rightNodeKey []byte size int64 leftNode *Node @@ -517,19 +521,29 @@ func (node *Node) writeHashBytes(w io.Writer, version int64) error { // (e.g. ProofLeafNode.ValueHash) valueHash := sha256.Sum256(node.value) - err = encoding.EncodeBytes(w, valueHash[:]) + err = encoding.Encode32BytesHash(w, valueHash[:]) if err != nil { return fmt.Errorf("writing value, %w", err) } } else { - if node.leftNode == nil || node.rightNode == nil { + if (node.leftNode == nil && len(node.leftNodeKey) != 32) || (node.rightNode == nil && len(node.rightNodeKey) != 32) { return ErrEmptyChild } - err = encoding.EncodeBytes(w, node.leftNode.hash) + // If left/rightNodeKey is 32 bytes, it is a legacy node whose value is just the hash. + // We may have skipped fetching leftNode/rightNode. + if len(node.leftNodeKey) == 32 { + err = encoding.Encode32BytesHash(w, node.leftNodeKey) + } else { + err = encoding.Encode32BytesHash(w, node.leftNode.hash) + } if err != nil { return fmt.Errorf("writing left hash, %w", err) } - err = encoding.EncodeBytes(w, node.rightNode.hash) + if len(node.rightNodeKey) == 32 { + err = encoding.Encode32BytesHash(w, node.rightNodeKey) + } else { + err = encoding.Encode32BytesHash(w, node.rightNode.hash) + } if err != nil { return fmt.Errorf("writing right hash, %w", err) } From c6c34dc2fd289455e53754461234d77a9bab46ff Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sat, 23 Dec 2023 11:27:27 -0600 Subject: [PATCH 09/26] Speedup key formatting --- keyformat/prefix_formatter.go | 41 +++++++++++++++++++++++++++++++++++ nodedb.go | 18 +++++++-------- 2 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 keyformat/prefix_formatter.go diff --git a/keyformat/prefix_formatter.go b/keyformat/prefix_formatter.go new file mode 100644 index 000000000..8f345a1a6 --- /dev/null +++ b/keyformat/prefix_formatter.go @@ -0,0 +1,41 @@ +package keyformat + +import "encoding/binary" + +// This file builds some dedicated key formatters for what appears in benchmarks. + +// Prefixes a single byte before a 32 byte hash. +type FastPrefixFormatter struct { + prefix byte + length int + prefixSlice []byte +} + +func NewFastPrefixFormatter(prefix byte, length int) *FastPrefixFormatter { + return &FastPrefixFormatter{prefix: prefix, length: length, prefixSlice: []byte{prefix}} +} + +func (f *FastPrefixFormatter) Key(bz []byte) []byte { + key := make([]byte, 1+f.length) + key[0] = f.prefix + copy(key[1:], bz) + return key +} + +func (f *FastPrefixFormatter) Scan(key []byte, a interface{}) { + scan(a, key[1:]) +} + +func (f *FastPrefixFormatter) KeyInt64(bz int64) []byte { + key := make([]byte, 1+f.length) + key[0] = f.prefix + binary.BigEndian.PutUint64(key[1:], uint64(bz)) + return key +} + +func (f *FastPrefixFormatter) Prefix() []byte { + return f.prefixSlice +} +func (f *FastPrefixFormatter) Length() int { + return 1 + f.length +} diff --git a/nodedb.go b/nodedb.go index 5194a85e0..228a1aeef 100644 --- a/nodedb.go +++ b/nodedb.go @@ -41,10 +41,10 @@ const ( var ( // All new node keys are prefixed with the byte 's'. This ensures no collision is // possible with the legacy nodes, and makes them easier to traverse. They are indexed by the version and the local nonce. - nodeKeyFormat = keyformat.NewKeyFormat('s', int64Size+int32Size) // s + nodeKeyFormat = keyformat.NewFastPrefixFormatter('s', int64Size+int32Size) // s // This is only used for the iteration purpose. - nodeKeyPrefixFormat = keyformat.NewKeyFormat('s', int64Size) // s + nodeKeyPrefixFormat = keyformat.NewFastPrefixFormatter('s', int64Size) // s // Key Format for making reads and iterates go through a data-locality preserving db. // The value at an entry will list what version it was written to. @@ -59,7 +59,7 @@ var ( metadataKeyFormat = keyformat.NewKeyFormat('m', 0) // m // All legacy node keys are prefixed with the byte 'n'. - legacyNodeKeyFormat = keyformat.NewKeyFormat('n', hashSize) // n + legacyNodeKeyFormat = keyformat.NewFastPrefixFormatter('n', hashSize) // n // All legacy orphan keys are prefixed with the byte 'o'. legacyOrphanKeyFormat = keyformat.NewKeyFormat('o', int64Size, int64Size, hashSize) // o @@ -584,7 +584,7 @@ func (ndb *nodeDB) DeleteVersionsFrom(fromVersion int64) error { } // Delete the nodes for new format - err = ndb.traverseRange(nodeKeyPrefixFormat.Key(fromVersion), nodeKeyPrefixFormat.Key(latest+1), func(k, v []byte) error { + err = ndb.traverseRange(nodeKeyPrefixFormat.KeyInt64(fromVersion), nodeKeyPrefixFormat.KeyInt64(latest+1), func(k, v []byte) error { return ndb.batch.Delete(k) }) @@ -664,7 +664,7 @@ func (ndb *nodeDB) nodeKey(nk []byte) []byte { } func (ndb *nodeDB) nodeKeyPrefix(version int64) []byte { - return nodeKeyPrefixFormat.Key(version) + return nodeKeyPrefixFormat.KeyInt64(version) } func (ndb *nodeDB) fastNodeKey(key []byte) []byte { @@ -760,8 +760,8 @@ func (ndb *nodeDB) getLatestVersion() (int64, error) { } itr, err := ndb.db.ReverseIterator( - nodeKeyPrefixFormat.Key(int64(1)), - nodeKeyPrefixFormat.Key(int64(math.MaxInt64)), + nodeKeyPrefixFormat.KeyInt64(int64(1)), + nodeKeyPrefixFormat.KeyInt64(int64(math.MaxInt64)), ) if err != nil { return 0, err @@ -1080,7 +1080,7 @@ func isReferenceToRoot(bz []byte) bool { func (ndb *nodeDB) traverseNodes(fn func(node *Node) error) error { nodes := []*Node{} - if err := ndb.traversePrefix(nodeKeyFormat.Key(), func(key, value []byte) error { + if err := ndb.traversePrefix(nodeKeyFormat.Prefix(), func(key, value []byte) error { if isReferenceToRoot(value) { return nil } @@ -1163,7 +1163,7 @@ func (ndb *nodeDB) String() (string, error) { index := 0 - err := ndb.traversePrefix(nodeKeyFormat.Key(), func(key, value []byte) error { + err := ndb.traversePrefix(nodeKeyFormat.Prefix(), func(key, value []byte) error { fmt.Fprintf(buf, "%s: %x\n", key, value) return nil }) From b7ccd32a43640cdb3e3a4ae3d72d60106ca27b66 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 11:12:31 -0600 Subject: [PATCH 10/26] Avoid making an extra heap copy in DecodeBytes --- fastnode/fast_node.go | 1 + internal/encoding/encoding.go | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/fastnode/fast_node.go b/fastnode/fast_node.go index 149ac9b26..5d00bd997 100644 --- a/fastnode/fast_node.go +++ b/fastnode/fast_node.go @@ -30,6 +30,7 @@ func NewNode(key []byte, value []byte, version int64) *Node { } // DeserializeNode constructs an *FastNode from an encoded byte slice. +// It assumes we do not mutate this input []byte. func DeserializeNode(key []byte, buf []byte) (*Node, error) { ver, n, err := encoding.DecodeVarint(buf) if err != nil { diff --git a/internal/encoding/encoding.go b/internal/encoding/encoding.go index e198a7937..1ebdd53f1 100644 --- a/internal/encoding/encoding.go +++ b/internal/encoding/encoding.go @@ -30,6 +30,7 @@ var uvarintPool = &sync.Pool{ // decodeBytes decodes a varint length-prefixed byte slice, returning it along with the number // of input bytes read. +// Assumes bz will not be mutated. func DecodeBytes(bz []byte) ([]byte, int, error) { s, n, err := DecodeUvarint(bz) if err != nil { @@ -51,9 +52,9 @@ func DecodeBytes(bz []byte) ([]byte, int, error) { if len(bz) < end { return nil, n, fmt.Errorf("insufficient bytes decoding []byte of length %v", size) } - bz2 := make([]byte, size) - copy(bz2, bz[n:end]) - return bz2, end, nil + // bz2 := make([]byte, size) + // copy(bz2, bz[n:end]) + return bz[n:end], end, nil } // decodeUvarint decodes a varint-encoded unsigned integer from a byte slice, returning it and the From 722e09df6de0b730c43d54ba6667f3a09722521f Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 16:07:35 -0600 Subject: [PATCH 11/26] Single side fetch for legacy --- mutable_tree.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++-- node.go | 41 +++++++++++++++++++++++++++++-- 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index f342c04e0..0ad0722da 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -275,12 +275,20 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( } if bytes.Compare(key, node.key) < 0 { - node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) + if len(node.leftNodeKey) == 32 { + node.leftNode, updated, err = tree.recursiveSetLegacy(node.leftNode, key, value) + } else { + node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) + } if err != nil { return nil, updated, err } } else { - node.rightNode, updated, err = tree.recursiveSet(node.rightNode, key, value) + if len(node.rightNodeKey) == 32 { + node.rightNode, updated, err = tree.recursiveSetLegacy(node.rightNode, key, value) + } else { + node.rightNode, updated, err = tree.recursiveSet(node.rightNode, key, value) + } if err != nil { return nil, updated, err } @@ -332,6 +340,58 @@ func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) } } +// recursiveSetLegacy is the same as recursiveSet but takes an optimization where +// if you are updating an existing leaf, you do not need to get both children, you only need one child. +// +// This operates on the assumption that when recursiveSet enters a legacy node, +// all of the legacy nodes children will be legacy nodes. +func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte) ( + newSelf *Node, updated bool, err error, +) { + if node.isLeaf() { + return tree.recursiveSetLeaf(node, key, value) + } else { + node, err = node.cloneNoChildFetch(tree) + if err != nil { + return nil, false, err + } + + recurseLeft := false + if bytes.Compare(key, node.key) < 0 { + recurseLeft = true + } + child, err := node.fetchOneChild(tree, recurseLeft) + if err != nil { + return nil, false, err + } + + newChild, updated, err := tree.recursiveSetLegacy(child, key, value) + if err != nil { + return nil, updated, err + } + if recurseLeft { + node.leftNode = newChild + } else { + node.rightNode = newChild + } + + if updated { + return node, updated, nil + } + node.fetchOneChild(tree, !recurseLeft) + + err = node.calcHeightAndSize(tree.ImmutableTree) + if err != nil { + return nil, false, err + } + newNode, err := tree.balance(node) + if err != nil { + return nil, false, err + } + return newNode, updated, err + } +} + // Remove removes a key from the working tree. The given key byte slice should not be modified // after this call, since it may point to data stored inside IAVL. func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { diff --git a/node.go b/node.go index 0f1bc0542..5ce14d7ca 100644 --- a/node.go +++ b/node.go @@ -323,6 +323,43 @@ func (node *Node) clone(tree *MutableTree) (*Node, error) { }, nil } +// cloneNoChildFetch clones a node without fetching its children. +func (node *Node) cloneNoChildFetch(tree *MutableTree) (*Node, error) { + if node.isLeaf() { + return nil, ErrCloneLeafNode + } + + return &Node{ + key: node.key, + subtreeHeight: node.subtreeHeight, + size: node.size, + hash: nil, + nodeKey: nil, + leftNodeKey: node.leftNodeKey, + rightNodeKey: node.rightNodeKey, + }, nil +} + +func (node *Node) fetchOneChild(tree *MutableTree, left bool) (*Node, error) { + var err error + var child *Node + if left { + child, err = node.getLeftNode(tree.ImmutableTree) + if err != nil { + return nil, err + } + node.leftNode = child + return child, nil + } + child, err = node.getRightNode(tree.ImmutableTree) + if err != nil { + return nil, err + } + node.rightNode = child + + return child, nil +} + func (node *Node) isLeaf() bool { return node.subtreeHeight == 0 } @@ -629,7 +666,7 @@ func (node *Node) writeBytes(w io.Writer) error { return fmt.Errorf("writing mode, %w", err) } if mode&ModeLegacyLeftNode != 0 { // legacy leftNodeKey - err = encoding.EncodeBytes(w, node.leftNodeKey) + err = encoding.Encode32BytesHash(w, node.leftNodeKey) if err != nil { return fmt.Errorf("writing the legacy left node key, %w", err) } @@ -648,7 +685,7 @@ func (node *Node) writeBytes(w io.Writer) error { return ErrRightNodeKeyEmpty } if mode&ModeLegacyRightNode != 0 { // legacy rightNodeKey - err = encoding.EncodeBytes(w, node.rightNodeKey) + err = encoding.Encode32BytesHash(w, node.rightNodeKey) if err != nil { return fmt.Errorf("writing the legacy right node key, %w", err) } From 6663e382b932b11c0fa11408413dc355d7d1a112 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 16:32:22 -0600 Subject: [PATCH 12/26] tryfix issue --- node.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/node.go b/node.go index 5ce14d7ca..2ffc867a7 100644 --- a/node.go +++ b/node.go @@ -328,6 +328,9 @@ func (node *Node) cloneNoChildFetch(tree *MutableTree) (*Node, error) { if node.isLeaf() { return nil, ErrCloneLeafNode } + // match compatability with old by clearing original node's pointer ref's. I don't really get why this is needed. + leftNode, rightNode := node.leftNode, node.rightNode + node.leftNode, node.rightNode = nil, nil return &Node{ key: node.key, @@ -335,6 +338,8 @@ func (node *Node) cloneNoChildFetch(tree *MutableTree) (*Node, error) { size: node.size, hash: nil, nodeKey: nil, + leftNode: leftNode, + rightNode: rightNode, leftNodeKey: node.leftNodeKey, rightNodeKey: node.rightNodeKey, }, nil From 2628908b03b06b47bfdb95ffeb919533f404f573 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 16:49:26 -0600 Subject: [PATCH 13/26] tryfix again --- node.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/node.go b/node.go index 2ffc867a7..076107142 100644 --- a/node.go +++ b/node.go @@ -329,9 +329,11 @@ func (node *Node) cloneNoChildFetch(tree *MutableTree) (*Node, error) { return nil, ErrCloneLeafNode } // match compatability with old by clearing original node's pointer ref's. I don't really get why this is needed. - leftNode, rightNode := node.leftNode, node.rightNode - node.leftNode, node.rightNode = nil, nil - + var leftNode, rightNode *Node + if node.nodeKey != nil { + leftNode, rightNode = node.leftNode, node.rightNode + node.leftNode, node.rightNode = nil, nil + } return &Node{ key: node.key, subtreeHeight: node.subtreeHeight, From 49995caa56b8e88d906912385354bcbbbcaf7a65 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 17:18:12 -0600 Subject: [PATCH 14/26] make debug logs --- mutable_tree.go | 137 +++++++++++++++++++++++++----------------------- 1 file changed, 71 insertions(+), 66 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 0ad0722da..779fa0685 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -161,6 +161,7 @@ func (tree *MutableTree) String() (string, error) { // to slices stored within IAVL. It returns true when an existing value was // updated, while false means it was a new key. func (tree *MutableTree) Set(key, value []byte) (updated bool, err error) { + fmt.Println("Set", key) updated, err = tree.set(key, value) if err != nil { return false, err @@ -268,50 +269,51 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( ) { if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) - } else { - node, err = node.clone(tree) - if err != nil { - return nil, false, err - } + } + fmt.Println("Recursive Set", node.key) + node, err = node.clone(tree) + if err != nil { + return nil, false, err + } - if bytes.Compare(key, node.key) < 0 { - if len(node.leftNodeKey) == 32 { - node.leftNode, updated, err = tree.recursiveSetLegacy(node.leftNode, key, value) - } else { - node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) - } - if err != nil { - return nil, updated, err - } + if bytes.Compare(key, node.key) < 0 { + if len(node.leftNodeKey) == 32 { + node.leftNode, updated, err = tree.recursiveSetLegacy(node.leftNode, key, value) } else { - if len(node.rightNodeKey) == 32 { - node.rightNode, updated, err = tree.recursiveSetLegacy(node.rightNode, key, value) - } else { - node.rightNode, updated, err = tree.recursiveSet(node.rightNode, key, value) - } - if err != nil { - return nil, updated, err - } - } - - if updated { - return node, updated, nil + node.leftNode, updated, err = tree.recursiveSet(node.leftNode, key, value) } - err = node.calcHeightAndSize(tree.ImmutableTree) if err != nil { - return nil, false, err + return nil, updated, err + } + } else { + if len(node.rightNodeKey) == 32 { + node.rightNode, updated, err = tree.recursiveSetLegacy(node.rightNode, key, value) + } else { + node.rightNode, updated, err = tree.recursiveSet(node.rightNode, key, value) } - newNode, err := tree.balance(node) if err != nil { - return nil, false, err + return nil, updated, err } - return newNode, updated, err } + + if updated { + return node, updated, nil + } + err = node.calcHeightAndSize(tree.ImmutableTree) + if err != nil { + return nil, false, err + } + newNode, err := tree.balance(node) + if err != nil { + return nil, false, err + } + return newNode, updated, err } func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { + fmt.Println("Recursive Set Leaf", node.key) version := tree.version + 1 if !tree.skipFastStorageUpgrade { tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) @@ -350,46 +352,46 @@ func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte ) { if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) - } else { - node, err = node.cloneNoChildFetch(tree) - if err != nil { - return nil, false, err - } + } + fmt.Println("Recursive Set Legacy", string(node.key)) + node, err = node.cloneNoChildFetch(tree) + if err != nil { + return nil, false, err + } - recurseLeft := false - if bytes.Compare(key, node.key) < 0 { - recurseLeft = true - } - child, err := node.fetchOneChild(tree, recurseLeft) - if err != nil { - return nil, false, err - } + recurseLeft := false + if bytes.Compare(key, node.key) < 0 { + recurseLeft = true + } + child, err := node.fetchOneChild(tree, recurseLeft) + if err != nil { + return nil, false, err + } - newChild, updated, err := tree.recursiveSetLegacy(child, key, value) - if err != nil { - return nil, updated, err - } - if recurseLeft { - node.leftNode = newChild - } else { - node.rightNode = newChild - } + newChild, updated, err := tree.recursiveSetLegacy(child, key, value) + if err != nil { + return nil, updated, err + } + if recurseLeft { + node.leftNode = newChild + } else { + node.rightNode = newChild + } - if updated { - return node, updated, nil - } - node.fetchOneChild(tree, !recurseLeft) + if updated { + return node, updated, nil + } + node.fetchOneChild(tree, !recurseLeft) - err = node.calcHeightAndSize(tree.ImmutableTree) - if err != nil { - return nil, false, err - } - newNode, err := tree.balance(node) - if err != nil { - return nil, false, err - } - return newNode, updated, err + err = node.calcHeightAndSize(tree.ImmutableTree) + if err != nil { + return nil, false, err } + newNode, err := tree.balance(node) + if err != nil { + return nil, false, err + } + return newNode, updated, err } // Remove removes a key from the working tree. The given key byte slice should not be modified @@ -398,6 +400,7 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { if tree.root == nil { return nil, false, nil } + fmt.Println("Remove", string(key)) newRoot, _, value, removed, err := tree.recursiveRemove(tree.root, key) if err != nil { return nil, false, err @@ -422,7 +425,9 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { // - the removed value func (tree *MutableTree) recursiveRemove(node *Node, key []byte) (newSelf *Node, newKey []byte, newValue []byte, removed bool, err error) { tree.logger.Debug("recursiveRemove", "node", node, "key", key) + fmt.Println("Recursive remove", string(node.key)) if node.isLeaf() { + if bytes.Equal(key, node.key) { return nil, nil, node.value, true, nil } From 225999554967e9ea6de51f0128456f39fea99a32 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sun, 24 Dec 2023 17:36:14 -0600 Subject: [PATCH 15/26] better debug logs --- mutable_tree.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index 779fa0685..d14341d82 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -267,10 +267,10 @@ func (tree *MutableTree) set(key []byte, value []byte) (updated bool, err error) func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { + fmt.Println("Recursive Set cur_node=", string(node.key)) if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) } - fmt.Println("Recursive Set", node.key) node, err = node.clone(tree) if err != nil { return nil, false, err @@ -313,7 +313,7 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { - fmt.Println("Recursive Set Leaf", node.key) + fmt.Println("Recursive Set Leaf, leaf node key=", string(node.key)) version := tree.version + 1 if !tree.skipFastStorageUpgrade { tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) @@ -350,10 +350,10 @@ func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { + fmt.Println("Recursive Set Legacy cur_node=", string(node.key)) if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) } - fmt.Println("Recursive Set Legacy", string(node.key)) node, err = node.cloneNoChildFetch(tree) if err != nil { return nil, false, err @@ -425,7 +425,7 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { // - the removed value func (tree *MutableTree) recursiveRemove(node *Node, key []byte) (newSelf *Node, newKey []byte, newValue []byte, removed bool, err error) { tree.logger.Debug("recursiveRemove", "node", node, "key", key) - fmt.Println("Recursive remove", string(node.key)) + fmt.Println("Recursive remove cur_node=", string(node.key)) if node.isLeaf() { if bytes.Equal(key, node.key) { From 8edf0b7fac3f6a6c53e415cdc0a5251a1633423d Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:37:29 -0700 Subject: [PATCH 16/26] clean up --- internal/encoding/encoding.go | 2 -- mutable_tree.go | 7 ------- 2 files changed, 9 deletions(-) diff --git a/internal/encoding/encoding.go b/internal/encoding/encoding.go index 1ebdd53f1..78a4f9899 100644 --- a/internal/encoding/encoding.go +++ b/internal/encoding/encoding.go @@ -52,8 +52,6 @@ func DecodeBytes(bz []byte) ([]byte, int, error) { if len(bz) < end { return nil, n, fmt.Errorf("insufficient bytes decoding []byte of length %v", size) } - // bz2 := make([]byte, size) - // copy(bz2, bz[n:end]) return bz[n:end], end, nil } diff --git a/mutable_tree.go b/mutable_tree.go index d14341d82..daa5a2baa 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -161,7 +161,6 @@ func (tree *MutableTree) String() (string, error) { // to slices stored within IAVL. It returns true when an existing value was // updated, while false means it was a new key. func (tree *MutableTree) Set(key, value []byte) (updated bool, err error) { - fmt.Println("Set", key) updated, err = tree.set(key, value) if err != nil { return false, err @@ -267,7 +266,6 @@ func (tree *MutableTree) set(key []byte, value []byte) (updated bool, err error) func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { - fmt.Println("Recursive Set cur_node=", string(node.key)) if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) } @@ -313,7 +311,6 @@ func (tree *MutableTree) recursiveSet(node *Node, key []byte, value []byte) ( func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { - fmt.Println("Recursive Set Leaf, leaf node key=", string(node.key)) version := tree.version + 1 if !tree.skipFastStorageUpgrade { tree.addUnsavedAddition(key, fastnode.NewNode(key, value, version)) @@ -350,7 +347,6 @@ func (tree *MutableTree) recursiveSetLeaf(node *Node, key []byte, value []byte) func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte) ( newSelf *Node, updated bool, err error, ) { - fmt.Println("Recursive Set Legacy cur_node=", string(node.key)) if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) } @@ -400,7 +396,6 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { if tree.root == nil { return nil, false, nil } - fmt.Println("Remove", string(key)) newRoot, _, value, removed, err := tree.recursiveRemove(tree.root, key) if err != nil { return nil, false, err @@ -425,9 +420,7 @@ func (tree *MutableTree) Remove(key []byte) ([]byte, bool, error) { // - the removed value func (tree *MutableTree) recursiveRemove(node *Node, key []byte) (newSelf *Node, newKey []byte, newValue []byte, removed bool, err error) { tree.logger.Debug("recursiveRemove", "node", node, "key", key) - fmt.Println("Recursive remove cur_node=", string(node.key)) if node.isLeaf() { - if bytes.Equal(key, node.key) { return nil, nil, node.value, true, nil } From 1081debe57843fea1b72e8d2f6a403c766d3c344 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:40:29 -0700 Subject: [PATCH 17/26] upgrade lint CI to use go 1.21 --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 2a4cd670f..181866713 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -15,7 +15,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '^1.20.0' + go-version: "^1.21.0" - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: From 236067a6d6c23eb0a6f5ddcf78c34ac6af328191 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:48:49 -0700 Subject: [PATCH 18/26] attempt fix linter --- .github/workflows/lint.yml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 181866713..b929dd76a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -12,11 +12,19 @@ jobs: name: golangci-lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@v2.2.0 with: - go-version: "^1.21.0" + go-version: 1.20 + - uses: technote-space/get-diff-action@v6.0.1 + id: git_diff + with: + PATTERNS: | + **/**.go + go.mod + go.sum - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.51.2 + version: latest + args: --out-format=tab + if: env.GIT_DIFF From 826e663331dd7cf01f40414ed2db21e250ff48f7 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:49:57 -0700 Subject: [PATCH 19/26] go 1.21 --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b929dd76a..36f56e14a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,7 +14,7 @@ jobs: steps: - uses: actions/setup-go@v2.2.0 with: - go-version: 1.20 + go-version: 1.21 - uses: technote-space/get-diff-action@v6.0.1 id: git_diff with: From 27f52b2c4d8785c6c20ad7dd331ce1f36c65f79f Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:51:12 -0700 Subject: [PATCH 20/26] remove diff check --- .github/workflows/lint.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 36f56e14a..8468b3d40 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -15,16 +15,8 @@ jobs: - uses: actions/setup-go@v2.2.0 with: go-version: 1.21 - - uses: technote-space/get-diff-action@v6.0.1 - id: git_diff - with: - PATTERNS: | - **/**.go - go.mod - go.sum - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: version: latest args: --out-format=tab - if: env.GIT_DIFF From 03fc7c08b2aa0ab7da0b86b3f3372d613e22f81e Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:55:31 -0700 Subject: [PATCH 21/26] add makefile cmd --- .github/workflows/lint.yml | 5 +---- Makefile | 5 +++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8468b3d40..59ea86c21 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,7 +16,4 @@ jobs: with: go-version: 1.21 - name: golangci-lint - uses: golangci/golangci-lint-action@v3 - with: - version: latest - args: --out-format=tab + run: make lint-all diff --git a/Makefile b/Makefile index b23af8b26..ed9e7205d 100644 --- a/Makefile +++ b/Makefile @@ -48,6 +48,11 @@ lint-fix: @go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(golangci_version) @$(golangci_lint_cmd) run --fix --out-format=tab --issues-exit-code=0 +lint-all: + @echo "--> Running linter" + @go run github.com/golangci/golangci-lint/cmd/golangci-lint run --timeout=10m + @docker run -v $(PWD):/workdir ghcr.io/igorshubovych/markdownlint-cli:latest "**/*.md" + # bench is the basic tests that shouldn't crash an aws instance bench: cd benchmarks && \ From a03a1fdc93a561e4fcf3b781746ef86964e8ccd2 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 17:57:30 -0700 Subject: [PATCH 22/26] existing lint cmd --- .github/workflows/lint.yml | 2 +- Makefile | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 59ea86c21..35b663af6 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,4 +16,4 @@ jobs: with: go-version: 1.21 - name: golangci-lint - run: make lint-all + run: make lint diff --git a/Makefile b/Makefile index ed9e7205d..b23af8b26 100644 --- a/Makefile +++ b/Makefile @@ -48,11 +48,6 @@ lint-fix: @go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(golangci_version) @$(golangci_lint_cmd) run --fix --out-format=tab --issues-exit-code=0 -lint-all: - @echo "--> Running linter" - @go run github.com/golangci/golangci-lint/cmd/golangci-lint run --timeout=10m - @docker run -v $(PWD):/workdir ghcr.io/igorshubovych/markdownlint-cli:latest "**/*.md" - # bench is the basic tests that shouldn't crash an aws instance bench: cd benchmarks && \ From e0e723c8479b07aa011413a527fc7715cbea9090 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 18:00:51 -0700 Subject: [PATCH 23/26] check out repo code --- .github/workflows/lint.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 35b663af6..1e03feecc 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -12,7 +12,10 @@ jobs: name: golangci-lint runs-on: ubuntu-latest steps: - - uses: actions/setup-go@v2.2.0 + - name: Check out repository code + uses: actions/checkout@v4 + - name: 🐿 Setup Golang + uses: actions/setup-go@v4 with: go-version: 1.21 - name: golangci-lint From 6b2cdefb70f5fcd93524c49b6f16531d8a6ea869 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 20:13:53 -0700 Subject: [PATCH 24/26] lints --- mutable_tree.go | 7 +++++-- node.go | 4 ++-- nodedb.go | 9 ++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/mutable_tree.go b/mutable_tree.go index daa5a2baa..cec2909b5 100644 --- a/mutable_tree.go +++ b/mutable_tree.go @@ -350,7 +350,7 @@ func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte if node.isLeaf() { return tree.recursiveSetLeaf(node, key, value) } - node, err = node.cloneNoChildFetch(tree) + node, err = node.cloneNoChildFetch() if err != nil { return nil, false, err } @@ -377,7 +377,10 @@ func (tree *MutableTree) recursiveSetLegacy(node *Node, key []byte, value []byte if updated { return node, updated, nil } - node.fetchOneChild(tree, !recurseLeft) + _, err = node.fetchOneChild(tree, !recurseLeft) + if err != nil { + return nil, false, err + } err = node.calcHeightAndSize(tree.ImmutableTree) if err != nil { diff --git a/node.go b/node.go index 076107142..2d8930f8a 100644 --- a/node.go +++ b/node.go @@ -324,11 +324,11 @@ func (node *Node) clone(tree *MutableTree) (*Node, error) { } // cloneNoChildFetch clones a node without fetching its children. -func (node *Node) cloneNoChildFetch(tree *MutableTree) (*Node, error) { +func (node *Node) cloneNoChildFetch() (*Node, error) { if node.isLeaf() { return nil, ErrCloneLeafNode } - // match compatability with old by clearing original node's pointer ref's. I don't really get why this is needed. + // match compatibility with old by clearing original node's pointer ref's. I don't really get why this is needed. var leftNode, rightNode *Node if node.nodeKey != nil { leftNode, rightNode = node.leftNode, node.rightNode diff --git a/nodedb.go b/nodedb.go index 228a1aeef..aa184fad9 100644 --- a/nodedb.go +++ b/nodedb.go @@ -421,7 +421,7 @@ func (ndb *nodeDB) deleteLegacyNodes(version int64, nk []byte) error { } var isDeletingLegacyVersionsMutex *sync.Mutex = &sync.Mutex{} -var isDeletingLegacyVersions bool = false +var isDeletingLegacyVersions = false // deleteLegacyVersions deletes all legacy versions from disk. func (ndb *nodeDB) deleteLegacyVersions() error { @@ -429,10 +429,9 @@ func (ndb *nodeDB) deleteLegacyVersions() error { if isDeletingLegacyVersions { isDeletingLegacyVersionsMutex.Unlock() return nil - } else { - isDeletingLegacyVersions = true - isDeletingLegacyVersionsMutex.Unlock() } + isDeletingLegacyVersions = true + isDeletingLegacyVersionsMutex.Unlock() go func() { defer func() { @@ -458,7 +457,7 @@ func (ndb *nodeDB) deleteLegacyVersions() error { rootKeys = append(rootKeys, itr.Key()) if prevVersion > 0 { if err := ndb.traverseOrphans(prevVersion, curVersion, func(orphan *Node) error { - counter += 1 + counter++ if counter == 1000 { counter = 0 time.Sleep(1000 * time.Millisecond) From 2c321d82a6245967524d6c611feb89108c2cdd23 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 20:16:38 -0700 Subject: [PATCH 25/26] fmt --- keyformat/prefix_formatter.go | 1 + 1 file changed, 1 insertion(+) diff --git a/keyformat/prefix_formatter.go b/keyformat/prefix_formatter.go index 8f345a1a6..873c3a7e1 100644 --- a/keyformat/prefix_formatter.go +++ b/keyformat/prefix_formatter.go @@ -36,6 +36,7 @@ func (f *FastPrefixFormatter) KeyInt64(bz int64) []byte { func (f *FastPrefixFormatter) Prefix() []byte { return f.prefixSlice } + func (f *FastPrefixFormatter) Length() int { return 1 + f.length } From 50e9df0259cfa6b2d1dc372e33d340e9be7b06a8 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Mon, 25 Dec 2023 20:21:17 -0700 Subject: [PATCH 26/26] fmt --- nodedb.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nodedb.go b/nodedb.go index aa184fad9..0dfe45464 100644 --- a/nodedb.go +++ b/nodedb.go @@ -420,8 +420,10 @@ func (ndb *nodeDB) deleteLegacyNodes(version int64, nk []byte) error { return ndb.batch.Delete(ndb.legacyNodeKey(nk)) } -var isDeletingLegacyVersionsMutex *sync.Mutex = &sync.Mutex{} -var isDeletingLegacyVersions = false +var ( + isDeletingLegacyVersionsMutex = &sync.Mutex{} + isDeletingLegacyVersions = false +) // deleteLegacyVersions deletes all legacy versions from disk. func (ndb *nodeDB) deleteLegacyVersions() error {