Re: [Maria-developers] 6b06a99e4a4: Avoid creating the .frm file twice in some cases
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@gmail.com> committer: Michael Widenius <michael.widenius@gmail.com> 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? Why ha_create_table() is invoked twice at all?
share.frm_image= frm;
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"
if (thd->lock) { if (thd->locked_tables_mode != LTM_LOCK_TABLES &&
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi! On Mon, Apr 19, 2021 at 8:17 PM Sergei Golubchik <serg@mariadb.org> 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@gmail.com> committer: Michael Widenius <michael.widenius@gmail.com> 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
participants (2)
-
Michael Widenius -
Sergei Golubchik