← Back to team overview

maria-developers team mailing list archive

Re: b95c5105e28: MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT

 

Hi, Aleksey!

On Dec 05, Aleksey Midenkov wrote:
> > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> > > index ac22a63bdba..459edb614f7 100644
> > > --- a/sql/sql_base.cc
> > > +++ b/sql/sql_base.cc
> > > @@ -4203,6 +4444,15 @@ bool open_tables(THD *thd, const DDL_options_st &options,
> > >      }
> > >
> > >    thd->current_tablenr= 0;
> > > +
> > > +#ifdef WITH_PARTITION_STORAGE_ENGINE
> > > +  if (!thd->stmt_arena->is_conventional())
> > > +  {
> > > +    for (tables= *start; tables; tables= tables->next_global)
> > > +      tables->vers_skip_create= false;
> > > +  }
> >
> > This looks a bit like an overkill, you do it for every prepare,
> > while auto-adding a partition is a rare event, as you said.
> >
> > May be you can do instead in TABLE::vers_switch_partition
> >
> >  uint *create_count= !thd->stmt_arena->is_conventional() &&
> >                      table_list->vers_skip_create ?
> 
> No, this is inside backoff action loop. That will interfere with the
> normal vers_skip_create algorithm (which also applies to PS, SP). The
> goal of the above code is to reset vers_skip_create from the previous
> statement execution.

In that case you can use an auto-resetting value, like,
if tables->vers_skip_create == thd->query_id it means, "skip".
That is vers_skip_create must be of query_id_t. You set it with
tables->vers_skip_create= thd->query_id;
And on the next statement it automatically expires.

This semantics is a bit more complex than boolean, so it'd need a
comment, like

  /*
      Protect single thread from repeating partition auto-create over
      multiple share instances (as the share is closed on backoff action).
      Skips auto-create only for one given query id.
  */
  query_id_t    vers_skip_create;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups

References