Bug #116792 Contribution by Tencent: rename an incorrect space in post ddl
Submitted: 27 Nov 2024 3:44 Modified: 27 Nov 2024 8:59
Reporter: Xiaodong Huang (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: DDL Severity:S3 (Non-critical)
Version:8.0.40 OS:Any
Assigned to: CPU Architecture:Any

[27 Nov 2024 3:44] Xiaodong Huang
Description:
The issue here is similar to the one mentioned in https://bugs.mysql.com/bug.php?id=107959. However, this problem occurs not only with `algorithm=copy` DDL but also with `algorithm=inplace` DDL. During execution, the following errors appear in the logs, and subsequent DML statements will cause a crash:

[Note] [MY-012233] [InnoDB] VALID: space:4 page_no:0 page_size:16384
[Note] [MY-012233] [InnoDB] VALID: space:4 page_no:0 page_size:16384
[Note] [MY-012233] [InnoDB] VALID: space:4 page_no:0 page_size:16384
[Note] [MY-012233] [InnoDB] VALID: space:4 page_no:0 page_size:16384
[Note] [MY-012233] [InnoDB] VALID: space:4 page_no:0 page_size:16384
[Note] [MY-012234] [InnoDB] Page size: 16384. Possible space_id count:1
[Note] [MY-012235] [InnoDB] space_id:4, Number of pages matched: 5/5 (16384)
[Note] [MY-012236] [InnoDB] Chosen space:4
Datafile::restore_from_doublewrite:  
[ERROR] [MY-012237] [InnoDB] Corrupted page [page id: space=4, page number=0] of datafile './test/#sql-3ee1_e.ibd' could not be found in the doublewrite buffer.  
[Warning] [MY-012192] [InnoDB] Failed to read the first page of the file './test/#sql-3ee1_e.ibd'. You will need to verify and move the file out of the way retry recovery.  
[Note] [MY-012481] [InnoDB] DDL log replay : RENAME from ./test/t1.ibd to ./test/#sql-3ee1_e.ibd failed  

After restarting, the table data will be lost, and the table data will also be unavailable.

How to repeat:
Add the following patch to help with reproduction this problem:

diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 6adba7d15f0..83ce9f47f87 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -13630,6 +13630,11 @@ static bool mysql_inplace_alter_table(
     altered_table_def->set_name(alter_ctx->alias);
     altered_table_def->set_hidden(dd::Abstract_table::HT_VISIBLE);
 
+    DBUG_EXECUTE_IF("partition_ddl_rollback", {
+      my_error(ER_SECONDARY_ENGINE_DDL, MYF(0));
+      goto cleanup2;
+    });
+
     /*
       Copy pre-existing triggers to the new table definition.
       Since trigger names have to be unique per schema, we cannot
       
diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc
index d8c8f6c6c15..cc2634359f4 100644
--- a/storage/innobase/dict/dict0dict.cc
+++ b/storage/innobase/dict/dict0dict.cc
@@ -1329,7 +1329,8 @@ ulint dict_make_room_in_cache(

   i = len = UT_LIST_GET_LEN(dict_sys->table_LRU);

-  if (len < max_tables) {
+  if (len < max_tables &&
+      DBUG_EVALUATE_IF("ib_force_evict_target_table", false, true)) {
     return (0);
   }

@@ -1340,9 +1341,19 @@ ulint dict_make_room_in_cache(

   /* Find a suitable candidate to evict from the cache. Don't scan the
   entire LRU list. Only scan pct_check list entries. */
+  auto should_check_table = [&](const dict_table_t *table) -> bool {
+    if (DBUG_EVALUATE_IF("ib_force_evict_target_table", true, false)) {
+       if (!table) return false;
+       if (strcmp(table->name.m_name, "test/t1") == 0) {
+         ib::warn() << "force to evict table cache:test/t1";
+        return true;
+       }
+    }
+    return table && i > check_up_to && (len - n_evicted) > max_tables;
+  };

   for (table = UT_LIST_GET_LAST(dict_sys->table_LRU);
-       table != nullptr && i > check_up_to && (len - n_evicted) > max_tables;
+       should_check_table(table);
        --i) {
     dict_table_t *prev_table;

diff --git a/storage/innobase/log/log0ddl.cc b/storage/innobase/log/log0ddl.cc
index c5dd637c547..18cfe558db7 100644
--- a/storage/innobase/log/log0ddl.cc
+++ b/storage/innobase/log/log0ddl.cc
@@ -1822,6 +1822,8 @@ void Log_DDL::replay_rename_table_log(const char *old_name,
   trx->mysql_thd = current_thd;
   trx_start_if_not_started(trx, true, UT_LOCATION_HERE);

+  DEBUG_SYNC(trx->mysql_thd, "replay_rename_table_before_table_open");
+
   row_mysql_lock_data_dictionary(trx, UT_LOCATION_HERE);
   trx_set_dict_operation(trx, TRX_DICT_OP_TABLE);
       

MTR

--source include/have_debug.inc
connect (con1,localhost,root,,);

CREATE TABLE t1 (
    id INT,
    data INT,
    PRIMARY KEY(id),
    KEY idx1(data)
)
PARTITION BY LIST(id) (
    PARTITION p0 VALUES IN (5, 10, 15),
    PARTITION p1 VALUES IN (6, 12, 18)
);

set global debug = "+d,partition_ddl_rollback";
set debug_sync = "replay_rename_table_before_table_open wait_for go";

--connection con1
--send ALTER TABLE t1 ADD new_col_0902105001505 int default 10 FIRST
--sleep 3

--connection default
set global debug = "+d,ib_force_evict_target_table";
--sleep 5
set debug_sync = "now signal go";

--connection con1
--error ER_SECONDARY_ENGINE_DDL
--reap

set global debug = "-d,partition_ddl_rollback";
set global debug = "-d,ib_force_evict_target_table";
select * from test.t1;
drop table test.t1;

Suggested fix:
Root Cause:

Here, take the following DDL as an example:

CREATE TABLE t1 (
    id INT,
    data INT,
    PRIMARY KEY(id),
    KEY idx1(data)
)
PARTITION BY LIST(id) (
    PARTITION p0 VALUES IN (5, 10, 15),
    PARTITION p1 VALUES IN (6, 12, 18)
);

ALTER TABLE t1 ADD new_col_0902105001505 int default 10 FIRST;

The following process will occur:

// Online DDL prepar phase
//table space id: 7
create temp_table1#p1  // new table

// Online DDL execute phase
Fill data into the new table temp_table1#p1

// Online DDL commit phase
//t1#p1's tablespace id: 5, each rename will have two DDL logs: rename space and rename table
rename t1#p1  to temp_table2#p1 
rename temp_table1#p1  to t1#p1

delete temp_table2#p1

If a rollback occurs, the currently uncommitted metadata will be cleared. In the post DDL phase, the replay of the delete temp_table2#p1 DDL log will be ignored first, and the replay of the rename will be performed directly.

// post ddl
rename table t1#p1  to  temp_table1#p1
rename space t1#p1  to  temp_table1#p1 (table space id: 7)

rename table temp_table2#p1  to t1#p1 
rename space temp_table2#p1  to t1#p1  (table space id: 5)

delete space temp_table1#p1 

During the process of `rename t1#p1 to temp_table1#p1`, if there is no table cache for `t1#p1`, the committed metadata will be used. At this point, table->id (se_private_id in the metadata) will be used to search, and there are two possibilities (refer to: `row_rename_table_for_mysql/dd_table_open_on_name`):

1. If a crash occurs before `delete temp_table2#p1`, the table cache for `temp_table2#p1` will be found directly in the current dict_sys cache because its table->id is 5;
2. If a crash occurs after `delete temp_table2#p1`, the old metadata will be used to form the table cache for the old version of `t1#p1`;

In the subsequent row_rename_table_for_mysql/dict_table_rename_in_cache phase, during the rename space operation, regardless of the above situations, `temp_table2#p1` will be renamed to `temp_table1#p1` in two steps (refer to: `row_rename_table_for_mysql/dict_table_rename_in_cache`):

1. First, in `char *old_path = fil_space_get_first_path(table->space);`, the old_path obtained will always be `temp_table2#p1` because the table->space obtained is always 5, not 7;
2. Then, in `fil_rename_tablespace`, the operation `rename space temp_table2#p1 to temp_table1#p1` is completed;

In the subsequent rename space phase, it will first check if `temp_table1#p1` exists and confirm whether its tablespace is 7. Here, the space is 5, resulting in an inconsistency, and the above log will be printed.
[27 Nov 2024 8:59] MySQL Verification Team
Hello Xiaodong,

Thank you for the report and feedback.

regards,
Umesh