-
Notifications
You must be signed in to change notification settings - Fork 31
Lmdb #5
Lmdb #5
Conversation
|
Results with valsz 128 |
|
|
Thanks, @hyc . Merged. |
|
Hi @hyc I have cherry-picked these commits onto master branch now as 8b9bed8 and 2b66ab3. I realise that we were not reusing the txn for the read, and your change modified the code to do that. However, I thought it would be cleaner to keep the invocation to err := lmdbEnv.View(func(txn *lmdb.Txn) error {
txn.RawRead = true
for pb.Next() {
key := newKey()
v, err := txn.Get(lmdbDBI, key)
if lmdb.IsNotFound(err) {
c.notFound++
continue
} else if err != nil {
c.errored++
continue
}
y.AssertTruef(len(v) == *flagValueSize, "Assertion failed. value size is %d, expected %d", len(v), *flagValueSize)
c.found++
}
return nil
})
if err != nil {
y.Check(err)
} |
|
No, it's not the same, but it's not clear what your test's intent really is. If you use a single transaction like this, then all of your reads are using that same snapshot. Any writes that happened after the transaction began will be invisible. Using Reset/Renew as my patch did means you're always getting the latest view of the DB, so you'd be seeing the effects of concurrent writers. |
|
Yes, you are right that your patch would make the read see the effects of concurrent writers. I missed that. However, this specific benchmark is meant to run against a database that has been pre-populated. So we don’t really need to worry about the effects of concurrent writes. It should be fine reading from the same snapshot. |
|
Ok, that's cool. |
Two tweaks - populate was extremely slow because it's fsyncing after every transaction, changed it to use Nosync.
ReadRandomLmdb seemed to be bottlenecked, changed to a reusable Read txn.