ddl: Skip updating the tombstone_ts if the table is already tombstone (release-7.1) #9231

JaySon-Huang · 2024-07-12T08:54:25Z

What problem does this PR solve?

Issue Number: close #9227

Problem Summary: #9227 (comment)
In SchemaBuilder::syncAllSchema, each table that does not exists in tikv but still in the tiflash instance. TiFlash will execute applyDropPhysicalTable for each table, wich will update the "tombstone_ts" of the table in its ".sql" file.
Even if the table is already "tombstone", the function will update the "tombstone_ts" by a newer tso from PD. This make the IStorage instance still exist even after the gc_safepoint is exceed than the drop timepoint + gc_lifetime.

tiflash/dbms/src/TiDB/Schema/SchemaBuilder.cpp

Lines 1437 to 1451 in 28a2314

    
           /// Drop all unmapped tables. 
        
           auto storage_map = tmt_context.getStorages().getAllStorage(); 
        
           for (auto it = storage_map.begin(); it != storage_map.end(); it++) 
        
           { 
        
               auto table_info = it->second->getTableInfo(); 
        
               if (table_info.keyspace_id != keyspace_id) 
        
               { 
        
                   continue; 
        
               } 
        
               if (table_set.count(table_info.id) == 0) 
        
               { 
        
                   applyDropPhysicalTable(it->second->getDatabaseName(), table_info.id); 
        
                   LOG_INFO(log, "Table {}.{} dropped during sync all schemas", it->second->getDatabaseName(), name_mapper.debugTableName(table_info)); 
        
               } 
        
           }

tiflash/dbms/src/TiDB/Schema/SchemaBuilder.cpp

Lines 1240 to 1268 in 28a2314

    
           template <typename Getter, typename NameMapper> 
        
           void SchemaBuilder<Getter, NameMapper>::applyDropPhysicalTable(const String & db_name, TableID table_id) 
        
           { 
        
               auto & tmt_context = context.getTMTContext(); 
        
               auto storage = tmt_context.getStorages().get(keyspace_id, table_id); 
        
               if (storage == nullptr) 
        
               { 
        
                   LOG_DEBUG(log, "table {} does not exist.", table_id); 
        
                   return; 
        
               } 
        
               GET_METRIC(tiflash_schema_internal_ddl_count, type_drop_table).Increment(); 
        
               LOG_INFO(log, "Tombstoning table {}.{}", db_name, name_mapper.debugTableName(storage->getTableInfo())); 
        
               const UInt64 tombstone_ts = PDClientHelper::getTSO(tmt_context.getPDClient(), PDClientHelper::get_tso_maxtime); 
        
               AlterCommands commands; 
        
               { 
        
                   AlterCommand command; 
        
                   command.type = AlterCommand::TOMBSTONE; 
        
                   // We don't try to get a precise time that TiDB drops this table. 
        
                   // We use a more relaxing GC strategy: 
        
                   // 1. Use current timestamp, which is after TiDB's drop time, to be the tombstone of this table; 
        
                   // 2. Use the same GC safe point as TiDB. 
        
                   // In such way our table will be GC-ed later than TiDB, which is safe and correct. 
        
                   command.tombstone = tombstone_ts; 
        
                   commands.emplace_back(std::move(command)); 
        
               } 
        
               auto alter_lock = storage->lockForAlter(getThreadNameAndID()); 
        
               storage->alterFromTiDB(alter_lock, commands, db_name, storage->getTableInfo(), name_mapper, context); 
        
               LOG_INFO(log, "Tombstoned table {}.{}, tombstone={}", db_name, name_mapper.debugTableName(storage->getTableInfo()), tombstone_ts); 
        
           }

What is changed and how it works?

Skip updating the tombstone_ts if the table is already tombstone. So even if the "syncAllSchema" is called repeatedly, the dropped IStorage instance can be physically removed from TiFlash instances correctly. And the time spend on "syncAllSchema" won't become longer and longer.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
Run the following test scripts. It will make tiflash run into sync all schema frequently.

>  cat test.sh
set -x

