← Back to team overview

maria-developers team mailing list archive

Re: e9293e31ec6: MDEV-28930 ALTER TABLE Deadlocks with parallel TL_WRITE

 

Thanks! Updated here: 86ee039cdcff. Let's see, how it'll go on tests

On Wed, 29 Jun 2022 at 23:36, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Nikita,
>
> This is very good.
> Just one comment, see below:
>
> On Jun 29, Nikita Malyavin wrote:
> > revision-id: e9293e31ec6 (mariadb-10.6.1-499-ge9293e31ec6)
> > parent(s): 6cfcb36d027
> > author: Nikita Malyavin
> > committer: Nikita Malyavin
> > timestamp: 2022-06-29 20:16:19 +0300
> > message:
> >
> > MDEV-28930 ALTER TABLE Deadlocks with parallel TL_WRITE
> >
> > ALTER ONLINE TABLE acquires table with TL_READ. Myisam normally acquires
> > TL_WRITE for DML, which makes it hang until table is freed.
> >
> > We deadlock once ALTER upgrades its MDL lock.
> >
> > Solution:
> > Unlock table earlier. We don't need to hold TL_READ once we finished
> > copying. Relay log replication requires no data locks on `from` table.
> >
> > diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
> > index 425de469a7f..90f6283d9df 100644
> > --- a/sql/log_event_server.cc
> > +++ b/sql/log_event_server.cc
> > @@ -5734,7 +5734,7 @@ int Rows_log_event::do_apply_event(rpl_group_info
> *rgi)
> >      used in the transaction and proceed with execution of the actual
> >      event.
> >    */
> > -  if (!thd->lock)
> > +  if (!thd->lock && rgi->tables_to_lock_count)
> >    {
> >      /*
> >        Lock_tables() reads the contents of thd->lex, so they must be
>
> Instead of adding a second condition to the execution path for every row
> event in the replication, consider this:
>
> -  if (!thd->lock)
> +  if (!thd->open_tables)
>
> this will work identically for replication, I hope, but will be always
> false in your case, as you still have the table open. I mean, that's the
> idea. I run the rpl suite with it, but haven't done any further testing,
> so not completely sure it'll work.
>
> When making this change, don't forget to update the comment right above
> this if().
>
> Ok to push into bb-10.10-MDEV-16329 after that.
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx
>


-- 
Yours truly,
Nikita Malyavin

Follow ups

References