maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sergey!

On Dec 15, Sergey Vojtovich wrote:
> > > +  /*
> > >      An upgradable shared metadata lock which blocks all attempts to update
> > >      table data, allowing reads.
> > >      A connection holding this kind of lock can read table metadata and read
> > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> > > index 1ca7e9c..d3f4517 100644
> > > --- a/sql/sql_base.cc
> > > +++ b/sql/sql_base.cc
> > > @@ -1496,6 +1496,7 @@ bool open_table(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx)
> > >      to general/slow log would be disabled in read only transactions.
> > >    */
> > >    if (table_list->mdl_request.type >= MDL_SHARED_WRITE &&
> > > +      table_list->mdl_request.type != MDL_SHARED_READ_ONLY &&
> > 
> > if you're going to keep read and write locks mixed,
> > then I'd suggest to move this condition into a small inline helper,
> > like table_list->mdl_request.is_write_lock().  it's used few times
> > already in this very file and there will be only one place to change
> > if we'll need to add, say, MDL_SHARED_WRITE_LOW_PRIO
> I can merge this from MySQL, it is called is_write_lock_request() there. In fact
> MySQL didn't have to change this condition for MDL_SHARED_WRITE_LOW_PRIO.

I think it'd be good. There were three or four places in sql_base.cc
where it should replace the >= check.

> > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> > > index 6dc9192..0d740bb 100644
> > > --- a/sql/sql_parse.cc
> > > +++ b/sql/sql_parse.cc
...
> > > +        thd->pop_internal_handler();
> > > +
> > > +        if (deadlock_handler.need_reopen())
> > > +        {
> > > +          /*
> > > +            Deadlock occurred during upgrade of metadata lock.
> > > +            Let us restart acquring and opening tables for LOCK TABLES.
> > 
> > will you use MDL_SHARED_READ_ONLY right away when retrying?
> > or it'll do the upgrade thing again?
> Upgrade thing again, which is farily stupid. That's how it was in original code
> and I didn't change it in a hurry.

Can you do it?  It's probably just updating the lock in the
corresponding table_list entry.

Still, better do it as a separate change, after you push this MDEV-11227
commit, so that 10.2.3 wouldn't have to wait for it.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx