Skip to content

Commit

Permalink
ddl: Fix rename come after database has been dropped (release-7.5) (#…
Browse files Browse the repository at this point in the history
…9275)

close #9266
  • Loading branch information
JaySon-Huang authored Aug 1, 2024
1 parent 2eb2cfe commit 8ed9835
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 31 deletions.
9 changes: 5 additions & 4 deletions dbms/src/Debug/MockTiDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -875,17 +875,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)
Expand Down
63 changes: 36 additions & 27 deletions dbms/src/TiDB/Schema/SchemaBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,14 @@
#include <Debug/MockSchemaNameMapper.h>
#include <IO/WriteHelpers.h>
#include <Interpreters/Context.h>
#include <Interpreters/InterpreterAlterQuery.h>
#include <Interpreters/InterpreterCreateQuery.h>
#include <Interpreters/InterpreterDropQuery.h>
#include <Interpreters/InterpreterRenameQuery.h>
#include <Parsers/ASTCreateQuery.h>
#include <Parsers/ASTDropQuery.h>
#include <Parsers/ASTLiteral.h>
#include <Parsers/ASTRenameQuery.h>
#include <Parsers/ParserCreateQuery.h>
#include <Parsers/parseQuery.h>
#include <Storages/AlterCommands.h>
#include <Storages/IManageableStorage.h>
#include <Storages/KVStore/TMTContext.h>
#include <Storages/MutableSupport.h>
Expand Down Expand Up @@ -727,6 +725,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.
const auto action = fmt::format("applyRenamePhysicalTable-table_id={}", new_table_info.id);
ensureLocalDatabaseExist(new_database_id, new_mapped_db_name, action);
const auto old_mapped_tbl_name = storage->getTableName();
GET_METRIC(tiflash_schema_internal_ddl_count, type_rename_table).Increment();
LOG_INFO(
Expand Down Expand Up @@ -1171,28 +1174,7 @@ void SchemaBuilder<Getter, NameMapper>::applyCreateStorageInstance(
// 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(database_id, keyspace_id);
if (!context.isDatabaseExist(database_mapped_name))
{
LOG_WARNING(
log,
"database instance is not exist (applyCreateStorageInstance), 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
applyCreateDatabaseByInfo(database_info);
}
ensureLocalDatabaseExist(database_id, database_mapped_name, action);

ParserCreateQuery parser;
ASTPtr ast = parseQuery(parser, stmt.data(), stmt.data() + stmt.size(), "from syncSchema " + table_info->name, 0);
Expand All @@ -1208,13 +1190,41 @@ void SchemaBuilder<Getter, NameMapper>::applyCreateStorageInstance(
interpreter.execute();
LOG_INFO(
log,
"Creat table {} end, database_id={} table_id={} action={}",
"Create table {} end, database_id={} table_id={} action={}",
name_mapper.debugCanonicalName(*table_info, database_id, keyspace_id),
database_id,
table_info->id,
action);
}

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 don't known the actual name
database_info->charset = "utf8mb4"; // default value
database_info->collate = "utf8mb4_bin"; // default value
database_info->state = TiDB::StateNone; // special state
applyCreateDatabaseByInfo(database_info);
}

template <typename Getter, typename NameMapper>
void SchemaBuilder<Getter, NameMapper>::applyDropPhysicalTable(
Expand Down Expand Up @@ -1775,5 +1785,4 @@ template struct SchemaBuilder<SchemaGetter, SchemaNameMapper>;
template struct SchemaBuilder<MockSchemaGetter, MockSchemaNameMapper>;
// unit test
template struct SchemaBuilder<MockSchemaGetter, SchemaNameMapper>;
// end namespace
} // namespace DB
1 change: 1 addition & 0 deletions dbms/src/TiDB/Schema/SchemaBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ struct SchemaBuilder

bool applyCreateDatabase(DatabaseID database_id);
void applyCreateDatabaseByInfo(const TiDB::DBInfoPtr & db_info);
void ensureLocalDatabaseExist(DatabaseID database_id, const String & database_mapped_name, std::string_view action);

void applyRecoverDatabase(DatabaseID database_id);

Expand Down
22 changes: 22 additions & 0 deletions tests/fullstack-test2/ddl/rename_table_across_databases.test
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,25 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new
+----+----------+------+

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()

0 comments on commit 8ed9835

Please sign in to comment.