diff --git a/dbms/src/TiDB/Schema/DatabaseInfoCache.h b/dbms/src/TiDB/Schema/DatabaseInfoCache.h index 666b3dfe1c9..eaf56f72cb9 100644 --- a/dbms/src/TiDB/Schema/DatabaseInfoCache.h +++ b/dbms/src/TiDB/Schema/DatabaseInfoCache.h @@ -38,19 +38,6 @@ class DatabaseInfoCache return it->second; } - template - TiDB::DBInfoPtr getDBInfoByMappedName(const String & mapped_database_name) const - { - std::shared_lock lock(mtx_databases); - - auto it = std::find_if(databases.begin(), databases.end(), [&](const auto & pair) { - return NameMapper().mapDatabaseName(*pair.second) == mapped_database_name; - }); - if (it == databases.end()) - return nullptr; - return it->second; - } - void addDatabaseInfo(const TiDB::DBInfoPtr & db_info) { std::unique_lock lock(mtx_databases); diff --git a/dbms/src/TiDB/Schema/SchemaNameMapper.h b/dbms/src/TiDB/Schema/SchemaNameMapper.h index b578dea887c..52a6c39bf73 100644 --- a/dbms/src/TiDB/Schema/SchemaNameMapper.h +++ b/dbms/src/TiDB/Schema/SchemaNameMapper.h @@ -24,14 +24,14 @@ struct SchemaNameMapper { virtual ~SchemaNameMapper() = default; - static constexpr auto DATABASE_PREFIX = "db_"; - static constexpr auto TABLE_PREFIX = "t_"; + static constexpr std::string_view DATABASE_PREFIX = "db_"; + static constexpr std::string_view TABLE_PREFIX = "t_"; static constexpr std::string_view KEYSPACE_PREFIX = "ks_"; static KeyspaceID getMappedNameKeyspaceID(const String & name) { - auto keyspace_prefix_len = KEYSPACE_PREFIX.length(); + static constexpr auto keyspace_prefix_len = KEYSPACE_PREFIX.length(); auto pos = name.find(KEYSPACE_PREFIX); if (pos == String::npos) return NullspaceID; @@ -41,20 +41,39 @@ struct SchemaNameMapper return std::stoull(name.substr(keyspace_prefix_len, pos - keyspace_prefix_len)); } + static std::optional tryGetDatabaseID(const String & name) + { + auto pos = name.find(DATABASE_PREFIX); + if (pos == String::npos || name.length() <= pos + DATABASE_PREFIX.length()) + return std::nullopt; + try + { + return std::stoull(name.substr(pos + DATABASE_PREFIX.length())); + } + catch (std::invalid_argument & e) + { + return std::nullopt; + } + catch (std::out_of_range & e) + { + return std::nullopt; + } + } + static String map2Keyspace(KeyspaceID keyspace_id, const String & name) { - return keyspace_id == NullspaceID ? name : KEYSPACE_PREFIX.data() + std::to_string(keyspace_id) + "_" + name; + return keyspace_id == NullspaceID ? name : fmt::format("{}{}_{}", KEYSPACE_PREFIX, keyspace_id, name); } virtual String mapDatabaseName(DatabaseID db_id, KeyspaceID keyspace_id) const { - auto db_name = DATABASE_PREFIX + std::to_string(db_id); + auto db_name = fmt::format("{}{}", DATABASE_PREFIX, db_id); return map2Keyspace(keyspace_id, db_name); } virtual String mapDatabaseName(const TiDB::DBInfo & db_info) const { - auto db_name = DATABASE_PREFIX + std::to_string(db_info.id); + auto db_name = fmt::format("{}{}", DATABASE_PREFIX, db_info.id); return map2Keyspace(db_info.keyspace_id, db_name); } @@ -64,7 +83,7 @@ struct SchemaNameMapper } virtual String mapTableName(const TiDB::TableInfo & table_info) const { - auto table_name = TABLE_PREFIX + std::to_string(table_info.id); + auto table_name = fmt::format("{}{}", TABLE_PREFIX, table_info.id); return map2Keyspace(table_info.keyspace_id, table_name); } virtual String displayTableName(const TiDB::TableInfo & table_info) const @@ -90,7 +109,7 @@ struct SchemaNameMapper virtual String debugCanonicalName(const TiDB::TableInfo & table_info, DatabaseID db_id, KeyspaceID keyspace_id) const { - auto db_name = DATABASE_PREFIX + std::to_string(db_id); + auto db_name = fmt::format("{}{}", DATABASE_PREFIX, db_id); return map2Keyspace(keyspace_id, db_name) + "." + debugTableName(table_info); } }; diff --git a/dbms/src/TiDB/Schema/SchemaSyncService.cpp b/dbms/src/TiDB/Schema/SchemaSyncService.cpp index 247f1a4a196..b67180aa9f1 100644 --- a/dbms/src/TiDB/Schema/SchemaSyncService.cpp +++ b/dbms/src/TiDB/Schema/SchemaSyncService.cpp @@ -255,6 +255,8 @@ bool SchemaSyncService::gc(Timestamp gc_safepoint, KeyspaceID keyspace_id) } } + auto schema_sync_manager = tmt_context.getSchemaSyncerManager(); + // Physically drop tables bool succeeded = true; for (auto & storage_ptr : storages_to_gc) @@ -268,21 +270,18 @@ bool SchemaSyncService::gc(Timestamp gc_safepoint, KeyspaceID keyspace_id) String table_name = storage->getTableName(); const auto & table_info = storage->getTableInfo(); - tmt_context.getSchemaSyncerManager()->removeTableID(keyspace_id, table_info.id); - auto canonical_name = [&]() { - // DB info maintenance is parallel with GC logic so we can't always assume one specific DB info's existence, thus checking its validity. - auto db_info = tmt_context.getSchemaSyncerManager()->getDBInfoByMappedName(keyspace_id, database_name); - return db_info ? fmt::format( - "{}, database_id={} table_id={}", - SchemaNameMapper().debugCanonicalName(*db_info, table_info), - db_info->id, - table_info.id) - : fmt::format( - "({}).{}, table_id={}", - database_name, - SchemaNameMapper().debugTableName(table_info), - table_info.id); + auto database_id = SchemaNameMapper::tryGetDatabaseID(database_name); + if (!database_id.has_value()) + { + return fmt::format("{}.{} table_id={}", database_name, table_name, table_info.id); + } + return fmt::format( + "{}.{} database_id={} table_id={}", + database_name, + table_name, + *database_id, + table_info.id); }(); LOG_INFO( keyspace_log, @@ -301,6 +300,8 @@ bool SchemaSyncService::gc(Timestamp gc_safepoint, KeyspaceID keyspace_id) InterpreterDropQuery drop_interpreter(ast_drop_query, context); drop_interpreter.execute(); LOG_INFO(keyspace_log, "Physically dropped table {}", canonical_name); + // remove the id mapping after physically dropped + schema_sync_manager->removeTableID(keyspace_id, table_info.id); ++num_tables_removed; } catch (DB::Exception & e) diff --git a/dbms/src/TiDB/Schema/SchemaSyncer.h b/dbms/src/TiDB/Schema/SchemaSyncer.h index 9ef7fb49001..5eccfe7428d 100644 --- a/dbms/src/TiDB/Schema/SchemaSyncer.h +++ b/dbms/src/TiDB/Schema/SchemaSyncer.h @@ -52,8 +52,6 @@ class SchemaSyncer virtual TiDB::DBInfoPtr getDBInfoByName(const String & database_name) = 0; - virtual TiDB::DBInfoPtr getDBInfoByMappedName(const String & mapped_database_name) = 0; - virtual void removeTableID(TableID table_id) = 0; virtual void dropAllSchema(Context & context) = 0; diff --git a/dbms/src/TiDB/Schema/TiDBSchemaManager.h b/dbms/src/TiDB/Schema/TiDBSchemaManager.h index 931527e1944..8e9d4289936 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaManager.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaManager.h @@ -90,18 +90,6 @@ class TiDBSchemaSyncerManager return schema_syncer->getDBInfoByName(database_name); } - TiDB::DBInfoPtr getDBInfoByMappedName(KeyspaceID keyspace_id, const String & mapped_database_name) - { - std::shared_lock read_lock(schema_syncers_mutex); - auto schema_syncer = getSchemaSyncer(keyspace_id); - if (schema_syncer == nullptr) - { - LOG_ERROR(log, "SchemaSyncer not found, keyspace={}", keyspace_id); - return nullptr; - } - return schema_syncer->getDBInfoByMappedName(mapped_database_name); - } - bool removeSchemaSyncer(KeyspaceID keyspace_id) { std::unique_lock lock(schema_syncers_mutex); diff --git a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h index 8e928b7cb60..1f54123d4cd 100644 --- a/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h +++ b/dbms/src/TiDB/Schema/TiDBSchemaSyncer.h @@ -116,11 +116,6 @@ class TiDBSchemaSyncer : public SchemaSyncer return databases.getDBInfoByName(database_name); } - TiDB::DBInfoPtr getDBInfoByMappedName(const String & mapped_database_name) override - { - return databases.getDBInfoByMappedName(mapped_database_name); - } - // clear all states. // just for testing restart void reset() override