for (( i=0 ; i<1000 ; i++ )); do
echo "running with i=${i}"
mycli -h 10.2.12.81 -P 8042 -u root -D test -e 'CREATE TABLE if not exists test_range_1 (id INT,name VARCHAR(20)) PARTITION BY RANGE (id) (PARTITION p0 VALUES LESS THAN (10),PARTITION p1 VALUES LESS THAN (20),PARTITION p2 VALUES LESS THAN (30),PARTITION p3 VALUES LESS THAN (40),PARTITION p4 VALUES LESS THAN (50));'

mycli -h 10.2.12.81 -P 8042 -u root -D test -e 'CREATE TABLE if not exists test_swap_1 (id INT,name VARCHAR(20));'
mycli -h 10.2.12.81 -P 8042 -u root -D test -e 'ALTER TABLE test_range_1 EXCHANGE PARTITION p0 WITH TABLE test_swap_1;'
mycli -h 10.2.12.81 -P 8042 -u root -D test -e 'DROP TABLE test_swap_1;'
done

In the v6.5.10 version, after that script ran,

## At the beginning, sync all schema taks about 38ms
[2024/09/02 14:53:02.454 +08:00] [INFO] [TiDBSchemaSyncer.h:124] ["Start to sync schemas. current version is: 589 and try to sync schema version to: 600"] [thread_id=85]
[2024/09/02 14:53:02.455 +08:00] [DEBUG] [TiDBSchemaSyncer.h:181] ["Try load schema diffs."] [thread_id=85]
[2024/09/02 14:53:02.457 +08:00] [DEBUG] [TiDBSchemaSyncer.h:192] ["End load schema diffs with total 11 entries."] [thread_id=85]
[2024/09/02 14:53:02.460 +08:00] [WARN] [TiDBSchemaSyncer.h:236] ["apply diff meets exception : DB::TiFlashException: miss partition table in TiKV, may have been dropped, physical_table_id=446 \n stack is \n       0x1779921\tDB::TiFlashException::TiFlashException(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, DB::TiFlashError const&) [tiflash+24615201]\n                \tdbms/src/Common/TiFlashException.h:250\n       0x6cac991\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyExchangeTablePartition(DB::SchemaDiff const&) [tiflash+113953169]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:908\n       0x6ca9123\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyDiff(DB::SchemaDiff const&) [tiflash+113938723]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:551\n       0x6bf165b\tDB::TiDBSchemaSyncer<false, false>::syncSchemas(DB::Context&) [tiflash+113186395]\n                \tdbms/src/TiDB/Schema/TiDBSchemaSyncer.h:139\n       0x6cf1ff4\tstd::__1::__function::__func<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0, std::__1::allocator<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0>, bool ()>::operator()() [tiflash+114237428]\n                \t/usr/local/bin/../include/c++/v1/__functional/function.h:345\n       0x68e7fbc\tvoid* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, DB::BackgroundProcessingPool::BackgroundProcessingPool(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)::$_1> >(void*) [tiflash+110002108]\n                \t/usr/local/bin/../include/c++/v1/thread:291\n  0x7f415ae7d802\tstart_thread [libc.so.6+653314]\n  0x7f415ae1d450\tclone3 [libc.so.6+259152]"] [thread_id=85]
[2024/09/02 14:53:02.492 +08:00] [INFO] [TiDBSchemaSyncer.h:146] ["End sync schema, version has been updated to 600"] [thread_id=85]

