Skip to content

Commit

Permalink
ddl: Fix failure on executing exchange partition(release-7.1) (#8375)
Browse files Browse the repository at this point in the history 
close #8372
  • Loading branch information
@JaySon-Huang
JaySon-Huang authored Nov 16, 2023
1 parent 2d8afbe commit 4d7b3f6
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 34 deletions.
75 changes: 42 additions & 33 deletions dbms/src/TiDB/Schema/SchemaBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,11 +602,11 @@ void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr
throw TiFlashException(fmt::format("miss table in TiFlash {}", table_id), Errors::DDL::MissingTable);
}

applyPartitionDiff(db_info, table_info, storage);
applyPartitionDiff(db_info, table_info, storage, /*drop_part_if_not_exist*/ true);
}

template <typename Getter, typename NameMapper>
void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr & db_info, const TableInfoPtr & table_info, const ManageableStoragePtr & storage)
void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr & db_info, const TableInfoPtr & table_info, const ManageableStoragePtr & storage, bool drop_part_if_not_exist)
{
const auto & orig_table_info = storage->getTableInfo();
if (!orig_table_info.isLogicalPartitionTable())
Expand Down Expand Up @@ -648,13 +648,17 @@ void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(const TiDB::DBInfoPtr
updated_table_info.partition = table_info->partition;

/// Apply changes to physical tables.
for (const auto & orig_def : orig_defs)
if (drop_part_if_not_exist)
{
if (new_part_id_set.count(orig_def.id) == 0)
for (const auto & orig_def : orig_defs)
{
applyDropPhysicalTable(name_mapper.mapDatabaseName(*db_info), orig_def.id);
if (new_part_id_set.count(orig_def.id) == 0)
{
applyDropPhysicalTable(name_mapper.mapDatabaseName(*db_info), orig_def.id);
}
}
}

for (const auto & new_def : new_defs)
{
if (orig_part_id_set.count(new_def.id) == 0)
Expand Down Expand Up @@ -783,7 +787,7 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
diff.affected_opts[0].table_id,
diff.table_id);
/// Exchange table partition is used for ddl:
/// alter table partition_table exchange partition partition_name with table non_partition_table
/// `ALTER TABLE partition_table EXCHANGE PARTITION partition_name WITH TABLE non_partition_table`
/// It involves three table/partition: partition_table, partition_name and non_partition_table
/// The table id/schema id for the 3 table/partition are stored in SchemaDiff as follows:
/// Table_id in diff is the partition id of which will be exchanged,
Expand All @@ -794,48 +798,49 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
GET_METRIC(tiflash_schema_internal_ddl_count, type_exchange_partition).Increment();
if (diff.affected_opts.empty())
throw Exception("Incorrect schema diff, no affected_opts for alter table exchange partition schema diff", ErrorCodes::DDL_ERROR);
auto npt_db_info = getter.getDatabase(diff.schema_id);
const auto npt_database_id = diff.schema_id;
const auto pt_database_id = diff.affected_opts[0].schema_id;
auto npt_db_info = getter.getDatabase(npt_database_id);
if (npt_db_info == nullptr)
throw TiFlashException(fmt::format("miss database: {}", diff.schema_id), Errors::DDL::StaleSchema);
auto pt_db_info = getter.getDatabase(diff.affected_opts[0].schema_id);
auto pt_db_info = getter.getDatabase(pt_database_id);
if (pt_db_info == nullptr)
throw TiFlashException(fmt::format("miss database: {}", diff.affected_opts[0].schema_id), Errors::DDL::StaleSchema);
auto npt_table_id = diff.old_table_id;
auto pt_partition_id = diff.table_id;
auto pt_table_info = diff.affected_opts[0].table_id;
const auto npt_table_id = diff.old_table_id;
const auto pt_partition_id = diff.table_id;
const auto pt_table_id = diff.affected_opts[0].table_id;

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);
/// step 1 change the mete data of partition table
auto table_info = getter.getTableInfo(pt_db_info->id, pt_table_info);
auto table_info = getter.getTableInfo(pt_db_info->id, pt_table_id); // latest partition table info from TiKV
if (table_info == nullptr)
throw TiFlashException(fmt::format("miss table in TiKV : {}", pt_table_info), Errors::DDL::StaleSchema);
throw TiFlashException(fmt::format("miss table in TiKV : pt_table_id={}", pt_table_id), Errors::DDL::StaleSchema);
auto & tmt_context = context.getTMTContext();
auto storage = tmt_context.getStorages().get(keyspace_id, table_info->id);
if (storage == nullptr)
throw TiFlashException(
fmt::format("miss table in TiFlash : {}", name_mapper.debugCanonicalName(*pt_db_info, *table_info)),
Errors::DDL::MissingTable);

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

/// step 2 change non partition table to a partition of the partition table
storage = tmt_context.getStorages().get(keyspace_id, npt_table_id);
if (storage == nullptr)
throw TiFlashException(fmt::format("miss table in TiFlash : {}", name_mapper.debugCanonicalName(*npt_db_info, *table_info)),
throw TiFlashException(fmt::format("miss table in TiFlash, npt_table_id={} : {}", npt_table_id, name_mapper.debugCanonicalName(*npt_db_info, *table_info)),
Errors::DDL::MissingTable);
orig_table_info = storage->getTableInfo();
orig_table_info.belonging_table_id = pt_table_info;
auto orig_table_info = storage->getTableInfo();
orig_table_info.belonging_table_id = pt_table_id;
orig_table_info.is_partition_table = true;
/// partition does not have explicit name, so use default name here
orig_table_info.name = name_mapper.mapTableName(orig_table_info);
Expand All @@ -858,11 +863,14 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
/// step 3 change partition of the partition table to non partition table
table_info = getter.getTableInfo(npt_db_info->id, pt_partition_id);
if (table_info == nullptr)
throw TiFlashException(fmt::format("miss table in TiKV : {}", pt_partition_id), Errors::DDL::StaleSchema);
storage = tmt_context.getStorages().get(keyspace_id, table_info->id);
{
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);
throw TiFlashException(fmt::format("miss partition table in TiKV, may have been dropped, physical_table_id={}", pt_partition_id), Errors::DDL::StaleSchema);
}
storage = tmt_context.getStorages().get(keyspace_id, pt_partition_id);
if (storage == nullptr)
throw TiFlashException(
fmt::format("miss table in TiFlash : {}", name_mapper.debugCanonicalName(*pt_db_info, *table_info)),
fmt::format("miss partition table in TiFlash, physical_table_id={}", pt_partition_id),
Errors::DDL::MissingTable);
orig_table_info = storage->getTableInfo();
orig_table_info.belonging_table_id = DB::InvalidTableID;
Expand All @@ -883,6 +891,7 @@ void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const Schema
if (npt_db_info->id != pt_db_info->id)
applyRenamePhysicalTable(npt_db_info, orig_table_info, storage);
FAIL_POINT_TRIGGER_EXCEPTION(FailPoints::exception_after_step_3_in_exchange_partition);
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);
}

