Skip to content
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

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

Merged
merged 1 commit into from
Jul 23, 2020

Conversation

coreprocess
Copy link

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
Copy link
Owner

hoytech commented Jul 22, 2020

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 added a commit that referenced this pull request Jul 23, 2020
@hoytech hoytech merged commit 8b443d4 into hoytech:master Jul 23, 2020
@hoytech
Copy link
Owner

hoytech commented Jul 23, 2020

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants