Skip to content

Commit 717405f

Browse files
authored
ddl: Fix failure on executing exchange partition(release-6.5) (#8374)
close #8372
1 parent 8138e8a commit 717405f

File tree

4 files changed

+142
-35
lines changed

4 files changed

+142
-35
lines changed

dbms/src/Encryption/RateLimiter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ IOLimitTuner::TuneResult IOLimitTuner::tune() const
712712
auto msg = fmt::format("limiter {} write {} read {}", limiterCount(), writeLimiterCount(), readLimiterCount());
713713
if (limiterCount() < 2)
714714
{
715-
LOG_INFO(log, "{} NOT need to tune.", msg);
715+
LOG_TRACE(log, "{} NOT need to tune.", msg);
716716
return {0, 0, false, 0, 0, false};
717717
}
718718
LOG_INFO(log, "{} need to tune.", msg);

dbms/src/TiDB/Schema/SchemaBuilder.cpp

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -595,11 +595,11 @@ void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr
595595
throw TiFlashException(fmt::format("miss table in TiFlash {}", table_id), Errors::DDL::MissingTable);
596596
}
597597

598-
applyPartitionDiff(db_info, table_info, storage);
598+
applyPartitionDiff(db_info, table_info, storage, /*drop_part_if_not_exist*/ true);
599599
}
600600

