Re: [Maria-developers] 6b06a99e4a4: Avoid creating the .frm file twice in some cases

Hi! On Mon, Apr 19, 2021 at 8:17 PM Sergei Golubchik wrote:

Hi, Monty!

On Apr 19, Michael Widenius wrote:

...

revision-id: 6b06a99e4a4 (mariadb-10.5.2-557-g6b06a99e4a4) parent(s): 9c1e36696f3 author: Michael Widenius committer: Michael Widenius 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