ddl: Fix rename come after database has been dropped (#9274) (#9278)

close #9266

Signed-off-by: ti-chi-bot <[email protected]>
Signed-off-by: JaySon-Huang <[email protected]>

Co-authored-by: JaySon <[email protected]>
Co-authored-by: JaySon-Huang <[email protected]>

dbms/src/Debug/MockTiDB.cpp

@@ -717,17 +717,18 @@ TiDB::TableInfoPtr MockTiDB::getTableInfoByID(TableID table_id)
 
 TiDB::DBInfoPtr MockTiDB::getDBInfoByID(DatabaseID db_id)
 {
-    TiDB::DBInfoPtr db_ptr = std::make_shared<TiDB::DBInfo>(TiDB::DBInfo());
-    db_ptr->id = db_id;
     for (const auto & database : databases)
     {
         if (database.second == db_id)
         {
+            TiDB::DBInfoPtr db_ptr = std::make_shared<TiDB::DBInfo>(TiDB::DBInfo());
+            db_ptr->id = db_id;
             db_ptr->name = database.first;
-            break;
+            return db_ptr;
         }
     }
-    return db_ptr;
+    // If the database has been dropped in TiKV, TiFlash get a nullptr
+    return nullptr;
 }
 
 std::pair<bool, DatabaseID> MockTiDB::getDBIDByName(const String & database_name)

dbms/src/TiDB/Schema/SchemaBuilder.cpp

@@ -790,6 +790,11 @@ void SchemaBuilder<Getter, NameMapper>::applyRenamePhysicalTable(
         return;
     }
 
+    // There could be a chance that the target database has been dropped in TiKV before
+    // TiFlash accepts the "create database" schema diff. We need to ensure the local
+    // database exist before executing renaming.
+    ensureLocalDatabaseExist(new_db_info->id, new_mapped_db_name, action);
+
     const auto old_mapped_tbl_name = storage->getTableName();
     GET_METRIC(tiflash_schema_internal_ddl_count, type_rename_column).Increment();
     LOG_INFO(
@@ -960,7 +965,7 @@ bool SchemaBuilder<Getter, NameMapper>::applyCreateSchema(DatabaseID schema_id)
     {
         return false;
     }
-    applyCreateSchema(db);
+    applyCreateSchemaByInfo(db);
     return true;
 }
 
@@ -1006,10 +1011,10 @@ String createDatabaseStmt(Context & context, const DBInfo & db_info, const Schem
 }
 
 template <typename Getter, typename NameMapper>
-void SchemaBuilder<Getter, NameMapper>::applyCreateSchema(const TiDB::DBInfoPtr & db_info)
+void SchemaBuilder<Getter, NameMapper>::applyCreateSchemaByInfo(const TiDB::DBInfoPtr & db_info)
 {
     GET_METRIC(tiflash_schema_internal_ddl_count, type_create_db).Increment();
-    LOG_INFO(log, "Creating database {}", name_mapper.debugDatabaseName(*db_info));
+    LOG_INFO(log, "Create database begin {} database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id);
 
     auto statement = createDatabaseStmt(context, *db_info, name_mapper);
 
@@ -1021,7 +1026,36 @@ void SchemaBuilder<Getter, NameMapper>::applyCreateSchema(const TiDB::DBInfoPtr
     interpreter.execute();
 
     databases.emplace(KeyspaceDatabaseID{keyspace_id, db_info->id}, db_info);
-    LOG_INFO(log, "Created database {}", name_mapper.debugDatabaseName(*db_info));
+    LOG_INFO(log, "Create database end {} database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id);
+}
+
+template <typename Getter, typename NameMapper>
+void SchemaBuilder<Getter, NameMapper>::ensureLocalDatabaseExist(
+    DatabaseID database_id,
+    const String & database_mapped_name,
+    std::string_view action)
+{
+    if (likely(context.isDatabaseExist(database_mapped_name)))
+        return;
+
+    LOG_WARNING(
+        log,
+        "database instance is not exist, may has been dropped, create a database "
+        "with fake DatabaseInfo for it, database_id={} database_name={} action={}",
+        database_id,
+        database_mapped_name,
+        action);
+    // The database is dropped in TiKV and we can not fetch it. Generate a fake
+    // DatabaseInfo for it. It is OK because the DatabaseInfo will be updated
+    // when the database is `FLASHBACK`.
+    TiDB::DBInfoPtr database_info = std::make_shared<TiDB::DBInfo>();
+    database_info->id = database_id;
+    database_info->keyspace_id = keyspace_id;
+    database_info->name = database_mapped_name; // use the mapped name because we done known the actual name
+    database_info->charset = "utf8mb4"; // default value
+    database_info->collate = "utf8mb4_bin"; // default value
+    database_info->state = TiDB::StateNone; // special state
+    applyCreateSchemaByInfo(database_info);
 }
 
 template <typename Getter, typename NameMapper>
@@ -1033,7 +1067,7 @@ void SchemaBuilder<Getter, NameMapper>::applyDropSchema(DatabaseID schema_id)
     {
         LOG_INFO(
             log,
-            "Syncer wants to drop database [id={}], but database is not found, may has been dropped.",
+            "Syncer wants to drop database, but database is not found, may has been dropped, database_id={}",
             schema_id);
         return;
     }
@@ -1241,13 +1275,19 @@ void SchemaBuilder<Getter, NameMapper>::applyCreatePhysicalTable(const DBInfoPtr
 
     LOG_INFO(log, "Creating table {} with statement: {}", name_mapper.debugCanonicalName(*db_info, *table_info), stmt);
 
+    // If "CREATE DATABASE" is executed in TiFlash after user has executed "DROP DATABASE"
+    // in TiDB, then TiFlash may not create the IDatabase instance. Make sure we can access
+    // to the IDatabase when creating IStorage.
+    const auto database_mapped_name = name_mapper.mapDatabaseName(*db_info);
+    ensureLocalDatabaseExist(db_info->id, database_mapped_name, fmt::format("applyCreatePhysicalTable-table_id={}", table_info->id));
+
     ParserCreateQuery parser;
     ASTPtr ast = parseQuery(parser, stmt.data(), stmt.data() + stmt.size(), "from syncSchema " + table_info->name, 0);
 
     auto * ast_create_query = typeid_cast<ASTCreateQuery *>(ast.get());
     ast_create_query->attach = true;
     ast_create_query->if_not_exists = true;
-    ast_create_query->database = name_mapper.mapDatabaseName(*db_info);
+    ast_create_query->database = database_mapped_name;
 
     InterpreterCreateQuery interpreter(ast, context);
     interpreter.setInternal(true);
@@ -1440,7 +1480,7 @@ void SchemaBuilder<Getter, NameMapper>::syncAllSchema()
         db_set.emplace(name_mapper.mapDatabaseName(*db));
         if (databases.find(KeyspaceDatabaseID{keyspace_id, db->id}) == databases.end())
         {
-            applyCreateSchema(db);
+            applyCreateSchemaByInfo(db);
             LOG_INFO(log, "Database {} created during sync all schemas", name_mapper.debugDatabaseName(*db));
         }
     }
@@ -1581,5 +1621,4 @@ template struct SchemaBuilder<MockSchemaGetter, MockSchemaNameMapper>;
 // unit test
 template struct SchemaBuilder<MockSchemaGetter, SchemaNameMapper>;
 
-// end namespace
 } // namespace DB

dbms/src/TiDB/Schema/SchemaBuilder.h

@@ -63,7 +63,8 @@ struct SchemaBuilder
 
     bool applyCreateSchema(DatabaseID schema_id);
 
-    void applyCreateSchema(const TiDB::DBInfoPtr & db_info);
+    void applyCreateSchemaByInfo(const TiDB::DBInfoPtr & db_info);
+    void ensureLocalDatabaseExist(DatabaseID database_id, const String & database_mapped_name, std::string_view action);
 
     void applyCreateTable(const TiDB::DBInfoPtr & db_info, TableID table_id);
 

tests/_env.sh

@@ -30,7 +30,7 @@ if [ -z ${storage_bin+x} ]; then
 fi
 
 # Server address for connecting
-export storage_server="127.0.0.1"
+export storage_server=${storage_server:-"127.0.0.1"}
 
 # Server port for connecting
 export storage_port=${storage_port:-9000}
@@ -39,13 +39,13 @@ export storage_port=${storage_port:-9000}
 export storage_db="system"
 
 # TiDB address
-export tidb_server="127.0.0.1"
+export tidb_server=${tidb_server:-"127.0.0.1"}
 
 # TiDB port
-export tidb_port="${tidb_port:-4000}"
+export tidb_port=${tidb_port:-"4000"}
 
 # TiDB status port
-export tidb_status_port="10080"
+export tidb_status_port=${tidb_status_port:-"10080"}
 
 # TiDB default database
 export tidb_db="test"
@@ -54,11 +54,8 @@ export tidb_db="test"
 export tidb_table="t"
 
 # Whether run scripts with verbose output
-export verbose="${verbose:-"false"}"
-
-# Setup running env vars
-#source ../../_vars.sh
-#setup_dylib_path
+# "true" or "false"
+export verbose=${verbose:-"false"}
 
 export LANG=en_US.utf-8
 export LC_ALL=en_US.utf-8

tests/delta-merge-test/query/misc/timestamp_rough_set_filter.test

@@ -18,14 +18,11 @@
 => DBGInvoke __enable_schema_sync_service('true')
 
 => DBGInvoke __drop_tidb_table(default, test)
-=> DBGInvoke __drop_tidb_db(default)
 => drop table if exists default.test
-=> drop database if exists default
 
 => DBGInvoke __set_flush_threshold(1000000, 1000000)
 
 # Data.
-=> DBGInvoke __mock_tidb_db(default)
 => DBGInvoke __mock_tidb_table(default, test, 'col_1 Int64, col_2 default \'asTiDBType|timestamp(5)\'')
 => DBGInvoke __refresh_schemas()
 => DBGInvoke __put_region(4, 0, 100, default, test)

tests/fullstack-test2/ddl/rename_table_across_databases.test

@@ -16,22 +16,19 @@ mysql> drop table if exists test.t;
 mysql> drop table if exists test_new.t2;
 mysql> drop database if exists test_new;
 
+# (case 1) rename table across database
+## prepare some data
 mysql> create table test.t(a int, b int)
 mysql> alter table test.t set tiflash replica 1 location labels 'rack', 'host', 'abc'
-
+mysql> insert into test.t values (1, 1),(1, 2);
 func> wait_table test t
-
-mysql> insert into test.t values (1, 1);
-mysql> insert into test.t values (1, 2);
-
 mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t;
 +------+------+
 | a    | b    |
 +------+------+
 |    1 |    1 |
 |    1 |    2 |
 +------+------+
-
 # check table info in tiflash
 >> select tidb_database,tidb_name from system.tables where tidb_database = 'test' and tidb_name='t' and is_tombstone = 0
 ┌─tidb_database─┬─tidb_name─┐
@@ -41,6 +38,8 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t;
 # rename table across databases
 mysql> create database if not exists test_new;
 mysql> rename table test.t to test_new.t2;
+=> DBGInvoke __refresh_schemas() 
+
 mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t;
 ERROR 1146 (42S02) at line 1: Table 'test.t' doesn't exist
 mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new.t2;
@@ -61,3 +60,91 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new
 mysql> drop table if exists test.t;
 mysql> drop table if exists test_new.t2;
 mysql> drop database if exists test_new;
+
+# (case 2) rename table across database
+mysql> create database if not exists test
+mysql> create database if not exists test_new
+## (required) stop regular schema sync
+=> DBGInvoke __enable_schema_sync_service('false')
+
+mysql> create table test.t(a int, b int);
+mysql> insert into test.t values (1, 1); insert into test.t values (1, 2);
+## (required) sync table id mapping to tiflash
+=> DBGInvoke __refresh_schemas()
+mysql> rename table test.t to test_new.t2;
+mysql> alter table test_new.t2 set tiflash replica 1;
+## new snapshot sync to tiflash, but the table id mapping is not updated
+func> wait_table test_new t2
+mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new.t2;
++------+------+
+| a    | b    |
++------+------+
+|    1 |    1 |
+|    1 |    2 |
++------+------+
+
+mysql> drop table if exists test.t;
+mysql> drop table if exists test_new.t2;
+mysql> drop database if exists test_new;
+
+## (required) create a new table and sync to tiflash, check whether it can apply
+mysql> drop table if exists test.t3;
+mysql> create table test.t3(c int, d int);
+mysql> insert into test.t3 values (3,3),(3,4);
+mysql> alter table test.t3 set tiflash replica 1;
+func> wait_table test t3
+mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t3;
++------+------+
+| c    | d    |
++------+------+
+|    3 |    3 |
+|    3 |    4 |
++------+------+
+
+mysql> drop table if exists test.t3;
+
+# (case 3) rename partitioned table across database
+mysql> create database if not exists test_new;
+mysql> drop table if exists test.part4;
+mysql> CREATE TABLE test.part4 (id INT NOT NULL,store_id INT NOT NULL)PARTITION BY RANGE (store_id) (PARTITION p0 VALUES LESS THAN (6),PARTITION p1 VALUES LESS THAN (11),PARTITION p2 VALUES LESS THAN (16),PARTITION p3 VALUES LESS THAN (21));
+# (1,1),(2,2),(3,3) => p0; p1 is empty;(11,11) => p2;(16,16) => p3
+mysql> insert into test.part4(id, store_id) values(1,1),(2,2),(3,3),(11,11),(16,16);
+mysql> alter table test.part4 set tiflash replica 1;
+func> wait_table test part4
+
+mysql> rename table test.part4 to test_new.part4;
+mysql> alter table test_new.part4 add column c1 int;
+mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new.part4 order by id;
++----+----------+------+
+| id | store_id | c1   |
++----+----------+------+
+|  1 |        1 | NULL |
+|  2 |        2 | NULL |
+|  3 |        3 | NULL |
+| 11 |       11 | NULL |
+| 16 |       16 | NULL |
++----+----------+------+
+
+mysql> drop table if exists test_new.part4
+
+# (case 4) rename partitioned table across database
+# (required) stop regular schema sync
+=> DBGInvoke __enable_schema_sync_service('false')
+mysql> drop database if exists test_new;
+mysql> drop table if exists test.part5;
+mysql> CREATE TABLE test.part5 (id INT NOT NULL,store_id INT NOT NULL)PARTITION BY RANGE (store_id) (PARTITION p0 VALUES LESS THAN (6),PARTITION p1 VALUES LESS THAN (11),PARTITION p2 VALUES LESS THAN (16),PARTITION p3 VALUES LESS THAN (21));
+# (1,1),(2,2),(3,3) => p0; p1 is empty;(11,11) => p2;(16,16) => p3
+mysql> alter table test.part5 set tiflash replica 1;
+func> wait_table test part5
+
+>> DBGInvoke __enable_fail_point(pause_before_apply_raft_cmd)
+mysql> insert into test.part5(id, store_id) values(1,1),(2,2),(3,3),(11,11),(16,16);
+
+# create target db, rename table to target db, then drop target db
+mysql> create database if not exists test_new;
+mysql> rename table test.part5 to test_new.part5;
+mysql> alter table test_new.part5 add column c1 int;
+mysql> drop database if exists test_new;
+# raft command comes after target db dropped, no crashes
+>> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd)
+>> DBGInvoke __refresh_schemas()