Re: [Maria-developers] 7198e7c71fc: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
Hi, Aleksey! On Dec 27, Aleksey Midenkov wrote: > > > > > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > > > > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > > > * Versioning tests support > > > > As far as I can see, there are two approaches to fixing this bug. > > And if we go the way of HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS then > > your fix is good. > > > > But I'd still like to understand whether write_row is supposed to > > disturb the scan. Why should heap_write() overwrite > > info->current_ptr=pos and info->current_hash_ptr? Inserting a row > > doesn't happen at the "current position", so I don't quite understand > > why it modifies current_ptr at all. There is no functionality like > > "continue the scan from the last inserted row", right? It just doesn't > > make sense to me. > > I don't know why it works like this, maybe to keep consistency at API > level: we insert record and then we able to access it without search. > HEAP storage was copypasted from MyISAM where `lastpos` analogy also > is updated at mi_write(). Without updating `current_ptr` and `lastpos` > tests don't fail (main, funcs_1, heap). But I can't say whether the > performance is affected and how it will go in production. Thanks for the idea. I've tried to fix handler::write_row to not reset the scan. This, indeed, worked. I've also asked Monty and he also was of the opinion that handler::write_row should not reset the scan. I've pushed into a stage tree a fix with your test cases. Let's see how it'll go. Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] 7198e7c71fc: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
Hi! On Tue, Dec 25, 2018 at 1:27 PM Sergei Golubchik wrote: > > Hi, Aleksey! > > On Dec 23, Aleksey Midenkov wrote: > > revision-id: 7198e7c71fc (versioning-1.0.6-82-g7198e7c71fc) > > parent(s): 6be155757b7 > > author: Aleksey Midenkov > > committer: Aleksey Midenkov > > timestamp: 2018-12-20 13:25:00 +0300 > > message: > > > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > > * Versioning tests support > > As far as I can see, there are two approaches to fixing this bug. > And if we go the way of HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS then > your fix is good. > > But I'd still like to understand whether write_row is supposed to > disturb the scan. Why should heap_write() overwrite > info->current_ptr=pos and info->current_hash_ptr? Inserting a row > doesn't happen at the "current position", so I don't quite understand > why it modifies current_ptr at all. There is no functionality like > "continue the scan from the last inserted row", right? It just doesn't > make sense to me. I don't know why it works like this, maybe to keep consistency at API level: we insert record and then we able to access it without search. HEAP storage was copypasted from MyISAM where `lastpos` analogy also is updated at mi_write(). Without updating `current_ptr` and `lastpos` tests don't fail (main, funcs_1, heap). But I can't say whether the performance is affected and how it will go in production. > > Regards, > Sergei > Chief Architect MariaDB > and secur...@mariadb.org -- All the best, Aleksey Midenkov @midenok ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] 7198e7c71fc: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
Hi, Aleksey! On Dec 23, Aleksey Midenkov wrote: > revision-id: 7198e7c71fc (versioning-1.0.6-82-g7198e7c71fc) > parent(s): 6be155757b7 > author: Aleksey Midenkov > committer: Aleksey Midenkov > timestamp: 2018-12-20 13:25:00 +0300 > message: > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > * Versioning tests support As far as I can see, there are two approaches to fixing this bug. And if we go the way of HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS then your fix is good. But I'd still like to understand whether write_row is supposed to disturb the scan. Why should heap_write() overwrite info->current_ptr=pos and info->current_hash_ptr? Inserting a row doesn't happen at the "current position", so I don't quite understand why it modifies current_ptr at all. There is no functionality like "continue the scan from the last inserted row", right? It just doesn't make sense to me. Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
