Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions dbms/src/Debug/MockTiDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -877,17 +877,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
62 changes: 36 additions & 26 deletions dbms/src/TiDB/Schema/SchemaBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,14 @@
#include <IO/FileProvider/FileProvider.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 @@ -723,6 +721,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 @@ -1167,27 +1170,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 @@ -1203,13 +1186,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 @@ -1773,5 +1784,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)
JaySon-Huang marked this conversation as resolved.
Show resolved Hide resolved
>> DBGInvoke __refresh_schemas()