From 6999a2f2a20ff0b5d6e648fdf1c83c090a0834f2 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Wed, 8 Nov 2023 18:26:37 +0800 Subject: [PATCH 1/3] Use tidb_isolation_read_engine instead of hint --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 129 +++++++++++++----- dbms/src/TiDB/Schema/SchemaBuilder.h | 3 +- dbms/src/TiDB/Schema/SchemaGetter.cpp | 24 ++-- dbms/src/TiDB/Schema/TiDB.cpp | 10 +- .../ddl/alter_partition_by.test | 40 +++--- .../ddl/remove_partitioning.test | 34 ++++- .../ddl/reorganize_partition.test | 24 ++-- 7 files changed, 178 insertions(+), 86 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index db65b9b8416..a61392209d9 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -252,9 +252,14 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) } else { - /// The new non-partitioned table will have a new id + // Create the new table. + // If the new table is a partition table, this will also overwrite + // the partition id mapping to the new logical table and renew the + // partition info. + applyPartitionAlter(diff.schema_id, diff.table_id); + // Drop the old table. if the previous partitions of the old table are + // not mapping to the old logical table now, they will not be removed. applyDropTable(diff.schema_id, diff.old_table_id); - applyCreateTable(diff.schema_id, diff.table_id); } break; } @@ -373,6 +378,51 @@ void SchemaBuilder::applySetTiFlashReplica(DatabaseID databa } } +template +void SchemaBuilder::applyPartitionAlter(DatabaseID database_id, TableID table_id) +{ + auto table_info = getter.getTableInfo(database_id, table_id); + if (table_info == nullptr) // the database maybe dropped + { + LOG_DEBUG(log, "table is not exist in TiKV, may have been dropped, table_id={}", table_id); + return; + } + + table_id_map.emplaceTableID(table_id, database_id); + LOG_DEBUG(log, "register table to table_id_map, database_id={} table_id={}", database_id, table_id); + + if (!table_info->isLogicalPartitionTable()) + { + return; + } + + // If table is partition table, we will create the logical table here. + // Because we get the table_info, so we can ensure new_db_info will not be nullptr. + auto new_db_info = getter.getDatabase(database_id); + applyCreatePhysicalTable(new_db_info, table_info); + + for (const auto & part_def : table_info->partition.definitions) + { + LOG_DEBUG( + log, + "register table to table_id_map for partition table, logical_table_id={} physical_table_id={}", + table_id, + part_def.id); + table_id_map.emplacePartitionTableID(part_def.id, table_id); + } + + auto & tmt_context = context.getTMTContext(); + auto storage = tmt_context.getStorages().get(keyspace_id, table_info->id); + if (storage == nullptr) + { + LOG_ERROR(log, "table is not exist in TiFlash, table_id={}", table_id); + return; + } + + // Try to renew the partition info for the new table + applyPartitionDiff(new_db_info, table_info, storage); +} + template void SchemaBuilder::applyPartitionDiff(DatabaseID database_id, TableID table_id) { @@ -410,70 +460,70 @@ void SchemaBuilder::applyPartitionDiff( const TableInfoPtr & table_info, const ManageableStoragePtr & storage) { - const auto & orig_table_info = storage->getTableInfo(); - if (!orig_table_info.isLogicalPartitionTable()) + const auto & local_table_info = storage->getTableInfo(); + // ALTER TABLE t PARTITION BY ... may turn a non-partition table into partition table + // with some partition ids in `partition.adding_definitions`/`partition.definitions` + // and `partition.dropping_definitions`. We need to create those partitions. + if (!local_table_info.isLogicalPartitionTable()) { - LOG_ERROR( + LOG_INFO( log, - "old table in TiFlash not partition table {} with database_id={}, table_id={}", - name_mapper.debugCanonicalName(*db_info, orig_table_info), + "Altering non-partition table to be a partition table {} with database_id={}, table_id={}", + name_mapper.debugCanonicalName(*db_info, local_table_info), db_info->id, - orig_table_info.id); - return; + local_table_info.id); } - const auto & orig_defs = orig_table_info.partition.definitions; + const auto & local_defs = local_table_info.partition.definitions; const auto & new_defs = table_info->partition.definitions; - std::unordered_set orig_part_id_set, new_part_id_set; - std::vector orig_part_ids, new_part_ids; - std::for_each(orig_defs.begin(), orig_defs.end(), [&orig_part_id_set, &orig_part_ids](const auto & def) { - orig_part_id_set.emplace(def.id); - orig_part_ids.emplace_back(std::to_string(def.id)); + std::unordered_set local_part_id_set, new_part_id_set; + std::for_each(local_defs.begin(), local_defs.end(), [&local_part_id_set](const auto & def) { + local_part_id_set.emplace(def.id); }); - std::for_each(new_defs.begin(), new_defs.end(), [&new_part_id_set, &new_part_ids](const auto & def) { + std::for_each(new_defs.begin(), new_defs.end(), [&new_part_id_set](const auto & def) { new_part_id_set.emplace(def.id); - new_part_ids.emplace_back(std::to_string(def.id)); }); - auto orig_part_ids_str = boost::algorithm::join(orig_part_ids, ", "); - auto new_part_ids_str = boost::algorithm::join(new_part_ids, ", "); - LOG_INFO( log, "Applying partition changes {} with database_id={}, table_id={}, old: {}, new: {}", name_mapper.debugCanonicalName(*db_info, *table_info), db_info->id, table_info->id, - orig_part_ids_str, - new_part_ids_str); + local_part_id_set, + new_part_id_set); - if (orig_part_id_set == new_part_id_set) + if (local_part_id_set == new_part_id_set) { LOG_INFO( log, - "No partition changes {} with database_id={}, table_id={}", + "No partition changes, paritions_size={} {} with database_id={}, table_id={}", name_mapper.debugCanonicalName(*db_info, *table_info), + new_part_id_set.size(), db_info->id, table_info->id); return; } - auto updated_table_info = orig_table_info; + // Copy the local table info and update fileds on the copy + auto updated_table_info = local_table_info; + updated_table_info.is_partition_table = true; + updated_table_info.belonging_table_id = table_info->belonging_table_id; updated_table_info.partition = table_info->partition; /// Apply changes to physical tables. - for (const auto & orig_def : orig_defs) + for (const auto & local_def : local_defs) { - if (new_part_id_set.count(orig_def.id) == 0) + if (!new_part_id_set.contains(local_def.id)) { - applyDropPhysicalTable(name_mapper.mapDatabaseName(*db_info), orig_def.id); + applyDropPhysicalTable(name_mapper.mapDatabaseName(*db_info), local_def.id); } } for (const auto & new_def : new_defs) { - if (orig_part_id_set.count(new_def.id) == 0) + if (!local_part_id_set.contains(new_def.id)) { table_id_map.emplacePartitionTableID(new_def.id, updated_table_info.id); } @@ -790,7 +840,7 @@ void SchemaBuilder::applyDropSchema(const String & db_name) auto db = context.tryGetDatabase(db_name); if (db == nullptr) { - LOG_INFO(log, "Database {} does not exists", db_name); + LOG_INFO(log, "Database does not exist, db_name={}", db_name); return; } @@ -992,7 +1042,7 @@ void SchemaBuilder::applyDropPhysicalTable(const String & db auto storage = tmt_context.getStorages().get(keyspace_id, table_id); if (storage == nullptr) { - LOG_DEBUG(log, "table {} does not exist.", table_id); + LOG_DEBUG(log, "table does not exist, table_id={}", table_id); return; } GET_METRIC(tiflash_schema_internal_ddl_count, type_drop_table).Increment(); @@ -1033,7 +1083,7 @@ void SchemaBuilder::applyDropTable(DatabaseID database_id, T auto * storage = tmt_context.getStorages().get(keyspace_id, table_id).get(); if (storage == nullptr) { - LOG_DEBUG(log, "table {} does not exist.", table_id); + LOG_DEBUG(log, "table does not exist, table_id={}", table_id); return; } const auto & table_info = storage->getTableInfo(); @@ -1041,6 +1091,21 @@ void SchemaBuilder::applyDropTable(DatabaseID database_id, T { for (const auto & part_def : table_info.partition.definitions) { + if (TableID latest_logical_table_id = table_id_map.findTableIDInPartitionMap(part_def.id); + latest_logical_table_id == -1 || latest_logical_table_id != table_info.id) + { + // The partition is managed by another logical table now (caused by `alter table X partition by ...`), + // skip dropping this partition when dropping the old logical table + LOG_INFO( + log, + "The partition is not managed by current logical table, skip, partition_table_id={} " + "new_logical_table_id={} current_logical_table_id={}", + part_def.id, + latest_logical_table_id, + table_info.id); + continue; + } + applyDropPhysicalTable(name_mapper.mapDatabaseName(database_id, keyspace_id), part_def.id); } } diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.h b/dbms/src/TiDB/Schema/SchemaBuilder.h index 780521b5775..fd4be82d5cf 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.h +++ b/dbms/src/TiDB/Schema/SchemaBuilder.h @@ -206,8 +206,9 @@ struct SchemaBuilder /// Parameter schema_name should be mapped. void applyDropPhysicalTable(const String & db_name, TableID table_id); - void applyPartitionDiff(DatabaseID database_id, TableID table_id); + void applyPartitionAlter(DatabaseID database_id, TableID table_id); + void applyPartitionDiff(DatabaseID database_id, TableID table_id); void applyPartitionDiff( const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info, diff --git a/dbms/src/TiDB/Schema/SchemaGetter.cpp b/dbms/src/TiDB/Schema/SchemaGetter.cpp index 29ec04fdc46..5e170dd7fd6 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.cpp +++ b/dbms/src/TiDB/Schema/SchemaGetter.cpp @@ -248,9 +248,10 @@ std::optional SchemaGetter::getSchemaDiff(Int64 ver) String data = TxnStructure::get(snap, key); if (data.empty()) { - LOG_WARNING(log, "The schema diff for version {}, key {} is empty.", ver, key); + LOG_WARNING(log, "The schema diff is empty, schema_version={} key={}", ver, key); return std::nullopt; } + LOG_TRACE(log, "Get SchemaDiff from TiKV, schema_version={} data={}", ver, data); SchemaDiff diff; diff.deserialize(data); return diff; @@ -274,7 +275,7 @@ TiDB::DBInfoPtr SchemaGetter::getDatabase(DatabaseID db_id) if (json.empty()) return nullptr; - LOG_DEBUG(log, "Get DB Info from TiKV : " + json); + LOG_DEBUG(log, "Get DB Info from TiKV: {}", json); auto db_info = std::make_shared(json, keyspace_id); return db_info; } @@ -284,25 +285,26 @@ TiDB::TableInfoPtr SchemaGetter::getTableInfo(DatabaseID db_id, TableID table_id String db_key = getDBKey(db_id); if (!checkDBExists(db_key)) { - LOG_ERROR(log, "The database {} does not exist.", db_id); + LOG_ERROR(log, "The database does not exist, database_id={}", db_id); return nullptr; } String table_key = getTableKey(table_id); String table_info_json = TxnStructure::hGet(snap, db_key, table_key); if (table_info_json.empty()) { - LOG_WARNING(log, "The table {} is dropped in TiKV, try to get the latest table_info", table_id); + LOG_WARNING(log, "The table is dropped in TiKV, try to get the latest table_info, table_id={}", table_id); table_info_json = TxnStructure::mvccGet(snap, db_key, table_key); if (table_info_json.empty()) { LOG_ERROR( log, - "The table {} is dropped in TiKV, and the latest table_info is still empty, it should by gc", + "The table is dropped in TiKV, and the latest table_info is still empty, it should be GCed, " + "table_id={}", table_id); return nullptr; } } - LOG_DEBUG(log, "Get Table Info from TiKV : " + table_info_json); + LOG_DEBUG(log, "Get Table Info from TiKV: {}", table_info_json); TiDB::TableInfoPtr table_info = std::make_shared(table_info_json, keyspace_id); return table_info; @@ -318,26 +320,26 @@ std::tuple SchemaGetter::getDatabaseAndTabl if (db_json.empty()) return std::make_tuple(nullptr, nullptr); - LOG_DEBUG(log, "Get DB Info from TiKV : " + db_json); + LOG_DEBUG(log, "Get DB Info from TiKV: {}", db_json); auto db_info = std::make_shared(db_json, keyspace_id); String table_key = getTableKey(table_id); String table_info_json = TxnStructure::hGet(snap, db_key, table_key); if (table_info_json.empty()) { - LOG_WARNING(log, "The table {} is dropped in TiKV, try to get the latest table_info", table_id); + LOG_WARNING(log, "The table is dropped in TiKV, try to get the latest table_info, table_id={}", table_id); table_info_json = TxnStructure::mvccGet(snap, db_key, table_key); if (table_info_json.empty()) { LOG_ERROR( log, - "The table {} is dropped in TiKV, and the latest table_info is still empty, it should by gc", + "The table is dropped in TiKV, and the latest table_info is still empty, it should be GCed, " + "table_id={}", table_id); return std::make_tuple(db_info, nullptr); - ; } } - LOG_DEBUG(log, "Get Table Info from TiKV : " + table_info_json); + LOG_DEBUG(log, "Get Table Info from TiKV: {}", table_info_json); TiDB::TableInfoPtr table_info = std::make_shared(table_info_json, keyspace_id); return std::make_tuple(db_info, table_info); diff --git a/dbms/src/TiDB/Schema/TiDB.cpp b/dbms/src/TiDB/Schema/TiDB.cpp index a7224af78ae..8b867037627 100644 --- a/dbms/src/TiDB/Schema/TiDB.cpp +++ b/dbms/src/TiDB/Schema/TiDB.cpp @@ -540,6 +540,10 @@ try part_id_set.emplace(definition.id); } + /// Treat `adding_definitions` and `dropping_definitions` as the normal `definitions` + /// in TiFlash. Because TiFlash need to create the physical IStorage instance + /// to handle the data on those partitions during DDL. + auto add_defs_json = json->getArray("adding_definitions"); if (!add_defs_json.isNull()) { @@ -851,8 +855,6 @@ TableInfo::TableInfo(const String & table_info_json, KeyspaceID keyspace_id_) String TableInfo::serialize() const try { - std::stringstream buf; - Poco::JSON::Object::Ptr json = new Poco::JSON::Object(); json->set("id", id); json->set("keyspace_id", keyspace_id); @@ -867,8 +869,8 @@ try auto col_obj = col_info.getJSONObject(); cols_arr->add(col_obj); } - json->set("cols", cols_arr); + Poco::JSON::Array::Ptr index_arr = new Poco::JSON::Array(); for (const auto & index_info : index_infos) { @@ -904,8 +906,8 @@ try json->set("tiflash_replica", replica_info.getJSONObject()); + std::stringstream buf; json->stringify(buf); - return buf.str(); } catch (const Poco::Exception & e) diff --git a/tests/fullstack-test2/ddl/alter_partition_by.test b/tests/fullstack-test2/ddl/alter_partition_by.test index a05f2a43ce2..9c64a7099d3 100644 --- a/tests/fullstack-test2/ddl/alter_partition_by.test +++ b/tests/fullstack-test2/ddl/alter_partition_by.test @@ -28,7 +28,7 @@ mysql> insert into test.t select a+1000000,a+1000000,-(a+1000000) from test.t; func> wait_table test t -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ * from test.t order by a; +mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t order by a; +---------+---------+----------+ | a | b | c | +---------+---------+----------+ @@ -56,7 +56,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ * from test.t order by a; │ test │ t │ └───────────────┴───────────┘ -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -64,7 +64,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition ( +----------+ mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -72,14 +72,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio +----------+ mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ | 8 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ @@ -90,7 +90,7 @@ mysql> show warnings; mysql> alter table test.t partition by range (a) (partition p0 values less than (500000), partition p500k values less than (1000000), partition p1M values less than (2000000)); -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -99,7 +99,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p500k); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p500k); +----------+ | count(*) | +----------+ @@ -108,14 +108,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ | 4 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p500k); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p500k); +----------+ | count(*) | +----------+ @@ -124,14 +124,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition ( mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ | 8 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ @@ -143,8 +143,9 @@ mysql> create table test.t2 (a int primary key, b varchar(255), c int, key (b), mysql> alter table test.t2 set tiflash replica 1; mysql> insert into test.t2 select * from test.t; func> wait_table test t2 +mysql> drop table test.t; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ * from test.t2 order by a; +mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t2 order by a; +---------+---------+----------+ | a | b | c | +---------+---------+----------+ @@ -166,8 +167,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ * from test.t2 order by a; | 1500004 | 1500004 | -1500004 | +---------+---------+----------+ -mysql> drop table test.t; - +# check the t2 is created in TiFlash >> select tidb_database,tidb_name from system.tables where tidb_database = 'test' and tidb_name = 't2' and is_tombstone = 0 ┌─tidb_database─┬─tidb_name─┐ │ test │ t2 │ @@ -176,7 +176,7 @@ mysql> drop table test.t; mysql> alter table test.t2 partition by hash (a) partitions 3; mysql> analyze table test.t2; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ count(*) from test.t2 partition (p0); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t2 partition (p0); +----------+ | count(*) | +----------+ @@ -185,7 +185,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ count(*) from test.t2 partit mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ count(*) from test.t2 partition (p1); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t2 partition (p1); +----------+ | count(*) | +----------+ @@ -194,14 +194,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ count(*) from test.t2 partit mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t2]) */ count(*) from test.t2 partition (p0); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t2 partition (p0); +----------+ | count(*) | +----------+ | 5 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIKV[t2]) */ count(*) from test.t2 partition (p1); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t2 partition (p1); +----------+ | count(*) | +----------+ @@ -211,14 +211,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIKV[t2]) */ count(*) from test.t2 partition mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t2]) */ count(*) from test.t2 partition (p2); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t2 partition (p2); +----------+ | count(*) | +----------+ | 5 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t2]) */ count(*) from test.t2 partition (p2); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t2 partition (p2); +----------+ | count(*) | +----------+ diff --git a/tests/fullstack-test2/ddl/remove_partitioning.test b/tests/fullstack-test2/ddl/remove_partitioning.test index 2f8f9574be8..2ce14aa9cc4 100644 --- a/tests/fullstack-test2/ddl/remove_partitioning.test +++ b/tests/fullstack-test2/ddl/remove_partitioning.test @@ -28,13 +28,35 @@ mysql> insert into test.t select a+1000000,a+1000000,-(a+1000000) from test.t; func> wait_table test t +mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t; ++---------+---------+----------+ +| a | b | c | ++---------+---------+----------+ +| 1 | 1 | -1 | +| 2 | 2 | -2 | +| 3 | 3 | -3 | +| 4 | 4 | -4 | +| 500001 | 500001 | -500001 | +| 500002 | 500002 | -500002 | +| 500003 | 500003 | -500003 | +| 500004 | 500004 | -500004 | +| 1000001 | 1000001 | -1000001 | +| 1000002 | 1000002 | -1000002 | +| 1000003 | 1000003 | -1000003 | +| 1000004 | 1000004 | -1000004 | +| 1500001 | 1500001 | -1500001 | +| 1500002 | 1500002 | -1500002 | +| 1500003 | 1500003 | -1500003 | +| 1500004 | 1500004 | -1500004 | ++---------+---------+----------+ + # 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─┐ │ test │ t │ └───────────────┴───────────┘ -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -42,7 +64,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition ( +----------+ mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -50,14 +72,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio +----------+ mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ | 8 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ @@ -68,7 +90,7 @@ mysql> show warnings; mysql> alter table test.t remove partitioning; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t; +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t; +----------+ | count(*) | +----------+ @@ -77,7 +99,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t; mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t; +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t; +----------+ | count(*) | +----------+ diff --git a/tests/fullstack-test2/ddl/reorganize_partition.test b/tests/fullstack-test2/ddl/reorganize_partition.test index 971a4ce8341..9453bc373e3 100644 --- a/tests/fullstack-test2/ddl/reorganize_partition.test +++ b/tests/fullstack-test2/ddl/reorganize_partition.test @@ -34,7 +34,7 @@ func> wait_table test t │ test │ t │ └───────────────┴───────────┘ -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -42,7 +42,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition ( +----------+ mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -50,14 +50,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio +----------+ mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ | 8 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ @@ -68,7 +68,7 @@ mysql> show warnings; mysql> alter table test.t reorganize partition p0 INTO (partition p0 values less than (500000), partition p500k values less than (1000000)); -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ @@ -77,7 +77,7 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p500k); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p500k); +----------+ | count(*) | +----------+ @@ -86,14 +86,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partitio mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p0); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p0); +----------+ | count(*) | +----------+ | 4 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p500k); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p500k); +----------+ | count(*) | +----------+ @@ -102,14 +102,14 @@ mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition ( mysql> show warnings; -mysql> select /*+ READ_FROM_STORAGE(TIKV[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tikv'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ | 8 | +----------+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[t]) */ count(*) from test.t partition (p1M); +mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t partition (p1M); +----------+ | count(*) | +----------+ @@ -147,14 +147,14 @@ mysql> alter table test.t1 reorganize partition p1 INTO (partition p1 values les # make write cmd take effect >> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd) -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[test.t1]) */ * from test.t1 partition (p1); +mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t1 partition (p1); +----+------+----+ | id | name | c | +----+------+----+ | 60 | cba |NULL| +----+------+----+ -mysql> select /*+ READ_FROM_STORAGE(TIFLASH[test.t1]) */ * from test.t1 partition (p2); +mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t1 partition (p2); +----+------+----+ | id | name | c | +----+------+----+ From 7f95f536df860883dca0fed648ea4aeabb0befb9 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 10 Nov 2023 10:56:12 +0800 Subject: [PATCH 2/3] Refine logging Signed-off-by: JaySon-Huang --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 30 +++++++++++++------------- dbms/src/TiDB/Schema/SchemaBuilder.h | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index a61392209d9..4f92c76a71a 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -81,7 +81,7 @@ void SchemaBuilder::applyCreateTable(DatabaseID database_id, // If table is partition table, we will create the logical table here. // Because we get the table_info, so we can ensure new_db_info will not be nullptr. auto new_db_info = getter.getDatabase(database_id); - applyCreatePhysicalTable(new_db_info, table_info); + applyCreateStorageInstance(new_db_info, table_info); for (const auto & part_def : table_info->partition.definitions) { @@ -399,7 +399,7 @@ void SchemaBuilder::applyPartitionAlter(DatabaseID database_ // If table is partition table, we will create the logical table here. // Because we get the table_info, so we can ensure new_db_info will not be nullptr. auto new_db_info = getter.getDatabase(database_id); - applyCreatePhysicalTable(new_db_info, table_info); + applyCreateStorageInstance(new_db_info, table_info); for (const auto & part_def : table_info->partition.definitions) { @@ -783,7 +783,7 @@ template void SchemaBuilder::applyCreateSchema(const TiDB::DBInfoPtr & db_info) { GET_METRIC(tiflash_schema_internal_ddl_count, type_create_db).Increment(); - LOG_INFO(log, "Creating database {} with database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id); + LOG_INFO(log, "Create database {} begin, database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id); auto statement = createDatabaseStmt(context, *db_info, name_mapper); @@ -799,7 +799,7 @@ void SchemaBuilder::applyCreateSchema(const TiDB::DBInfoPtr databases.emplace(db_info->id, db_info); } - LOG_INFO(log, "Created database {} with database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id); + LOG_INFO(log, "Create database {} end, database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id); } template @@ -836,7 +836,7 @@ template void SchemaBuilder::applyDropSchema(const String & db_name) { GET_METRIC(tiflash_schema_internal_ddl_count, type_drop_db).Increment(); - LOG_INFO(log, "Tombstoning database {}", db_name); + LOG_INFO(log, "Tombstone database begin, db_name={}", db_name); auto db = context.tryGetDatabase(db_name); if (db == nullptr) { @@ -857,7 +857,7 @@ void SchemaBuilder::applyDropSchema(const String & db_name) auto tombstone = tmt_context.getPDClient()->getTS(); db->alterTombstone(context, tombstone); - LOG_INFO(log, "Tombstoned database {}", db_name); + LOG_INFO(log, "Tombstone database end, db_name={}", db_name); } std::tuple parseColumnsFromTableInfo(const TiDB::TableInfo & table_info) @@ -938,7 +938,7 @@ String createTableStmt( } template -void SchemaBuilder::applyCreatePhysicalTable( +void SchemaBuilder::applyCreateStorageInstance( const TiDB::DBInfoPtr & db_info, const TableInfoPtr & table_info) { @@ -1048,7 +1048,7 @@ void SchemaBuilder::applyDropPhysicalTable(const String & db GET_METRIC(tiflash_schema_internal_ddl_count, type_drop_table).Increment(); LOG_INFO( log, - "Tombstoning table {}.{}, table_id={}", + "Tombstone table {}.{} begin, table_id={}", db_name, name_mapper.debugTableName(storage->getTableInfo()), table_id); @@ -1070,7 +1070,7 @@ void SchemaBuilder::applyDropPhysicalTable(const String & db storage->updateTombstone(alter_lock, commands, db_name, storage->getTableInfo(), name_mapper, context); LOG_INFO( log, - "Tombstoned table {}.{}, table_id={}", + "Tombstone table {}.{} end, table_id={}", db_name, name_mapper.debugTableName(storage->getTableInfo()), table_id); @@ -1120,7 +1120,7 @@ void SchemaBuilder::applyDropTable(DatabaseID database_id, T template void SchemaBuilder::syncAllSchema() { - LOG_INFO(log, "Syncing all schemas."); + LOG_INFO(log, "Sync all schemas begin"); /// Create all databases. std::vector all_schemas = getter.listDBs(); @@ -1181,7 +1181,7 @@ void SchemaBuilder::syncAllSchema() table_id_map.emplaceTableID(table->id, db->id); LOG_DEBUG(log, "register table to table_id_map, database_id={} table_id={}", db->id, table->id); - applyCreatePhysicalTable(db, table); + applyCreateStorageInstance(db, table); if (table->isLogicalPartitionTable()) { for (const auto & part_def : table->partition.definitions) @@ -1238,7 +1238,7 @@ void SchemaBuilder::syncAllSchema() } } - LOG_INFO(log, "Loaded all schemas."); + LOG_INFO(log, "Sync all schemas end"); } template @@ -1295,7 +1295,7 @@ void SchemaBuilder::applyTable( return; } - applyCreatePhysicalTable(db_info, table_info); + applyCreateStorageInstance(db_info, table_info); } else { @@ -1320,7 +1320,7 @@ void SchemaBuilder::applyTable( template void SchemaBuilder::dropAllSchema() { - LOG_INFO(log, "Dropping all schemas."); + LOG_INFO(log, "Drop all schemas begin"); auto & tmt_context = context.getTMTContext(); @@ -1355,7 +1355,7 @@ void SchemaBuilder::dropAllSchema() LOG_DEBUG(log, "DB {} dropped during drop all schemas", db.first); } - LOG_INFO(log, "Dropped all schemas."); + LOG_INFO(log, "Drop all schemas end"); } // product env diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.h b/dbms/src/TiDB/Schema/SchemaBuilder.h index fd4be82d5cf..58150f20fe2 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.h +++ b/dbms/src/TiDB/Schema/SchemaBuilder.h @@ -195,7 +195,7 @@ struct SchemaBuilder void applyCreateSchema(const TiDB::DBInfoPtr & db_info); - void applyCreatePhysicalTable(const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info); + void applyCreateStorageInstance(const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info); void applyDropTable(DatabaseID database_id, TableID table_id); From 6f5e963b8303f693ef37c5a9224ea03fc3af6b3b Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 10 Nov 2023 13:46:22 +0800 Subject: [PATCH 3/3] Replace applyPartitionAlter by applyCreateTable --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 86 +++++++------------------- dbms/src/TiDB/Schema/SchemaBuilder.h | 2 - 2 files changed, 22 insertions(+), 66 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index 4f92c76a71a..bcfd77b100d 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -76,22 +76,26 @@ void SchemaBuilder::applyCreateTable(DatabaseID database_id, table_id_map.emplaceTableID(table_id, database_id); LOG_DEBUG(log, "register table to table_id_map, database_id={} table_id={}", database_id, table_id); - if (table_info->isLogicalPartitionTable()) + // non partition table, done + if (!table_info->isLogicalPartitionTable()) { - // If table is partition table, we will create the logical table here. - // Because we get the table_info, so we can ensure new_db_info will not be nullptr. - auto new_db_info = getter.getDatabase(database_id); - applyCreateStorageInstance(new_db_info, table_info); + return; + } - for (const auto & part_def : table_info->partition.definitions) - { - LOG_DEBUG( - log, - "register table to table_id_map for partition table, logical_table_id={} physical_table_id={}", - table_id, - part_def.id); - table_id_map.emplacePartitionTableID(part_def.id, table_id); - } + // If table is partition table, we will create the logical table here. + // Because we get the table_info, so we can ensure new_db_info will not be nullptr. + auto new_db_info = getter.getDatabase(database_id); + applyCreateStorageInstance(new_db_info, table_info); + + // Register the partition_id -> logical_table_id mapping + for (const auto & part_def : table_info->partition.definitions) + { + LOG_DEBUG( + log, + "register table to table_id_map for partition table, logical_table_id={} physical_table_id={}", + table_id, + part_def.id); + table_id_map.emplacePartitionTableID(part_def.id, table_id); } } @@ -254,9 +258,8 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) { // Create the new table. // If the new table is a partition table, this will also overwrite - // the partition id mapping to the new logical table and renew the - // partition info. - applyPartitionAlter(diff.schema_id, diff.table_id); + // the partition id mapping to the new logical table + applyCreateTable(diff.schema_id, diff.table_id); // Drop the old table. if the previous partitions of the old table are // not mapping to the old logical table now, they will not be removed. applyDropTable(diff.schema_id, diff.old_table_id); @@ -378,51 +381,6 @@ void SchemaBuilder::applySetTiFlashReplica(DatabaseID databa } } -template -void SchemaBuilder::applyPartitionAlter(DatabaseID database_id, TableID table_id) -{ - auto table_info = getter.getTableInfo(database_id, table_id); - if (table_info == nullptr) // the database maybe dropped - { - LOG_DEBUG(log, "table is not exist in TiKV, may have been dropped, table_id={}", table_id); - return; - } - - table_id_map.emplaceTableID(table_id, database_id); - LOG_DEBUG(log, "register table to table_id_map, database_id={} table_id={}", database_id, table_id); - - if (!table_info->isLogicalPartitionTable()) - { - return; - } - - // If table is partition table, we will create the logical table here. - // Because we get the table_info, so we can ensure new_db_info will not be nullptr. - auto new_db_info = getter.getDatabase(database_id); - applyCreateStorageInstance(new_db_info, table_info); - - for (const auto & part_def : table_info->partition.definitions) - { - LOG_DEBUG( - log, - "register table to table_id_map for partition table, logical_table_id={} physical_table_id={}", - table_id, - part_def.id); - table_id_map.emplacePartitionTableID(part_def.id, table_id); - } - - auto & tmt_context = context.getTMTContext(); - auto storage = tmt_context.getStorages().get(keyspace_id, table_info->id); - if (storage == nullptr) - { - LOG_ERROR(log, "table is not exist in TiFlash, table_id={}", table_id); - return; - } - - // Try to renew the partition info for the new table - applyPartitionDiff(new_db_info, table_info, storage); -} - template void SchemaBuilder::applyPartitionDiff(DatabaseID database_id, TableID table_id) { @@ -945,7 +903,7 @@ void SchemaBuilder::applyCreateStorageInstance( GET_METRIC(tiflash_schema_internal_ddl_count, type_create_table).Increment(); LOG_INFO( log, - "Creating table {} with database_id={}, table_id={}", + "Create table {} begin, database_id={}, table_id={}", name_mapper.debugCanonicalName(*db_info, *table_info), db_info->id, table_info->id); @@ -1028,7 +986,7 @@ void SchemaBuilder::applyCreateStorageInstance( interpreter.execute(); LOG_INFO( log, - "Created table {}, database_id={} table_id={}", + "Creat table {} end, database_id={} table_id={}", name_mapper.debugCanonicalName(*db_info, *table_info), db_info->id, table_info->id); diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.h b/dbms/src/TiDB/Schema/SchemaBuilder.h index 58150f20fe2..dd9ea341b78 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.h +++ b/dbms/src/TiDB/Schema/SchemaBuilder.h @@ -206,8 +206,6 @@ struct SchemaBuilder /// Parameter schema_name should be mapped. void applyDropPhysicalTable(const String & db_name, TableID table_id); - void applyPartitionAlter(DatabaseID database_id, TableID table_id); - void applyPartitionDiff(DatabaseID database_id, TableID table_id); void applyPartitionDiff( const TiDB::DBInfoPtr & db_info,