maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Mon, Apr 19, 2021 at 8:17 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Monty!
>
> On Apr 19, Michael Widenius wrote:
> > revision-id: 6b06a99e4a4 (mariadb-10.5.2-557-g6b06a99e4a4)
> > parent(s): 9c1e36696f3
> > author: Michael Widenius <michael.widenius@xxxxxxxxx>
> > committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> > timestamp: 2021-03-25 12:06:34 +0200
> > message:
> >
> > Avoid creating the .frm file twice in some cases
> >
> > diff --git a/sql/handler.cc b/sql/handler.cc
> > index c286aef3a9e..fe2fe6e9426 100644
> > --- a/sql/handler.cc
> > +++ b/sql/handler.cc
> > @@ -5536,8 +5536,9 @@ int ha_create_table(THD *thd, const char *path,
> >
> >    if (frm)
> >    {
> > -    bool write_frm_now= !create_info->db_type->discover_table &&
> > -                        !create_info->tmp_table();
> > +    bool write_frm_now= (!create_info->db_type->discover_table &&
> > +                         !create_info->tmp_table() &&
> > +                         !create_info->frm_is_created);
>
> Is this the real problem? frm created twice?

Yes, this happens in case of discovery as part of alter table.
 I found this out while debugging ddl log crashing.
> Why ha_create_table() is invoked twice at all?

As far as I remember, once to discover (and create the frm) and once for
creating the internal table structures or something like that.

The code comment gives a bit more information:
    /*
      Avoid creating frm again in ha_create_table() if inplace alter will not
      be used.
    */
    create_info->frm_is_created= 1;

> > diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> > index e00d3ee1ed2..73642a5bd97 100644
> > --- a/sql/sql_table.cc
> > +++ b/sql/sql_table.cc
> > @@ -547,8 +547,13 @@ uint build_table_filename(char *buff, size_t bufflen, const char *db,
> >
> >    (void) tablename_to_filename(db, dbbuff, sizeof(dbbuff));
> >
> > -  /* Check if this is a temporary table name. Allow it if a corresponding .frm file exists */
> > -  if (is_prefix(table_name, tmp_file_prefix) && strlen(table_name) < NAME_CHAR_LEN &&
> > +  /*
> > +    Check if this is a temporary table name. Allow it if a corresponding .frm
> > +    file exists.
> > +  */
> > +  if (!(flags & FN_IS_TMP) &&
> > +      is_prefix(table_name, tmp_file_prefix) &&
> > +      strlen(table_name) < NAME_CHAR_LEN &&
>
> good idea!
>
> >        check_if_frm_exists(tbbuff, dbbuff, table_name))
> >      flags|= FN_IS_TMP;
> >
> > @@ -9817,7 +9827,7 @@ do_continue:;
> >
> >    if (table->s->tmp_table != NO_TMP_TABLE)
> >    {
> > -    /* Close lock if this is a transactional table */
> > +    /* Unlock lock if this is a transactional temporary table */
>
> strictly speaking, you don't "unlock lock",
> you "unlock a resource" (e.g. a table)
> or you "release a lock"

One a door, you unlock a lock.
Changed comment to release lock.

Regards,
Monty