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.

Sign up for GitHub

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 failure on executing exchange partition(release-6.1) #8376

Merged
merged 3 commits into from
Nov 16, 2023
Merged

ddl: Fix failure on executing exchange partition(release-6.1) #8376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3445958
Add fullstack test case
JaySon-Huang Nov 15, 2023
6e9d65f
Fix bug in exchange partition
JaySon-Huang Nov 15, 2023
99d0a36
Suppress a verbose logging
JaySon-Huang Nov 15, 2023
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
2 changes: 1 addition & 1 deletion dbms/src/Encryption/RateLimiter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ IOLimitTuner::TuneResult IOLimitTuner::tune() const
auto msg = fmt::format("limiter {} write {} read {}", limiterCount(), writeLimiterCount(), readLimiterCount());
if (limiterCount() < 2)
{
LOG_FMT_INFO(log, "{} NOT need to tune.", msg);
LOG_FMT_TRACE(log, "{} NOT need to tune.", msg);
return {0, 0, false, 0, 0, false};
}
LOG_FMT_INFO(log, "{} need to tune.", msg);
Expand Down
75 changes: 42 additions & 33 deletions dbms/src/Storages/Transaction/SchemaBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,11 @@ void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(TiDB::DBInfoPtr db_in
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(TiDB::DBInfoPtr db_info, TableInfoPtr table_info, ManageableStoragePtr storage)
void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(TiDB::DBInfoPtr db_info, TableInfoPtr table_info, ManageableStoragePtr storage, bool drop_part_if_not_exist)
{
const auto & orig_table_info = storage->getTableInfo();
if (!orig_table_info.isLogicalPartitionTable())
Expand Down Expand Up @@ -627,13 +627,17 @@ void SchemaBuilder<Getter, NameMapper>::applyPartitionDiff(TiDB::DBInfoPtr db_in
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 @@ -744,7 +748,7 @@ template <typename Getter, typename NameMapper>
void SchemaBuilder<Getter, NameMapper>::applyExchangeTablePartition(const SchemaDiff & diff)
{
/// 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 @@ -755,48 +759,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_FMT_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(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_FMT_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(getThreadName());
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(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 @@ -819,11 +824,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(table_info->id);
{
LOG_FMT_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(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 @@ -844,6 +852,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_FMT_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 @@ -1294,7 +1303,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/Storages/Transaction/SchemaBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ struct SchemaBuilder

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

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

void applyAlterTable(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
Open in desktop
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