## the last sync all schema takes about 300ms (to update about 429 tables)
238043:[2024/09/02 15:29:05.265 +08:00] [INFO] [TiDBSchemaSyncer.h:124] ["Start to sync schemas. current version is: 6576 and try to sync schema version to: 6606"] [thread_id=84]
238044:[2024/09/02 15:29:05.265 +08:00] [DEBUG] [TiDBSchemaSyncer.h:181] ["Try load schema diffs."] [thread_id=84]
238045:[2024/09/02 15:29:05.270 +08:00] [DEBUG] [TiDBSchemaSyncer.h:192] ["End load schema diffs with total 30 entries."] [thread_id=84]
238065:[2024/09/02 15:29:05.274 +08:00] [WARN] [TiDBSchemaSyncer.h:236] ["apply diff meets exception : DB::TiFlashException: miss table in TiFlash, npt_table_id=4444 : test(2).test_range_1(83) \n stack is \n       0x1779921\tDB::TiFlashException::TiFlashException(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, DB::TiFlashError const&) [tiflash+24615201]\n                \tdbms/src/Common/TiFlashException.h:250\n       0x6cac822\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyExchangeTablePartition(DB::SchemaDiff const&) [tiflash+113952802]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:880\n       0x6ca9123\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyDiff(DB::SchemaDiff const&) [tiflash+113938723]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:551\n       0x6bf165b\tDB::TiDBSchemaSyncer<false, false>::syncSchemas(DB::Context&) [tiflash+113186395]\n                \tdbms/src/TiDB/Schema/TiDBSchemaSyncer.h:139\n       0x6cf1ff4\tstd::__1::__function::__func<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0, std::__1::allocator<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0>, bool ()>::operator()() [tiflash+114237428]\n                \t/usr/local/bin/../include/c++/v1/__functional/function.h:345\n       0x68e7fbc\tvoid* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, DB::BackgroundProcessingPool::BackgroundProcessingPool(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)::$_1> >(void*) [tiflash+110002108]\n                \t/usr/local/bin/../include/c++/v1/thread:291\n  0x7f415ae7d802\tstart_thread [libc.so.6+653314]\n  0x7f415ae1d450\tclone3 [libc.so.6+259152]"] [thread_id=84]
239987:[2024/09/02 15:29:05.565 +08:00] [INFO] [TiDBSchemaSyncer.h:146] ["End sync schema, version has been updated to 6606"] [thread_id=84]

## And the same table_id is being tombstone again and again
>  rg 'Tombstoned table db_2.t_94\(94\)' clusters/tiflash-5040/log/tiflash.log                                                                                                                                                                                                                                                                        2024-09-02 16:51:01
1309:[2024/09/02 14:46:29.519 +08:00] [INFO] [SchemaBuilder.cpp:1298] ["Tombstoned table db_2.t_94(94), tombstone=452266449829888006"] [thread_id=91]
1570:[2024/09/02 14:46:40.143 +08:00] [INFO] [SchemaBuilder.cpp:1298] ["Tombstoned table db_2.t_94(94), tombstone=452266452608614402"] [thread_id=92]
...
236355:[2024/09/02 15:28:54.154 +08:00] [INFO] [SchemaBuilder.cpp:1298] ["Tombstoned table db_2.t_94(94), tombstone=452267116894617606"] [thread_id=81]
238316:[2024/09/02 15:29:05.296 +08:00] [INFO] [SchemaBuilder.cpp:1298] ["Tombstoned table db_2.t_94(94), tombstone=452267119804416006"] [thread_id=84]

## At the beginning, sync all schema taks about 32ms
[2024/09/02 16:47:01.052 +08:00] [INFO] [TiDBSchemaSyncer.h:124] ["Start to sync schemas. current version is: 6626 and try to sync schema version to: 6658"] [thread_id=83]
[2024/09/02 16:47:01.052 +08:00] [DEBUG] [TiDBSchemaSyncer.h:181] ["Try load schema diffs."] [thread_id=83]
[2024/09/02 16:47:01.058 +08:00] [DEBUG] [TiDBSchemaSyncer.h:192] ["End load schema diffs with total 32 entries."] [thread_id=83]
[2024/09/02 16:47:01.061 +08:00] [WARN] [TiDBSchemaSyncer.h:236] ["apply diff meets exception : DB::TiFlashException: miss partition table in TiKV, may have been dropped, physical_table_id=4472 \n stack is \n       0x1752003\tStackTrace::StackTrace() [tiflash+24453123]\n                \tdbms/src/Common/StackTrace.cpp:23\n       0x177a67b\tDB::TiFlashException::TiFlashException(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, DB::TiFlashError const&) [tiflash+24618619]\n                \tdbms/src/Common/TiFlashException.h:250\n       0x6c7aaa1\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyExchangeTablePartition(DB::SchemaDiff const&) [tiflash+113748641]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:923\n       0x6c7790f\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyDiff(DB::SchemaDiff const&) [tiflash+113735951]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:551\n       0x6bb98fb\tDB::TiDBSchemaSyncer<false, false>::tryLoadSchemaDiffs(DB::SchemaGetter&, long, DB::Context&) [tiflash+112957691]\n                \tdbms/src/TiDB/Schema/TiDBSchemaSyncer.h:227\n       0x6bb8a85\tDB::TiDBSchemaSyncer<false, false>::syncSchemas(DB::Context&) [tiflash+112953989]\n                \tdbms/src/TiDB/Schema/TiDBSchemaSyncer.h:139\n       0x6cb1a3e\tstd::__1::__function::__func<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0, std::__1::allocator<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0>, bool ()>::operator()() [tiflash+113973822]\n                \t/DATA/disk1/ra_common/tiflash-env-13/sysroot/bin/../include/c++/v1/__functional/function.h:345\n       0x6895d5f\tDB::BackgroundProcessingPool::threadFunction(unsigned long) [tiflash+109665631]\n                \tdbms/src/Storages/BackgroundProcessingPool.cpp:226\n       0x6896655\tvoid* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, DB::BackgroundProcessingPool::BackgroundProcessingPool(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)::$_1> >(void*) [tiflash+109667925]\n                \t/DATA/disk1/ra_common/tiflash-env-13/sysroot/bin/../include/c++/v1/thread:291\n  0x7f3b92a52802\tstart_thread [libc.so.6+653314]\n  0x7f3b929f2450\tclone3 [libc.so.6+259152]"] [thread_id=83]
[2024/09/02 16:47:01.084 +08:00] [INFO] [TiDBSchemaSyncer.h:146] ["End sync schema, version has been updated to 6658"] [thread_id=83]

