timur.kristof@gmail.com wrote:
Hi,
I have an app that uses LMDB, and I've experienced an interesting issue: when trying to delete a certain item with mdb_cursor_del, it crashed with the following backtrace: https://pastebin.com/7p9wtkj9
It appears that it couldn't mark a page as dirty. Here is the relevant assertion from mdb_page_dirty: rc = insert(txn->mt_u.dirty_list, &mid); mdb_tassert(txn, rc == 0); // assertion failed
What might I be doing wrong in my application that triggers this sort of error?
Take a look at the value of rc, then look in midl.c. Most likely the dirty list is too big, which means you're trying to do too much in a single transaction.
Also interesting: The database was about 60MB, and I now compacted it using mdb_copy -c. Now it is only ~6MB, and running the app with the compacted database, the above error also disappeared.
I'm not sure why the compacting fixed the problem, could somebody offer an insight about this?
Thank you guys in advance for your answers! Best regards, Timur
Hi, Howard.
This is a bug (seems minor), I fixed it in the MDBX few days ago. https://github.com/leo-yuriev/libmdbx/commit/9fd7056b4f02098914d813f38ba1ed2...
Inside freelist_save() the mt_loose_pages could be merged into the mt_free_pgs. http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=blob;f=libraries/l...
But at this point all loose_pages also still present in the mt_u.dirty_list. On the next loop inside freelist_save() a one more page could be requested, and page_alloc() could return a page which is already in the mt_u.dirty_list. In this case mdb_mid2l_insert() will return -1.
2017-10-16 13:51 GMT+03:00 Howard Chu hyc@symas.com:
timur.kristof@gmail.com wrote:
Hi,
I have an app that uses LMDB, and I've experienced an interesting issue: when trying to delete a certain item with mdb_cursor_del, it crashed with the following backtrace: https://pastebin.com/7p9wtkj9
It appears that it couldn't mark a page as dirty. Here is the relevant assertion from mdb_page_dirty: rc = insert(txn->mt_u.dirty_list, &mid); mdb_tassert(txn, rc == 0); // assertion failed
What might I be doing wrong in my application that triggers this sort of error?
Take a look at the value of rc, then look in midl.c. Most likely the dirty list is too big, which means you're trying to do too much in a single transaction.
Also interesting: The database was about 60MB, and I now compacted it using mdb_copy -c. Now it is only ~6MB, and running the app with the compacted database, the above error also disappeared.
I'm not sure why the compacting fixed the problem, could somebody offer an insight about this?
Thank you guys in advance for your answers! Best regards, Timur
-- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
On 16. okt. 2017 13:43, Леонид Юрьев wrote:
This is a bug (seems minor), I fixed it in the MDBX few days ago. https://github.com/leo-yuriev/libmdbx/commit/9fd7056b4f02098914d813f38ba1ed2...
That's rather different from mdb.c. mdb's page_touch() does not mess with loose pages, for example.
Inside freelist_save() the mt_loose_pages could be merged into the mt_free_pgs. http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=blob;f=libraries/l...
But at this point all loose_pages also still present in the mt_u.dirty_list. On the next loop inside freelist_save() a one more page could be requested, and page_alloc() could return a page which is already in the mt_u.dirty_list. In this case mdb_mid2l_insert() will return -1.
I don't understand you. page_alloc doesn't call mdb_mid2l_insert() for loose pages, since they are in dirty_list.
freelist_save() does mess with loose pages - are you saying it is doing it incorrectly? If so, that's what we need to fix.
On 16/10/17 14:56, Hallvard Breien Furuseth wrote:
On 16. okt. 2017 13:43, Леонид Юрьев wrote:
(...) Inside freelist_save() the mt_loose_pages could be merged into the mt_free_pgs. http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=blob;f=libraries/l...
But at this point all loose_pages also still present in the mt_u.dirty_list. (...)
I don't understand you. page_alloc doesn't call mdb_mid2l_insert() for loose pages, since they are in dirty_list.
Duh, never mind, I was reading your explanation backwards.
So, when freelist_save() returns loose pages to mt_free_pgs(), you remove them from mt_u.dirty_list and update dirty_room. OK.
Howard, I accidentally suggested a different fix to this recently. Maybe in LMDB 1.0 we should switch to this:
Set INITIAL_TXNID > 2, so freelist_save() always has an unused freeDB key < txnid which it can back up to. Now allow ovpage_free() and freelist_save() to create me_pghead. That means finding a nonzero value to set me_pglast to as well, or removing the assumtion that me_phead == NULL if me_pglast == 0.
Discussed on irc on Aug 10 and rejected, but it may be cleaner after all - freelist_save() can quit messing with loose pages instead of adding to the code which does that. Unless we also rejected it because it wouldn't work:-)
On Wed, 2017-10-18 at 06:18 +0200, Hallvard Breien Furuseth wrote:
On 16/10/17 14:56, Hallvard Breien Furuseth wrote:
On 16. okt. 2017 13:43, Леонид Юрьев wrote:
(...) Inside freelist_save() the mt_loose_pages could be merged into the mt_free_pgs. http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=blob;f= libraries/liblmdb/mdb.c;h=1bf81ed4c87901991b7cac885b0d8cbc4dd2343 6;hb=bb8502f08800a44a6b91a94d6478aa7101c4cc77#l3444
But at this point all loose_pages also still present in the mt_u.dirty_list. (...)
I don't understand you. page_alloc doesn't call mdb_mid2l_insert() for loose pages, since they are in dirty_list.
Duh, never mind, I was reading your explanation backwards.
So, when freelist_save() returns loose pages to mt_free_pgs(), you remove them from mt_u.dirty_list and update dirty_room. OK.
Howard, I accidentally suggested a different fix to this recently. Maybe in LMDB 1.0 we should switch to this:
Set INITIAL_TXNID > 2, so freelist_save() always has an unused freeDB key < txnid which it can back up to. Now allow ovpage_free() and freelist_save() to create me_pghead. That means finding a nonzero value to set me_pglast to as well, or removing the assumtion that me_phead == NULL if me_pglast == 0.
Discussed on irc on Aug 10 and rejected, but it may be cleaner after all - freelist_save() can quit messing with loose pages instead of adding to the code which does that. Unless we also rejected it because it wouldn't work:-)
Maybe mdb_page_dirty could be allowed to simply accept -1 as a return value from insert() and that could fix it? Since the page is already listed in the txn's dirty list, it shouldn't really matter. Or am I misunderstanding the consequences of this?
2017-10-18 19:40 GMT+03:00 timur.kristof@gmail.com:
Maybe mdb_page_dirty could be allowed to simply accept -1 as a return value from insert() and that could fix it? Since the page is already listed in the txn's dirty list, it shouldn't really matter. Or am I misunderstanding the consequences of this?
Sure this will not create problems when working without MDB_WRITEMAP. But in MDB_WRITEMAP mode duplicates in the dirty-list seems may leads some side affects. Therefore in MDBX I had planned to completely rewrite freelist_save().
Regards, Leonid.
PS. Howard, I'm doing the slides right now and will not go to bed until it's done.
On Wed, 2017-10-18 at 20:08 +0300, Леонид Юрьев wrote:
2017-10-18 19:40 GMT+03:00 timur.kristof@gmail.com:
Maybe mdb_page_dirty could be allowed to simply accept -1 as a return value from insert() and that could fix it? Since the page is already listed in the txn's dirty list, it shouldn't really matter. Or am I misunderstanding the consequences of this?
Sure this will not create problems when working without MDB_WRITEMAP. But in MDB_WRITEMAP mode duplicates in the dirty-list seems may leads some side affects. Therefore in MDBX I had planned to completely rewrite freelist_save().
I think simply accepting -1 would fix this problem for everyone but users of MDB_WRITEMAP in which case it doesn't affect the outcome because a different function is called: mdb_mid2l_append which cannot return -1.
So, I'd suggest this for mdb_page_dirty: - mdb_tassert(txn, rc == 0); + mdb_tassert(txn, rc == 0 || rc == -1);
At least until a more proper solution is found. Would that work?
Cheers, Timur
On 18/10/17 22:44, timur.kristof@gmail.com wrote:
On Wed, 2017-10-18 at 20:08 +0300, Леонид Юрьев wrote:
Sure this will not create problems when working without MDB_WRITEMAP. But in MDB_WRITEMAP mode duplicates in the dirty-list seems may leads some side affects. Therefore in MDBX I had planned to completely rewrite freelist_save().
I think simply accepting -1 would fix this problem for everyone but users of MDB_WRITEMAP in which case it doesn't affect the outcome because a different function is called: mdb_mid2l_append which cannot return -1.
No, Leo is right. Duplicates in the freelist give a broken DB.
A cleaner fix is for freelist_save() to quit messing with loose pages and just reserve room for them, like it does for me_pghead.
I've reported ITS#8756 http://www.openldap.org/its/?findid=8756.
-- Hallvard
On Thu, 2017-10-19 at 08:12 +0200, Hallvard Breien Furuseth wrote:
On 18/10/17 22:44, timur.kristof@gmail.com wrote:
On Wed, 2017-10-18 at 20:08 +0300, Леонид Юрьев wrote:
Sure this will not create problems when working without MDB_WRITEMAP. But in MDB_WRITEMAP mode duplicates in the dirty-list seems may leads some side affects. Therefore in MDBX I had planned to completely rewrite freelist_save().
I think simply accepting -1 would fix this problem for everyone but users of MDB_WRITEMAP in which case it doesn't affect the outcome because a different function is called: mdb_mid2l_append which cannot return -1.
No, Leo is right. Duplicates in the freelist give a broken DB.
I didn't suggest duplicates. I suggested accepting -1 from insert() which merely indicated that the page is already on the dirty list. I does not make a duplicate.
Or am I getting it wrong?
A cleaner fix is for freelist_save() to quit messing with loose pages and just reserve room for them, like it does for me_pghead.
I've reported ITS#8756 http://www.openldap.org/its/?findid=8756.
-- Hallvard
Hi, Timur.
There two different cases: with MDB_WRITEMAP, and without it.
1) With MDB_WRITEMAP the mdb_mid2l_append() will be used. Internally mdb_mid2l_append() don't check for duplicated, but just append a given page number to the list. Therefore when database opens with MDB_WRITEMAP this bug leads to duplicates inside the dirty-list and then (seems) to database corruption.
2) Without MDB_WRITEMAP the mdb_mid2l_insert() will be used. Internally mdb_mid2l_insert() made insertion into a sorted list, so it checks for duplicates and returns -1 for such case. Therefore when database opens without MDB_WRITEMAP this bug leads only to assertion failure, or nothing if assertions checking was disabled.
Regards, Leonid.
2017-10-19 19:11 GMT+03:00 timur.kristof@gmail.com:
On Thu, 2017-10-19 at 08:12 +0200, Hallvard Breien Furuseth wrote:
On 18/10/17 22:44, timur.kristof@gmail.com wrote:
On Wed, 2017-10-18 at 20:08 +0300, Леонид Юрьев wrote:
Sure this will not create problems when working without MDB_WRITEMAP. But in MDB_WRITEMAP mode duplicates in the dirty-list seems may leads some side affects. Therefore in MDBX I had planned to completely rewrite freelist_save().
I think simply accepting -1 would fix this problem for everyone but users of MDB_WRITEMAP in which case it doesn't affect the outcome because a different function is called: mdb_mid2l_append which cannot return -1.
No, Leo is right. Duplicates in the freelist give a broken DB.
I didn't suggest duplicates. I suggested accepting -1 from insert() which merely indicated that the page is already on the dirty list. I does not make a duplicate.
Or am I getting it wrong?
A cleaner fix is for freelist_save() to quit messing with loose pages and just reserve room for them, like it does for me_pghead.
I've reported ITS#8756 http://www.openldap.org/its/?findid=8756.
-- Hallvard
Hi Leonid,
- With MDB_WRITEMAP the mdb_mid2l_append() will be used.
Internally mdb_mid2l_append() don't check for duplicated, but just append a given page number to the list. Therefore when database opens with MDB_WRITEMAP this bug leads to duplicates inside the dirty-list and then (seems) to database corruption.
- Without MDB_WRITEMAP the mdb_mid2l_insert() will be used.
Internally mdb_mid2l_insert() made insertion into a sorted list, so it checks for duplicates and returns -1 for such case. Therefore when database opens without MDB_WRITEMAP this bug leads only to assertion failure, or nothing if assertions checking was disabled.
Yes, that's exactly what I said. That simply allowing -1 would fix the problem for the non-writemap case, because then there is no duplicate. Right?
- Timur
2017-10-20 16:34 GMT+03:00 timur.kristof@gmail.com:
Hi Leonid,
- With MDB_WRITEMAP the mdb_mid2l_append() will be used.
Internally mdb_mid2l_append() don't check for duplicated, but just append a given page number to the list. Therefore when database opens with MDB_WRITEMAP this bug leads to duplicates inside the dirty-list and then (seems) to database corruption.
- Without MDB_WRITEMAP the mdb_mid2l_insert() will be used.
Internally mdb_mid2l_insert() made insertion into a sorted list, so it checks for duplicates and returns -1 for such case. Therefore when database opens without MDB_WRITEMAP this bug leads only to assertion failure, or nothing if assertions checking was disabled.
Yes, that's exactly what I said. That simply allowing -1 would fix the problem for the non-writemap case, because then there is no duplicate. Right?
Yes, obvious.
Leonid.
On 16. okt. 2017 12:51, Howard Chu wrote:
timur.kristof@gmail.com wrote:
I have an app that uses LMDB, and I've experienced an interesting issue: when trying to delete a certain item with mdb_cursor_del, it crashed with the following backtrace: https://pastebin.com/7p9wtkj9
Weird backtrace. It says mdb_page_dirty(), which is small, streches over 300+ lines (frames #3-#4). And mdb_page_alloc() alone has no hex address for prefix. Maybe miscompilation, two liblmdb libraries linked into the same executable, or something like that? Or some wild pointer write or whatever messed things up.
It appears that it couldn't mark a page as dirty. Here is the relevant assertion from mdb_page_dirty: rc = insert(txn->mt_u.dirty_list, &mid); mdb_tassert(txn, rc == 0); // assertion failed
What might I be doing wrong in my application that triggers this sort of error?
Take a look at the value of rc, then look in midl.c.
Also txn->mt_dirty_room and txn->mt_u.dirty_list[0].mid (the array length).
Most likely the dirty list is too big, which means you're trying to do too much in a single transaction.
Shouldn't happen though. The txn should have failed earlier with MDB_TXN_FULL.
Which also shouldn't happen since LMDB should have spilled enough pages to make room - unless you have hundreds of cursors at modified pages so LMDB can't spill enough.
But we should probably test LMDB with impractically tight dirty-list arrays (i.e. a very small MDB_IDL_UM_MAX), so LMDB keeps running into such cases.
On Mon, 2017-10-16 at 13:58 +0200, Hallvard Breien Furuseth wrote:
On 16. okt. 2017 12:51, Howard Chu wrote:
timur.kristof@gmail.com wrote:
I have an app that uses LMDB, and I've experienced an interesting issue: when trying to delete a certain item with mdb_cursor_del, it crashed with the following backtrace: https://pastebin.com/7p9wtk j9
Weird backtrace. It says mdb_page_dirty(), which is small, streches over 300+ lines (frames #3-#4). And mdb_page_alloc() alone has no hex address for prefix. Maybe miscompilation, two liblmdb libraries linked into the same executable, or something like that? Or some wild pointer write or whatever messed things up.
Not sure what was going on there, maybe -O3 messed it up. Still, the issue does appear with -O0 too and here is a backtrace with -O0: https://pastebin.com/SfeMMEPH
Most likely the dirty list is too big, which means you're trying to do too much in a single transaction.
Shouldn't happen though. The txn should have failed earlier with MDB_TXN_FULL.
Which also shouldn't happen since LMDB should have spilled enough pages to make room - unless you have hundreds of cursors at modified pages so LMDB can't spill enough.
But we should probably test LMDB with impractically tight dirty-list arrays (i.e. a very small MDB_IDL_UM_MAX), so LMDB keeps running into such cases.
I've taken a look at the value of rc (see my reply to Howard), and it seems to me that Леонид Юрьев's assessment may be correct here. rc is -1 which indicates that the page (even though newly allocated, maybe a reused page?) is already on the txn's dirty pages list.
- Timur
On Mon, 2017-10-16 at 11:51 +0100, Howard Chu wrote:
It appears that it couldn't mark a page as dirty. Here is the relevant assertion from mdb_page_dirty: rc = insert(txn->mt_u.dirty_list, &mid); mdb_tassert(txn, rc == 0); // assertion failed
What might I be doing wrong in my application that triggers this sort of error?
Take a look at the value of rc, then look in midl.c.
I've taken a look. The value of rc is -1, which according to the mdb_mid2l_insert code means that it's a duplicate value, so the page is already in the txn's dirty pages list.
Since it comes from an mdb_page_alloc call, it is a bit weird that the newly allocated page is already on the list, but maybe it's a reused page?
Most likely the dirty list is too big, which means you're trying to do too much in a single transaction.
The code did work on the compacted database, and it was doing exactly the same operations on it, so this was unlikely.
- Timur
-
Hallvard Breien Furuseth -
Howard Chu -
timur.kristof＠gmail.com -
Леонид Юрьев