601601
template <typename Getter, typename NameMapper>
602-
void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr & db_info, const TableInfoPtr & table_info, const ManageableStoragePtr & storage)
602+
void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr & db_info, const TableInfoPtr & table_info, const ManageableStoragePtr & storage, bool drop_part_if_not_exist)
603603
{
604604
const auto & orig_table_info = storage->getTableInfo();
605605
if (!orig_table_info.isLogicalPartitionTable())
@@ -641,13 +641,17 @@ void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr
641641
updated_table_info.partition = table_info->partition;
642642

643643
/// Apply changes to physical tables.
644-
for (const auto & orig_def : orig_defs)
644+
if (drop_part_if_not_exist)
645645
{
646-
if (new_part_id_set.count(orig_def.id) == 0)
646+
for (const auto & orig_def : orig_defs)
647647
{
648-
applyDropPhysicalTable(name_mapper.mapDatabaseName(*db_info), orig_def.id);
648+
if (new_part_id_set.count(orig_def.id) == 0)
649+
{
650+
applyDropPhysicalTable(name_mapper.mapDatabaseName(*db_info), orig_def.id);
651+
}
649652
}
650653
}
654+
651655
for (const auto & new_def : new_defs)
652656
{
653657
if (orig_part_id_set.count(new_def.id) == 0)
@@ -777,7 +781,7 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
777781
diff.table_id);
778782

779783
/// Exchange table partition is used for ddl:
780-
/// alter table partition_table exchange partition partition_name with table non_partition_table
784+
/// `ALTER TABLE partition_table EXCHANGE PARTITION partition_name WITH TABLE non_partition_table`
781785
/// It involves three table/partition: partition_table, partition_name and non_partition_table
782786
/// The table id/schema id for the 3 table/partition are stored in SchemaDiff as follows:
783787
/// Table_id in diff is the partition id of which will be exchanged,
@@ -788,48 +792,49 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
788792
GET_METRIC(tiflash_schema_internal_ddl_count, type_exchange_partition).Increment();
789793
if (diff.affected_opts.empty())
790794
throw Exception("Incorrect schema diff, no affected_opts for alter table exchange partition schema diff", ErrorCodes::DDL_ERROR);
791-
auto npt_db_info = getter.getDatabase(diff.schema_id);
795+
const auto npt_database_id = diff.schema_id;
796+
const auto pt_database_id = diff.affected_opts[0].schema_id;
797+
auto npt_db_info = getter.getDatabase(npt_database_id);
792798
if (npt_db_info == nullptr)
793799
throw TiFlashException(fmt::format("miss database: {}", diff.schema_id), Errors::DDL::StaleSchema);
794-
auto pt_db_info = getter.getDatabase(diff.affected_opts[0].schema_id);
800+
auto pt_db_info = getter.getDatabase(pt_database_id);
795801
if (pt_db_info == nullptr)
796802
throw TiFlashException(fmt::format("miss database: {}", diff.affected_opts[0].schema_id), Errors::DDL::StaleSchema);
797-
auto npt_table_id = diff.old_table_id;
798-
auto pt_partition_id = diff.table_id;
799-
auto pt_table_info = diff.affected_opts[0].table_id;
803+
const auto npt_table_id = diff.old_table_id;
804+
const auto pt_partition_id = diff.table_id;
805+
const auto pt_table_id = diff.affected_opts[0].table_id;
806+
807+
LOG_INFO(log, "Execute exchange partition begin, npt_table_id={} npt_database_id={} pt_table_id={} pt_partition_id={} pt_database_id={}", npt_table_id, npt_database_id, pt_table_id, pt_partition_id, pt_database_id);
800808
/// step 1 change the mete data of partition table
801-
auto table_info = getter.getTableInfo(pt_db_info->id, pt_table_info);
809+
auto table_info = getter.getTableInfo(pt_db_info->id, pt_table_id); // latest partition table info from TiKV
802810
if (table_info == nullptr)
803-
throw TiFlashException(fmt::format("miss table in TiKV : {}", pt_table_info), Errors::DDL::StaleSchema);
811+
throw TiFlashException(fmt::format("miss table in TiKV : pt_table_id={}", pt_table_id), Errors::DDL::StaleSchema);
804812
auto & tmt_context = context.getTMTContext();
805813
auto storage = tmt_context.getStorages().get(table_info->id);
806814
if (storage == nullptr)
807815
throw TiFlashException(
808816
fmt::format("miss table in TiFlash : {}", name_mapper.debugCanonicalName(*pt_db_info, *table_info)),
809817
Errors::DDL::MissingTable);
810818

811-
LOG_INFO(log, "Exchange partition for table {}", name_mapper.debugCanonicalName(*pt_db_info, *table_info));
812-
auto orig_table_info = storage->getTableInfo();
813-
orig_table_info.partition = table_info->partition;
814-
{
815-
auto alter_lock = storage->lockForAlter(getThreadName());
816-
storage->alterFromTiDB(
817-
alter_lock,
818-
AlterCommands{},
819-
name_mapper.mapDatabaseName(*pt_db_info),
820-
orig_table_info,
821-
name_mapper,
822-
context);
823-
}
819+
// Apply the new partitions to the logical table.
820+
/// - create the new physical tables according to the new partition definitions
821+
/// - persist the new table info to disk
822+
// The latest table info could be the table info after `EXCHANGE PARTITION` is executed
823+
// on TiDB. So we need to apply and also create the physical tables of new ids appear in
824+
// the partition list. Because we can not get a table schema by its physical_table_id
825+
// once it became a partition.
826+
// But this method will skip dropping partition id that is not exist in the new table_info,
827+
// because the physical table could be changed into a normal table without dropping.
828+
applyPartitionDiff(pt_db_info, table_info, storage, /*drop_part_if_not_exist*/ false);
824829
FAIL_POINT_TRIGGER_EXCEPTION(FailPoints::exception_after_step_1_in_exchange_partition);
825830

826831
/// step 2 change non partition table to a partition of the partition table
827832
storage = tmt_context.getStorages().get(npt_table_id);
828833
if (storage == nullptr)
829-
throw TiFlashException(fmt::format("miss table in TiFlash : {}", name_mapper.debugCanonicalName(*npt_db_info, *table_info)),
834+
throw TiFlashException(fmt::format("miss table in TiFlash, npt_table_id={} : {}", npt_table_id, name_mapper.debugCanonicalName(*npt_db_info, *table_info)),
830835
Errors::DDL::MissingTable);
831-
orig_table_info = storage->getTableInfo();
832-
orig_table_info.belonging_table_id = pt_table_info;
836+
auto orig_table_info = storage->getTableInfo();
837+
orig_table_info.belonging_table_id = pt_table_id;
833838
orig_table_info.is_partition_table = true;
834839
/// partition does not have explicit name, so use default name here
835840
orig_table_info.name = name_mapper.mapTableName(orig_table_info);
@@ -852,11 +857,14 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
852857
/// step 3 change partition of the partition table to non partition table
853858
table_info = getter.getTableInfo(npt_db_info->id, pt_partition_id);
854859
if (table_info == nullptr)
855-
throw TiFlashException(fmt::format("miss table in TiKV : {}", pt_partition_id), Errors::DDL::StaleSchema);
856-
storage = tmt_context.getStorages().get(table_info->id);
860+
{
861+
LOG_WARNING(log, "Execute exchange partition, the table info of partition can not get from TiKV, npt_database_id={} partition_id={}", npt_database_id, pt_partition_id);
862+
throw TiFlashException(fmt::format("miss partition table in TiKV, may have been dropped, physical_table_id={}", pt_partition_id), Errors::DDL::StaleSchema);
863+
}
864+
storage = tmt_context.getStorages().get(pt_partition_id);
857865
if (storage == nullptr)
858866
throw TiFlashException(
859-
fmt::format("miss table in TiFlash : {}", name_mapper.debugCanonicalName(*pt_db_info, *table_info)),
867+
fmt::format("miss partition table in TiFlash, physical_table_id={}", pt_partition_id),
860868
Errors::DDL::MissingTable);
861869
orig_table_info = storage->getTableInfo();
862870
orig_table_info.belonging_table_id = DB::InvalidTableID;
@@ -877,6 +885,7 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
877885
if (npt_db_info->id != pt_db_info->id)
878886
applyRenamePhysicalTable(npt_db_info, orig_table_info, storage);
879887
FAIL_POINT_TRIGGER_EXCEPTION(FailPoints::exception_after_step_3_in_exchange_partition);
888+
LOG_INFO(log, "Execute exchange partition done, npt_table_id={} npt_database_id={} pt_table_id={} pt_partition_id={} pt_database_id={}", npt_table_id, npt_database_id, pt_table_id, pt_partition_id, pt_database_id);
880889
}
881890

882891
template <typename Getter, typename NameMapper>
@@ -1347,7 +1356,7 @@ void SchemaBuilder<Getter, NameMapper>::syncAllSchema()
13471356
if (table->isLogicalPartitionTable())
13481357
{
13491358
/// Apply partition diff if needed.
1350-
applyPartitionDiff(db, table, storage);
1359+
applyPartitionDiff(db, table, storage, /*drop_part_if_not_exist*/ true);
13511360
}
13521361
/// Rename if needed.
13531362
applyRenameLogicalTable(db, table, storage);

dbms/src/TiDB/Schema/SchemaBuilder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ struct SchemaBuilder
7070

7171
void applyPartitionDiff(const TiDB::DBInfoPtr & db_info, TableID table_id);
7272

73-
void applyPartitionDiff(const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info, const ManageableStoragePtr & storage);
73+
void applyPartitionDiff(const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info, const ManageableStoragePtr & storage, bool drop_part_if_not_exist);
7474

7575
void applyAlterTable(const TiDB::DBInfoPtr & db_info, TableID table_id);
7676

tests/fullstack-test2/ddl/alter_exchange_partition.test

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,104 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new
267267
mysql> alter table test.e drop column c1;
268268
>> DBGInvoke __refresh_schemas()
269269

270+
# cleanup
271+
mysql> drop table if exists test.e;
272+
mysql> drop table if exists test.e2;
273+
mysql> drop table if exists test_new.e2;
274+
mysql> drop database if exists test_new;
275+
276+
# case 11, create non-partition table and execute exchagne partition immediately
277+
mysql> create table test.e(id INT NOT NULL,fname VARCHAR(30),lname VARCHAR(30)) PARTITION BY RANGE (id) ( PARTITION p0 VALUES LESS THAN (50),PARTITION p1 VALUES LESS THAN (100),PARTITION p2 VALUES LESS THAN (150), PARTITION p3 VALUES LESS THAN (MAXVALUE));
278+
mysql> insert into test.e values (1, 'a', 'b'),(108, 'a', 'b');
279+
# sync the partition table to tiflash
280+
>> DBGInvoke __refresh_schemas()
281+
282+
mysql> create table test.e2(id int not null,fname varchar(30),lname varchar(30));
283+
mysql> insert into test.e2 values (2, 'a', 'b');
284+
mysql> set @@tidb_enable_exchange_partition=1; alter table test.e exchange partition p0 with table test.e2
285+
286+
mysql> alter table test.e set tiflash replica 1;
287+
mysql> alter table test.e2 set tiflash replica 1;
288+
func> wait_table test e e2
289+
>> DBGInvoke __refresh_schemas()
290+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
291+
+-----+-------+-------+
292+
| id | fname | lname |
293+
+-----+-------+-------+
294+
| 2 | a | b |
295+
| 108 | a | b |
296+
+-----+-------+-------+
297+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
298+
+-----+-------+-------+
299+
| id | fname | lname |
300+
+-----+-------+-------+
301+
| 1 | a | b |
302+
+-----+-------+-------+
303+
304+
# ensure the swap out table is not mark as tombstone
305+
>> DBGInvoke __enable_schema_sync_service('true')
306+
>> DBGInvoke __gc_schemas(18446744073709551615)
307+
>> DBGInvoke __enable_schema_sync_service('false')
308+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
309+
+-----+-------+-------+
310+
| id | fname | lname |
311+
+-----+-------+-------+
312+
| 2 | a | b |
313+
| 108 | a | b |
314+
+-----+-------+-------+
315+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
316+
+-----+-------+-------+
317+
| id | fname | lname |
318+
+-----+-------+-------+
319+
| 1 | a | b |
320+
+-----+-------+-------+
321+
322+
# case 12, create partition table, non-partition table and execute exchagne partition immediately
323+
mysql> drop table if exists test.e
324+
mysql> drop table if exists test.e2
325+
mysql> create table test.e(id INT NOT NULL,fname VARCHAR(30),lname VARCHAR(30)) PARTITION BY RANGE (id) ( PARTITION p0 VALUES LESS THAN (50),PARTITION p1 VALUES LESS THAN (100),PARTITION p2 VALUES LESS THAN (150), PARTITION p3 VALUES LESS THAN (MAXVALUE));
326+
mysql> insert into test.e values (1, 'a', 'b'),(108, 'a', 'b');
327+
mysql> create table test.e2(id int not null,fname varchar(30),lname varchar(30));
328+
mysql> insert into test.e2 values (2, 'a', 'b');
329+
mysql> set @@tidb_enable_exchange_partition=1; alter table test.e exchange partition p0 with table test.e2
330+
331+
mysql> alter table test.e set tiflash replica 1;
332+
mysql> alter table test.e2 set tiflash replica 1;
333+
func> wait_table test e e2
334+
# tiflash the final result
335+
>> DBGInvoke __refresh_schemas()
336+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
337+
+-----+-------+-------+
338+
| id | fname | lname |
339+
+-----+-------+-------+
340+
| 2 | a | b |
341+
| 108 | a | b |
342+
+-----+-------+-------+
343+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
344+
+-----+-------+-------+
345+
| id | fname | lname |
346+
+-----+-------+-------+
347+
| 1 | a | b |
348+
+-----+-------+-------+
349+
# ensure the swap out table is not mark as tombstone
350+
>> DBGInvoke __enable_schema_sync_service('true')
351+
>> DBGInvoke __gc_schemas(18446744073709551615)
352+
>> DBGInvoke __enable_schema_sync_service('false')
353+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
354+
+-----+-------+-------+
355+
| id | fname | lname |
356+
+-----+-------+-------+
357+
| 2 | a | b |
358+
| 108 | a | b |
359+
+-----+-------+-------+
360+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
361+
+-----+-------+-------+
362+
| id | fname | lname |
363+
+-----+-------+-------+
364+
| 1 | a | b |
365+
+-----+-------+-------+
366+
367+
# cleanup
270368
mysql> drop table if exists test.e;
271369
mysql> drop table if exists test.e2;
272370
mysql> drop table if exists test_new.e2;

0 commit comments

Comments
 (0)