-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Get() to work with new faster cache #447
Changes from 8 commits
6903620
dd739ee
c998b07
8e94c38
f1a9627
943c6f4
4f8716c
e8c675f
c004b17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,3 +12,6 @@ cpu*.out | |
mem*.out | ||
cpu*.pdf | ||
mem*.pdf | ||
|
||
# IDE files | ||
.idea/* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package iavl | ||
|
||
import ( | ||
"github.com/pkg/errors" | ||
) | ||
|
||
// NOTE: This file favors int64 as opposed to int for size/counts. | ||
// The Tree on the other hand favors int. This is intentional. | ||
|
||
type FastNode struct { | ||
key []byte | ||
versionLastUpdatedAt int64 | ||
value []byte | ||
leafHash []byte // TODO: Look into if this would help with proof stuff. | ||
} | ||
|
||
// NewFastNode returns a new fast node from a value and version. | ||
func NewFastNode(key []byte, value []byte, version int64) *FastNode { | ||
return &FastNode{ | ||
key: key, | ||
versionLastUpdatedAt: version, | ||
value: value, | ||
} | ||
} | ||
|
||
// DeserializeFastNode constructs an *FastNode from an encoded byte slice. | ||
func DeserializeFastNode(buf []byte) (*FastNode, error) { | ||
val, n, cause := decodeBytes(buf) | ||
if cause != nil { | ||
return nil, errors.Wrap(cause, "decoding fastnode.value") | ||
} | ||
buf = buf[n:] | ||
|
||
ver, _, cause := decodeVarint(buf) | ||
if cause != nil { | ||
return nil, errors.Wrap(cause, "decoding fastnode.version") | ||
} | ||
|
||
fastNode := &FastNode{ | ||
versionLastUpdatedAt: ver, | ||
value: val, | ||
} | ||
|
||
return fastNode, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,10 @@ type nodeDB struct { | |
nodeCache map[string]*list.Element // Node cache. | ||
nodeCacheSize int // Node cache size limit in elements. | ||
nodeCacheQueue *list.List // LRU queue of cache elements. Used for deletion. | ||
|
||
fastNodeCache map[string]*list.Element // FastNode cache. | ||
fastNodeCacheSize int // FastNode cache size limit in elements. | ||
fastNodeCacheQueue *list.List // LRU queue of cache elements. Used for deletion. | ||
} | ||
|
||
func newNodeDB(db dbm.DB, cacheSize int, opts *Options) *nodeDB { | ||
|
@@ -64,14 +68,17 @@ func newNodeDB(db dbm.DB, cacheSize int, opts *Options) *nodeDB { | |
opts = &o | ||
} | ||
return &nodeDB{ | ||
db: db, | ||
batch: db.NewBatch(), | ||
opts: *opts, | ||
latestVersion: 0, // initially invalid | ||
nodeCache: make(map[string]*list.Element), | ||
nodeCacheSize: cacheSize, | ||
nodeCacheQueue: list.New(), | ||
versionReaders: make(map[int64]uint32, 8), | ||
db: db, | ||
batch: db.NewBatch(), | ||
opts: *opts, | ||
latestVersion: 0, // initially invalid | ||
nodeCache: make(map[string]*list.Element), | ||
nodeCacheSize: cacheSize, | ||
nodeCacheQueue: list.New(), | ||
fastNodeCache: make(map[string]*list.Element), | ||
fastNodeCacheSize: cacheSize, | ||
fastNodeCacheQueue: list.New(), | ||
versionReaders: make(map[int64]uint32, 8), | ||
} | ||
} | ||
|
||
|
@@ -113,6 +120,41 @@ func (ndb *nodeDB) GetNode(hash []byte) *Node { | |
return node | ||
} | ||
|
||
func (ndb *nodeDB) GetFastNode(key []byte) (*FastNode, error) { | ||
ndb.mtx.Lock() | ||
defer ndb.mtx.Unlock() | ||
jtieri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if len(key) == 0 { | ||
panic("nodeDB.GetFastNode() requires key") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please do not panic! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch Robert, I meant to replace all the panics used with the traditional node logic with errors so that failure to fetch a fast node simply falls back to the original logic in IAVL. I'll get this taken care of in the next PR |
||
|
||
// TODO make a second write lock just for fastNodeCacheQueue later | ||
// Check the cache. | ||
if elem, ok := ndb.fastNodeCache[string(key)]; ok { | ||
// Already exists. Move to back of fastNodeCacheQueue. | ||
ndb.fastNodeCacheQueue.MoveToBack(elem) | ||
return elem.Value.(*FastNode), nil | ||
} | ||
|
||
// Doesn't exist, load. | ||
buf, err := ndb.db.Get(key) | ||
jtieri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return nil, fmt.Errorf("can't get fast-node %X: %v", key, err) | ||
} | ||
if buf == nil { | ||
return nil, fmt.Errorf("value missing for key %x ", key) | ||
} | ||
|
||
fastNode, err := DeserializeFastNode(buf) | ||
if err != nil { | ||
return nil, fmt.Errorf("error reading FastNode. bytes: %x, error: %v ", buf, err) | ||
} | ||
|
||
fastNode.key = key | ||
ndb.cacheFastNode(fastNode) | ||
return fastNode, nil | ||
} | ||
|
||
// SaveNode saves a node to disk. | ||
func (ndb *nodeDB) SaveNode(node *Node) { | ||
ndb.mtx.Lock() | ||
|
@@ -553,6 +595,26 @@ func (ndb *nodeDB) cacheNode(node *Node) { | |
} | ||
} | ||
|
||
func (ndb *nodeDB) uncacheFastNode(key []byte) { | ||
if elem, ok := ndb.fastNodeCache[string(key)]; ok { | ||
ndb.fastNodeCacheQueue.Remove(elem) | ||
delete(ndb.fastNodeCache, string(key)) | ||
} | ||
} | ||
|
||
// Add a node to the cache and pop the least recently used node if we've | ||
// reached the cache size limit. | ||
func (ndb *nodeDB) cacheFastNode(node *FastNode) { | ||
elem := ndb.fastNodeCacheQueue.PushBack(node) | ||
ndb.fastNodeCache[string(node.key)] = elem | ||
|
||
if ndb.fastNodeCacheQueue.Len() > ndb.fastNodeCacheSize { | ||
oldest := ndb.fastNodeCacheQueue.Front() | ||
key := ndb.fastNodeCacheQueue.Remove(oldest).(*FastNode).key | ||
delete(ndb.fastNodeCache, string(key)) | ||
} | ||
} | ||
|
||
// Write to disk. | ||
func (ndb *nodeDB) Commit() error { | ||
ndb.mtx.Lock() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we log error here? When this operation will fail? Would be good to add a comment. @ValarDragon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that if this
GetFastNode()
call fails for any reason, we assume there is noFastNode
equivalent for this key in the nodedb and fall back to the original, yet slower, IAVL logic in place.I'll add debug logging here and add a comment to make this more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
FastNode
is confusing. Maybe we should rename toCachedNode
?