## the last sync all schema takes about 38ms
[2024/09/02 17:22:10.555 +08:00] [INFO] [TiDBSchemaSyncer.h:124] ["Start to sync schemas. current version is: 12585 and try to sync schema version to: 12606"] [thread_id=95]
[2024/09/02 17:22:10.555 +08:00] [DEBUG] [TiDBSchemaSyncer.h:181] ["Try load schema diffs."] [thread_id=95]
[2024/09/02 17:22:10.560 +08:00] [DEBUG] [TiDBSchemaSyncer.h:192] ["End load schema diffs with total 21 entries."] [thread_id=95]
[2024/09/02 17:22:10.566 +08:00] [WARN] [TiDBSchemaSyncer.h:236] ["apply diff meets exception : DB::TiFlashException: miss table in TiFlash, npt_table_id=8452 : test(2).test_range_1(83) \n stack is \n       0x1752003\tStackTrace::StackTrace() [tiflash+24453123]\n                \tdbms/src/Common/StackTrace.cpp:23\n       0x177a67b\tDB::TiFlashException::TiFlashException(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, DB::TiFlashError const&) [tiflash+24618619]\n                \tdbms/src/Common/TiFlashException.h:250\n       0x6c7a9a3\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyExchangeTablePartition(DB::SchemaDiff const&) [tiflash+113748387]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:895\n       0x6c7790f\tDB::SchemaBuilder<DB::SchemaGetter, DB::SchemaNameMapper>::applyDiff(DB::SchemaDiff const&) [tiflash+113735951]\n                \tdbms/src/TiDB/Schema/SchemaBuilder.cpp:551\n       0x6bb98fb\tDB::TiDBSchemaSyncer<false, false>::tryLoadSchemaDiffs(DB::SchemaGetter&, long, DB::Context&) [tiflash+112957691]\n                \tdbms/src/TiDB/Schema/TiDBSchemaSyncer.h:227\n       0x6bb8a85\tDB::TiDBSchemaSyncer<false, false>::syncSchemas(DB::Context&) [tiflash+112953989]\n                \tdbms/src/TiDB/Schema/TiDBSchemaSyncer.h:139\n       0x6cb1a3e\tstd::__1::__function::__func<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0, std::__1::allocator<DB::SchemaSyncService::SchemaSyncService(DB::Context&)::$_0>, bool ()>::operator()() [tiflash+113973822]\n                \t/DATA/disk1/ra_common/tiflash-env-13/sysroot/bin/../include/c++/v1/__functional/function.h:345\n       0x6895d5f\tDB::BackgroundProcessingPool::threadFunction(unsigned long) [tiflash+109665631]\n                \tdbms/src/Storages/BackgroundProcessingPool.cpp:226\n       0x6896655\tvoid* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, DB::BackgroundProcessingPool::BackgroundProcessingPool(int, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)::$_1> >(void*) [tiflash+109667925]\n                \t/DATA/disk1/ra_common/tiflash-env-13/sysroot/bin/../include/c++/v1/thread:291\n  0x7f3b92a52802\tstart_thread [libc.so.6+653314]\n  0x7f3b929f2450\tclone3 [libc.so.6+259152]"] [thread_id=95]
[2024/09/02 17:22:10.593 +08:00] [INFO] [TiDBSchemaSyncer.h:146] ["End sync schema, version has been updated to 12606"] [thread_id=95]

