maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #09526
Re: 745b522: MDEV-8889 - Assertion `next_insert_id == 0' failed in handler::ha_external_lock
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
References