← Back to team overview

maria-developers team mailing list archive

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