-
Notifications
You must be signed in to change notification settings - Fork 20.4k
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
ethdb: add pebble for experiment #21575
Conversation
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.
Please rebase the PR and squash it so it's easier to apply on top of other prs for debugging and benchmarking
65cace4
to
1d689c5
Compare
That's 3%. Don't count the chain which lives outside of the database. Curious why out previous benchmarks got 20%+. Could you start a full sync perhaps? |
restarting this as a full sync now: https://geth-bench.ethdevops.io/d/Jpk-Be5Wk/dual-geth?orgId=1&from=1606125272921&to=now |
func (db *Database) NewBatch() ethdb.Batch { | ||
return &batch{ | ||
b: db.db.NewBatch(), | ||
} | ||
} |
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.
Pebble batches should be Close
d after use -- they are pooled internally, and Close put it back into the pool. This requires changes to our batch API.
// Pebble has a single combined cache area and the write | ||
// buffers are taken from this too. Assign all available | ||
// memory allowance for cache. | ||
Cache: pebble.NewCache(int64(cache * 1024 * 1024)), |
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.
From pebble docs
// The cache is created with a
// reference count of 1. Each DB it is associated with adds a reference, so the
// creator of the cache should usually release their reference after the DB is
// created.
//
// c := pebble.NewCache(...)
// defer c.Unref()
// d, err := pebble.Open(pebble.Options{Cache: c})
We should probably try this another time. |
Heh, I was looking at this PR only yesterday, thinking the exact same thing. Apart from just "using" pebble, we could also use their delete-range operations. Currently, we do range-deletion "manually", and don't have in in the database interface (we just iterate and delete). We do make use of it pretty much though, particularly during snap sync. |
ethdb/pebble: disable sync-style db write ethdb/pebble: update configs ethdb/pebble: fixes and tests ethdb/pebble: update configs
Rebased on master, updated to latest pebble |
Running again, this PR on |
New version: #24615 |
No description provided.