template <typename Getter, typename NameMapper>
Expand Down Expand Up @@ -1354,7 +1363,7 @@ void SchemaBuilder<Getter, NameMapper>::syncAllSchema()
if (table->isLogicalPartitionTable())
{
/// Apply partition diff if needed.
applyPartitionDiff(db, table, storage);
applyPartitionDiff(db, table, storage, /*drop_part_if_not_exist*/ true);
}
/// Rename if needed.
applyRenameLogicalTable(db, table, storage);
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/TiDB/Schema/SchemaBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ struct SchemaBuilder

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

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

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

Expand Down
98 changes: 98 additions & 0 deletions tests/fullstack-test2/ddl/alter_exchange_partition.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,104 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new
mysql> alter table test.e drop column c1;
>> DBGInvoke __refresh_schemas()

# cleanup
mysql> drop table if exists test.e;
mysql> drop table if exists test.e2;
mysql> drop table if exists test_new.e2;
mysql> drop database if exists test_new;

# case 11, create non-partition table and execute exchagne partition immediately
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));
mysql> insert into test.e values (1, 'a', 'b'),(108, 'a', 'b');
# sync the partition table to tiflash
>> DBGInvoke __refresh_schemas()

mysql> create table test.e2(id int not null,fname varchar(30),lname varchar(30));
mysql> insert into test.e2 values (2, 'a', 'b');
mysql> set @@tidb_enable_exchange_partition=1; alter table test.e exchange partition p0 with table test.e2

mysql> alter table test.e set tiflash replica 1;
mysql> alter table test.e2 set tiflash replica 1;
func> wait_table test e e2
>> DBGInvoke __refresh_schemas()
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 2 | a | b |
| 108 | a | b |
+-----+-------+-------+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 1 | a | b |
+-----+-------+-------+

# ensure the swap out table is not mark as tombstone
>> DBGInvoke __enable_schema_sync_service('true')
>> DBGInvoke __gc_schemas(18446744073709551615)
>> DBGInvoke __enable_schema_sync_service('false')
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 2 | a | b |
| 108 | a | b |
+-----+-------+-------+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 1 | a | b |
+-----+-------+-------+

# case 12, create partition table, non-partition table and execute exchagne partition immediately
mysql> drop table if exists test.e
mysql> drop table if exists test.e2
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));
mysql> insert into test.e values (1, 'a', 'b'),(108, 'a', 'b');
mysql> create table test.e2(id int not null,fname varchar(30),lname varchar(30));
mysql> insert into test.e2 values (2, 'a', 'b');
mysql> set @@tidb_enable_exchange_partition=1; alter table test.e exchange partition p0 with table test.e2

mysql> alter table test.e set tiflash replica 1;
mysql> alter table test.e2 set tiflash replica 1;
func> wait_table test e e2
# tiflash the final result
>> DBGInvoke __refresh_schemas()
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 2 | a | b |
| 108 | a | b |
+-----+-------+-------+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 1 | a | b |
+-----+-------+-------+
# ensure the swap out table is not mark as tombstone
>> DBGInvoke __enable_schema_sync_service('true')
>> DBGInvoke __gc_schemas(18446744073709551615)
>> DBGInvoke __enable_schema_sync_service('false')
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 2 | a | b |
| 108 | a | b |
+-----+-------+-------+
mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.e2 order by id;
+-----+-------+-------+
| id | fname | lname |
+-----+-------+-------+
| 1 | a | b |
+-----+-------+-------+

# cleanup
mysql> drop table if exists test.e;
mysql> drop table if exists test.e2;
mysql> drop table if exists test_new.e2;
Expand Down

0 comments on commit 4d7b3f6

Please sign in to comment.