Re: [Maria-developers] 745b522: MDEV-8889 - Assertion `next_insert_id == 0' failed in handler::ha_external_lock
Hi, Sergey! I believe this fix is ok... On Apr 21, Sergey Vojtovich wrote:
revision-id: 745b5226e68d376eed709d930f72c3fbc7f07d2a (mariadb-10.0.24-26-g745b522) parent(s): 072ca71d26487817d025ee97955e6360c3c5c086 committer: Sergey Vojtovich timestamp: 2016-04-21 16:51:00 +0400 message:
MDEV-8889 - Assertion `next_insert_id == 0' failed in handler::ha_external_lock
There was a race condition between delayed insert thread and connection thread actually performing INSERT/REPLACE DELAYED. It was triggered by concurrent INSERT/REPLACE DELAYED and statements that flush the same table either explicitely or implicitely (like FLUSH TABLE, ALTER TABLE, ...).
This race condition was caused by a gap in delayed thread shutdown logic, which allowed concurrent connection running INSERT/REPLACE DELAYED to change essential data consequently leaving table in semi-consistent state.
Specifically query thread could decrease "tables_in_use" reference counter in this gap, causing delayed insert thread to shutdown without releasing auto increment and table lock.
Fixed by extending condition so that delayed insert thread won't shutdown until there're locked tables.
s/until/if/
Also removed volatile qualifier from tables_in_use and stacked_inserts since they're supposed to be protected by mutexes.
It looks like stacked_inserts is accessed without a mutex at the end of Delayed_insert::handle_inserts(). Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi Sergei, Thanks for you review! On Fri, Apr 22, 2016 at 06:07:19PM +0200, Sergei Golubchik wrote:
Hi, Sergey!
I believe this fix is ok...
On Apr 21, Sergey Vojtovich wrote:
revision-id: 745b5226e68d376eed709d930f72c3fbc7f07d2a (mariadb-10.0.24-26-g745b522) parent(s): 072ca71d26487817d025ee97955e6360c3c5c086 committer: Sergey Vojtovich timestamp: 2016-04-21 16:51:00 +0400 message:
MDEV-8889 - Assertion `next_insert_id == 0' failed in handler::ha_external_lock
There was a race condition between delayed insert thread and connection thread actually performing INSERT/REPLACE DELAYED. It was triggered by concurrent INSERT/REPLACE DELAYED and statements that flush the same table either explicitely or implicitely (like FLUSH TABLE, ALTER TABLE, ...).
This race condition was caused by a gap in delayed thread shutdown logic, which allowed concurrent connection running INSERT/REPLACE DELAYED to change essential data consequently leaving table in semi-consistent state.
Specifically query thread could decrease "tables_in_use" reference counter in this gap, causing delayed insert thread to shutdown without releasing auto increment and table lock.
Fixed by extending condition so that delayed insert thread won't shutdown until there're locked tables.
s/until/if/ I believe "until" is fine here, because it describes things that happen inside a loop.
Also removed volatile qualifier from tables_in_use and stacked_inserts since they're supposed to be protected by mutexes.
It looks like stacked_inserts is accessed without a mutex at the end of Delayed_insert::handle_inserts().
That's why I was careful enough to write "supposed to be protected". :) Along with "stacked_inserts" it updates shared "rows" list without protection. To me it looks like a sure sign of another, though I didn't come up with a test case yet. Thanks, Sergey
participants (2)
-
Sergei Golubchik -
Sergey Vojtovich