## The same table_id only be dropped once
[2024/09/02 16:46:50.264 +08:00] [INFO] [SchemaBuilder.cpp:1343] ["Tombstone table db_2.t_4460(4460) begin, action=SyncAllSchema"] [thread_id=86]
[2024/09/02 16:46:50.264 +08:00] [INFO] [StorageDeltaMerge.cpp:115] ["updateTableColumnInfo: TableName t_4460 ordinary columns format version: 1\n3 columns:\n`id` Nullable(Int32)\n`name` Nullable(String)\n`_tidb_rowid` Int64\n materialized columns format version: 1\n2 columns:\n`_INTERNAL_VERSION` UInt64\n`_INTERNAL_DELMARK` UInt8\n"] [source=db_2.t_4460] [thr
ead_id=86]
[2024/09/02 16:46:50.265 +08:00] [INFO] [SchemaBuilder.cpp:1359] ["Tombstone table db_2.t_4460(4460) end, action=SyncAllSchema tombstone=452268342706176001"] [thread_id=86]
[2024/09/02 16:46:50.265 +08:00] [INFO] [SchemaBuilder.cpp:1542] ["Table db_2.t_4460(4460) dropped during sync all schemas"] [thread_id=86]
[2024/09/02 16:47:01.084 +08:00] [INFO] [SchemaBuilder.cpp:1337] ["Tombstone table db_2.t_4460(4460) has been done before, action=SyncAllSchema tombstone=452268342706176001"] [thread_id=83]
[2024/09/02 16:47:01.084 +08:00] [INFO] [SchemaBuilder.cpp:1542] ["Table db_2.t_4460(4460) dropped during sync all schemas"] [thread_id=83]
[2024/09/02 16:47:12.040 +08:00] [INFO] [SchemaBuilder.cpp:1337] ["Tombstone table db_2.t_4460(4460) has been done before, action=SyncAllSchema tombstone=452268342706176001"] [thread_id=84]
...

No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Fix an issue that rapidly executes `DROP TABLE` after other DDL operations on the same table for a long time, could potentially lead to a significant slowdown in TiFlash schema synchronization.

Signed-off-by: JaySon-Huang <tshent@qq.com>

JaySon-Huang · 2024-07-15T06:37:59Z

/run-all-tests

JaySon-Huang · 2024-09-03T04:30:08Z

/run-all-tests

ti-chi-bot · 2024-09-03T06:04:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JinheLin, Lloyd-Pottiger

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [JinheLin,Lloyd-Pottiger]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-09-03T06:04:15Z

[LGTM Timeline notifier]

Timeline:

2024-09-03 03:16:16.179454942 +0000 UTC m=+327900.697507867: ☑️ agreed by Lloyd-Pottiger.
2024-09-03 06:04:14.418608712 +0000 UTC m=+337978.936661721: ☑️ agreed by JinheLin.

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/cherry-pick-not-approved size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 12, 2024

JaySon-Huang marked this pull request as draft July 12, 2024 08:59

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 12, 2024

JaySon-Huang added 2 commits July 15, 2024 14:13

Add ut

2047ad4

Skip updating the tombstone_ts if the table is already tombstone.

a2e4d39

Signed-off-by: JaySon-Huang <tshent@qq.com>

JaySon-Huang force-pushed the tombstone_repeat_71 branch from 63e72c4 to a2e4d39 Compare July 15, 2024 06:15

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 15, 2024

JaySon-Huang added 3 commits July 15, 2024 14:21

Refine logging

952c552

Signed-off-by: JaySon-Huang <tshent@qq.com>

Drop table schema diff should update the tombstone

458ad9e

Refine logging about drop all schema

4a191a8

