Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

merkledb -- nits #1916

Merged
merged 4 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 0 additions & 3 deletions x/merkledb/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,12 @@ import "github.com/ava-labs/avalanchego/database"

var _ database.Batch = (*batch)(nil)

// batch is a write-only database that commits changes to its host database
// when Write is called.
Comment on lines -10 to -11
Copy link
Author

Choose a reason for hiding this comment

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

This is documented on database.Batch so no need to do it here

type batch struct {
database.BatchOps

db *merkleDB
}

// apply all operations in order to the database and write the result to disk
func (b *batch) Write() error {
Copy link
Author

Choose a reason for hiding this comment

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

This is documented on database.Batch so no need to do it here

return b.db.commitBatch(b.Ops)
}
Expand Down
9 changes: 6 additions & 3 deletions x/merkledb/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import (

// A cache that calls [onEviction] on the evicted element.
type onEvictCache[K comparable, V any] struct {
lock sync.RWMutex
maxSize int
fifo linkedhashmap.LinkedHashmap[K, V]
lock sync.RWMutex
maxSize int
fifo linkedhashmap.LinkedHashmap[K, V]
// Must not call any method that grabs [c.lock]
// because this would cause a deadlock.
onEviction func(V) error
}

Expand All @@ -27,6 +29,7 @@ func newOnEvictCache[K comparable, V any](maxSize int, onEviction func(V) error)
}

// removeOldest returns and removes the oldest element from this cache.
// Assumes [c.lock] is held.
func (c *onEvictCache[K, V]) removeOldest() (K, V, bool) {
k, v, exists := c.fifo.Oldest()
if exists {
Expand Down
13 changes: 7 additions & 6 deletions x/merkledb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,8 @@ type ChangeProofer interface {
// If [end] is nothing, all keys are considered < [end].
// - [proof.KeyValues] and [proof.DeletedKeys] are sorted in order of increasing key.
// - [proof.StartProof] and [proof.EndProof] are well-formed.
// - When the keys in [proof.KeyValues] are added to [db] and the keys in [proof.DeletedKeys]
// are removed from [db], the root ID of [db] is [expectedEndRootID].
//
// This is defined on Database instead of ChangeProof because it accesses
// database internals.
// - When the changes in [proof.KeyChanes] are applied,
// the root ID of the database is [expectedEndRootID].
Comment on lines +79 to +80
Copy link
Author

Choose a reason for hiding this comment

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

proof.KeyValues and proof.DeletedKeys no longer exist

VerifyChangeProof(
ctx context.Context,
proof *ChangeProof,
Expand Down Expand Up @@ -146,6 +143,7 @@ type merkleDB struct {
// Should be held before taking [db.lock]
commitLock sync.RWMutex

// Stores this trie's nodes.
nodeDB database.Database

// Stores data about the database's current state.
Expand All @@ -155,7 +153,8 @@ type merkleDB struct {
// Note that a call to Put may cause a node to be evicted
// from the cache, which will call [OnEviction].
// A non-nil error returned from Put is considered fatal.
nodeCache onEvictCache[path, *node]
nodeCache onEvictCache[path, *node]
// Stores any error returned by [onEviction].
onEvictionErr utils.Atomic[error]
evictionBatchSize int

Expand Down Expand Up @@ -979,6 +978,8 @@ func (*merkleDB) CommitToDB(context.Context) error {
return nil
}

// This is defined on merkleDB instead of ChangeProof
// because it accesses database internals.
Comment on lines +981 to +982
Copy link
Author

Choose a reason for hiding this comment

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

Moved from ChangeProofer spec

// Assumes [db.lock] isn't held.
func (db *merkleDB) VerifyChangeProof(
ctx context.Context,
Expand Down