hoytech / lmdbxx

C++17 wrapper for the LMDB embedded B+ tree database library
The Unlicense
51 stars 16 forks source link

discard tx on commit/abort in case of an error too #3

Closed coreprocess closed 3 years ago

coreprocess commented 3 years ago

Prevents double deallocation of tx on MDB_MAP_FULL when committing. I think this fix is semantically correct but judge for yourself. It avoids memory management issues regarding the transactions in case of an error when committing.

Our code reacts to MDB_MAP_FULL and MDB_MAP_RESIZED errors by updating the mapsize (once all tx are closed) and rescheduling the database ops. Without this fix, we have memory lifecycle issues in the LMDB code.

hoytech commented 3 years ago

It looks good to me. Please give me a short time to write a test to verify the problem and fix before I merge. Thank you!

hoytech commented 3 years ago

Merged. I added a test which needs to be manually enabled since it depends on how LMDB lays out the DB internally, which could change: https://github.com/hoytech/lmdbxx/blob/master/check.cc#L329

And I also mentioned this fix in the README.

Thank you!