JaySon-Huang marked this pull request as ready for review July 15, 2024 06:40

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2024

JaySon-Huang added 2 commits September 2, 2024 18:24

Merge branch 'release-7.1' into tombstone_repeat_71

c03c300

Add comments

3772af1

JaySon-Huang requested review from CalvinNeo, JinheLin and Lloyd-Pottiger September 2, 2024 10:29

JaySon-Huang self-assigned this Sep 2, 2024

Lloyd-Pottiger approved these changes Sep 3, 2024

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Sep 3, 2024

Align with master

bea5699

ti-chi-bot bot added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Sep 3, 2024

JinheLin approved these changes Sep 3, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 3, 2024

ti-chi-bot bot merged commit 45ef5ed into pingcap:release-7.1 Sep 3, 2024
5 checks passed

JaySon-Huang deleted the tombstone_repeat_71 branch September 3, 2024 06:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: Skip updating the tombstone_ts if the table is already tombstone (release-7.1) #9231

ddl: Skip updating the tombstone_ts if the table is already tombstone (release-7.1) #9231

JaySon-Huang commented Jul 12, 2024 •

edited

Loading

JaySon-Huang commented Jul 15, 2024

JaySon-Huang commented Sep 3, 2024

ti-chi-bot bot commented Sep 3, 2024

ti-chi-bot bot commented Sep 3, 2024

	/// Drop all unmapped tables.
	auto storage_map = tmt_context.getStorages().getAllStorage();
	for (auto it = storage_map.begin(); it != storage_map.end(); it++)
	{
	auto table_info = it->second->getTableInfo();
	if (table_info.keyspace_id != keyspace_id)
	{
	continue;
	}
	if (table_set.count(table_info.id) == 0)
	{
	applyDropPhysicalTable(it->second->getDatabaseName(), table_info.id);
	LOG_INFO(log, "Table {}.{} dropped during sync all schemas", it->second->getDatabaseName(), name_mapper.debugTableName(table_info));
	}
	}

	template <typename Getter, typename NameMapper>
	void SchemaBuilder<Getter, NameMapper>::applyDropPhysicalTable(const String & db_name, TableID table_id)
	{
	auto & tmt_context = context.getTMTContext();
	auto storage = tmt_context.getStorages().get(keyspace_id, table_id);
	if (storage == nullptr)
	{
	LOG_DEBUG(log, "table {} does not exist.", table_id);
	return;
	}
	GET_METRIC(tiflash_schema_internal_ddl_count, type_drop_table).Increment();
	LOG_INFO(log, "Tombstoning table {}.{}", db_name, name_mapper.debugTableName(storage->getTableInfo()));
	const UInt64 tombstone_ts = PDClientHelper::getTSO(tmt_context.getPDClient(), PDClientHelper::get_tso_maxtime);
	AlterCommands commands;
	{
	AlterCommand command;
	command.type = AlterCommand::TOMBSTONE;
	// We don't try to get a precise time that TiDB drops this table.
	// We use a more relaxing GC strategy:
	// 1. Use current timestamp, which is after TiDB's drop time, to be the tombstone of this table;
	// 2. Use the same GC safe point as TiDB.
	// In such way our table will be GC-ed later than TiDB, which is safe and correct.
	command.tombstone = tombstone_ts;
	commands.emplace_back(std::move(command));
	}
	auto alter_lock = storage->lockForAlter(getThreadNameAndID());
	storage->alterFromTiDB(alter_lock, commands, db_name, storage->getTableInfo(), name_mapper, context);
	LOG_INFO(log, "Tombstoned table {}.{}, tombstone={}", db_name, name_mapper.debugTableName(storage->getTableInfo()), tombstone_ts);
	}

ddl: Skip updating the tombstone_ts if the table is already tombstone (release-7.1) #9231

ddl: Skip updating the tombstone_ts if the table is already tombstone (release-7.1) #9231

Conversation

JaySon-Huang commented Jul 12, 2024 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

JaySon-Huang commented Jul 15, 2024

JaySon-Huang commented Sep 3, 2024

ti-chi-bot bot commented Sep 3, 2024

ti-chi-bot bot commented Sep 3, 2024

[LGTM Timeline notifier]

JaySon-Huang commented Jul 12, 2024 •

edited

Loading