From 21e59f2bb3744f0856e0064d73ea7f018372651b Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Sun, 26 Nov 2023 20:49:45 +0800 Subject: [PATCH 01/10] Support FLASHBACK DATABASE ... --- dbms/src/Databases/DatabaseTiFlash.cpp | 19 +++++++- dbms/src/Databases/DatabaseTiFlash.h | 2 +- dbms/src/Databases/IDatabase.h | 11 ++++- dbms/src/Databases/test/gtest_database.cpp | 11 +++-- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 47 ++++++++++++++++++- dbms/src/TiDB/Schema/SchemaBuilder.h | 4 +- dbms/src/TiDB/Schema/SchemaGetter.cpp | 35 +++++++++++++- .../ddl/alter_drop_database.test | 27 +++++++++-- .../ddl/partitions/reorganize_partition.test | 2 +- 9 files changed, 141 insertions(+), 17 deletions(-) diff --git a/dbms/src/Databases/DatabaseTiFlash.cpp b/dbms/src/Databases/DatabaseTiFlash.cpp index f7b300e1efc..db80683e212 100644 --- a/dbms/src/Databases/DatabaseTiFlash.cpp +++ b/dbms/src/Databases/DatabaseTiFlash.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include namespace DB @@ -610,7 +611,7 @@ void DatabaseTiFlash::shutdown() tables.clear(); } -void DatabaseTiFlash::alterTombstone(const Context & context, Timestamp tombstone_) +void DatabaseTiFlash::alterTombstone(const Context & context, Timestamp tombstone_, const TiDB::DBInfoPtr & new_db_info) { const auto database_metadata_path = getDatabaseMetadataPath(metadata_path); const auto database_metadata_tmp_path = database_metadata_path + ".tmp"; @@ -622,7 +623,18 @@ void DatabaseTiFlash::alterTombstone(const Context & context, Timestamp tombston { // Alter the attach statement in metadata. - auto dbinfo_literal = std::make_shared(Field(db_info == nullptr ? "" : (db_info->serialize()))); + std::shared_ptr dbinfo_literal = [&]() { + String seri_info; + if (new_db_info != nullptr) + { + seri_info = new_db_info->serialize(); + } + else if (db_info != nullptr) + { + seri_info = db_info->serialize(); + } + return std::make_shared(Field(seri_info)); + }(); Field format_version_field(static_cast(DatabaseTiFlash::CURRENT_VERSION)); auto version_literal = std::make_shared(format_version_field); auto tombstone_literal = std::make_shared(Field(tombstone_)); @@ -651,6 +663,9 @@ void DatabaseTiFlash::alterTombstone(const Context & context, Timestamp tombston } else { + // update the seri dbinfo + args.children[0] = dbinfo_literal; + args.children[1] = version_literal; // udpate the tombstone mark args.children[2] = tombstone_literal; } diff --git a/dbms/src/Databases/DatabaseTiFlash.h b/dbms/src/Databases/DatabaseTiFlash.h index 178e7e00c39..51f446fd2ae 100644 --- a/dbms/src/Databases/DatabaseTiFlash.h +++ b/dbms/src/Databases/DatabaseTiFlash.h @@ -89,7 +89,7 @@ class DatabaseTiFlash : public DatabaseWithOwnTablesBase bool isTombstone() const override { return tombstone != 0; } Timestamp getTombstone() const override { return tombstone; } - void alterTombstone(const Context & context, Timestamp tombstone_) override; + void alterTombstone(const Context & context, Timestamp tombstone_, const TiDB::DBInfoPtr & new_db_info) override; void drop(const Context & context) override; diff --git a/dbms/src/Databases/IDatabase.h b/dbms/src/Databases/IDatabase.h index 15fdce40c40..d6414c7a924 100644 --- a/dbms/src/Databases/IDatabase.h +++ b/dbms/src/Databases/IDatabase.h @@ -24,6 +24,11 @@ #include #include +namespace TiDB +{ +struct DBInfo; +using DBInfoPtr = std::shared_ptr; +} // namespace TiDB namespace DB { @@ -144,7 +149,11 @@ class IDatabase : public std::enable_shared_from_this virtual bool isTombstone() const { return false; } virtual Timestamp getTombstone() const { return 0; } - virtual void alterTombstone(const Context & /*context*/, Timestamp /*tombstone_*/) {} + virtual void alterTombstone( + const Context & /*context*/, + Timestamp /*tombstone_*/, + const TiDB::DBInfoPtr & /*new_db_info*/) + {} /// Delete metadata, the deletion of which differs from the recursive deletion of the directory, if any. virtual void drop(const Context & context) = 0; diff --git a/dbms/src/Databases/test/gtest_database.cpp b/dbms/src/Databases/test/gtest_database.cpp index 9a009874891..7ff716fc458 100644 --- a/dbms/src/Databases/test/gtest_database.cpp +++ b/dbms/src/Databases/test/gtest_database.cpp @@ -28,10 +28,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include @@ -968,7 +970,7 @@ try LOG_DEBUG(log, "After create [meta={}]", meta); DB::Timestamp tso = 1000; - db->alterTombstone(*ctx, tso); + db->alterTombstone(*ctx, tso, nullptr); EXPECT_TRUE(db->isTombstone()); EXPECT_EQ(db->getTombstone(), tso); @@ -977,8 +979,11 @@ try EXPECT_TRUE(db->isTombstone()); EXPECT_EQ(db->getTombstone(), tso); - // Recover - db->alterTombstone(*ctx, 0); + // Recover, usually recover with a new database name + auto new_db_info = std::make_shared( + R"json({"charset":"utf8mb4","collate":"utf8mb4_bin","db_name":{"L":"test_new_db","O":"test_db"},"id":1010,"state":5})json", + NullspaceID); + db->alterTombstone(*ctx, 0, new_db_info); EXPECT_FALSE(db->isTombstone()); // Try restore from disk diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index 816ca718c68..1957048552c 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -295,6 +295,11 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) applyRecoverTable(diff.schema_id, diff.table_id); break; } + case SchemaActionType::ActionRecoverSchema: + { + applyRecoverDatabase(diff.schema_id); + break; + } case SchemaActionType::DropTable: case SchemaActionType::DropView: { @@ -900,6 +905,44 @@ void SchemaBuilder::applyCreateSchema(const TiDB::DBInfoPtr LOG_INFO(log, "Create database {} end, database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id); } +template +void SchemaBuilder::applyRecoverDatabase(DatabaseID database_id) +{ + auto db_info = getter.getDatabase(database_id); + if (db_info == nullptr) + { + return; + } + LOG_INFO(log, "Recover database begin, database_id={}", database_id); + auto db_name = name_mapper.mapDatabaseName(database_id, keyspace_id); + auto db = context.tryGetDatabase(db_name); + if (!db) + { + LOG_INFO( + log, + "Recover database {} ignored because instance is not exists, may have been physically dropped, " + "database_id={}", + db_name, + database_id); + return; + } + + { + //TODO: it seems may need a lot time, maybe we can do it in a background thread + auto table_ids = table_id_map.findTablesByDatabaseID(database_id); + for (auto table_id : table_ids) + { + auto table_info = getter.getTableInfo(database_id, table_id); + applyRecoverLogicalTable(db_info, table_info); + } + } + + // Usually `FLASHBACK DATABASE ... TO ...` will rename the database + db->alterTombstone(context, 0, db_info); + databases.addDatabaseInfo(db_info); // add back database info cache + LOG_INFO(log, "Recover database end, database_id={}", database_id); +} + template void SchemaBuilder::applyDropSchema(DatabaseID schema_id) { @@ -917,7 +960,7 @@ void SchemaBuilder::applyDropSchema(DatabaseID schema_id) applyDropTable(schema_id, table_id); } - applyDropSchema(name_mapper.mapDatabaseName(*db_info)); + applyDropSchema(name_mapper.mapDatabaseName(schema_id, keyspace_id)); databases.eraseDBInfo(schema_id); } @@ -945,7 +988,7 @@ void SchemaBuilder::applyDropSchema(const String & db_name) // In such way our database (and its belonging tables) will be GC-ed later than TiDB, which is safe and correct. auto & tmt_context = context.getTMTContext(); auto tombstone = tmt_context.getPDClient()->getTS(); - db->alterTombstone(context, tombstone); + db->alterTombstone(context, tombstone, /*new_db_info*/ nullptr); // keep the old db_info LOG_INFO(log, "Tombstone database end, db_name={}", db_name); } diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.h b/dbms/src/TiDB/Schema/SchemaBuilder.h index 53207d8f320..20cdb154a6e 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.h +++ b/dbms/src/TiDB/Schema/SchemaBuilder.h @@ -65,14 +65,14 @@ struct SchemaBuilder private: void applyDropSchema(DatabaseID schema_id); - /// Parameter db_name should be mapped. void applyDropSchema(const String & db_name); bool applyCreateSchema(DatabaseID schema_id); - void applyCreateSchema(const TiDB::DBInfoPtr & db_info); + void applyRecoverDatabase(DatabaseID database_id); + void applyCreateStorageInstance( 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 1197fa3f043..703133bfef4 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.cpp +++ b/dbms/src/TiDB/Schema/SchemaGetter.cpp @@ -115,6 +115,7 @@ struct TxnStructure static String hGet(KeyspaceSnapshot & snap, const String & key, const String & field) { String encode_key = encodeHashDataKey(key, field); + // LOG_INFO(Logger::get(), "hGet, encode_key={}", encode_key); String value = snap.Get(encode_key); return value; } @@ -144,6 +145,24 @@ struct TxnStructure return target_value; } + static std::vector> mvccGetAll( + KeyspaceSnapshot & snap, + const String & key, + const String & field) + { + auto encode_key = encodeHashDataKey(key, field); + // LOG_INFO(Logger::get(), "mvccGetAll, encode_key={}", encode_key); + auto mvcc_info = snap.mvccGet(encode_key); + const auto & values = mvcc_info.values(); + + std::vector> res; + for (const auto & value_pair : values) + { + res.emplace_back(value_pair.value(), value_pair.start_ts()); + } + return res; + } + // For convinient, we only return values. static std::vector> hGetAll(KeyspaceSnapshot & snap, const String & key) { @@ -162,6 +181,19 @@ struct TxnStructure } return res; } + + static std::vector> scan(KeyspaceSnapshot & snap, const String & key) + { + auto tikv_key_end = pingcap::kv::prefixNext(key); + auto scanner = snap.Scan(key, tikv_key_end); + std::vector> res; + while (scanner.valid) + { + res.push_back(std::make_pair(scanner.key(), scanner.value())); + scanner.next(); + } + return res; + } }; AffectedOption::AffectedOption(Poco::JSON::Object::Ptr json) @@ -253,7 +285,7 @@ std::optional SchemaGetter::getSchemaDiff(Int64 ver) 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); + LOG_DEBUG(log, "Get SchemaDiff from TiKV, schema_version={} data={}", ver, data); SchemaDiff diff; diff.deserialize(data); return diff; @@ -273,7 +305,6 @@ TiDB::DBInfoPtr SchemaGetter::getDatabase(DatabaseID db_id) { String key = getDBKey(db_id); String json = TxnStructure::hGet(snap, DBs, key); - if (json.empty()) return nullptr; diff --git a/tests/fullstack-test2/ddl/alter_drop_database.test b/tests/fullstack-test2/ddl/alter_drop_database.test index 838d46afbcf..8484da77dfd 100644 --- a/tests/fullstack-test2/ddl/alter_drop_database.test +++ b/tests/fullstack-test2/ddl/alter_drop_database.test @@ -16,6 +16,7 @@ >> DBGInvoke __init_fail_point() mysql> drop database if exists d1; +mysql> drop database if exists d1_new; mysql> create database d1; => DBGInvoke __refresh_schemas() @@ -36,7 +37,7 @@ mysql> alter table d1.t1 add column b int; >> DBGInvoke __enable_fail_point(pause_before_apply_raft_cmd) # exactly write until fail point "pause_before_apply_raft_cmd" to be disable -mysql> insert into d1.t1 values(1,2); +mysql> insert into d1.t1 values(2,2); mysql> drop database d1; @@ -46,6 +47,26 @@ mysql> drop database d1; >> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd) # the `t1` is still mark as tombstone ->> select tidb_database,tidb_name from system.tables where is_tombstone = 0 and tidb_database = 'd1' and tidb_name='t1'; -#TODO: check the row is written to the storage or not +>> select tidb_database,tidb_name,is_tombstone,tidb_table_id from system.tables where is_tombstone = 0 and tidb_database = 'd1' and tidb_name='t1'; +# check the row is written to the storage or not +mysql> flashback database d1 to d1_new +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t1 order by a; ++------+------+ +| a | b | ++------+------+ +| 1 | NULL | +| 2 | 2 | ++------+------+ +=> DBGInvoke __refresh_schemas() +# ensure the flashbacked table and database 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 d1_new.t1 order by a; ++------+------+ +| a | b | ++------+------+ +| 1 | NULL | +| 2 | 2 | ++------+------+ diff --git a/tests/fullstack-test2/ddl/partitions/reorganize_partition.test b/tests/fullstack-test2/ddl/partitions/reorganize_partition.test index 7c208d843d8..47482e1c30a 100644 --- a/tests/fullstack-test2/ddl/partitions/reorganize_partition.test +++ b/tests/fullstack-test2/ddl/partitions/reorganize_partition.test @@ -185,4 +185,4 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test.t1 | 80 | aaa | 2 | +----+------+------+ -#mysql> drop table test.t1; +mysql> drop table test.t1; From e2ef50cf55450ba3bf4ee6a24bca34ec6e1b5a6a Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Sun, 26 Nov 2023 20:49:45 +0800 Subject: [PATCH 02/10] Add more test case about partition table --- .../ddl/alter_drop_database.test | 72 ------- .../ddl/flashback/flashback_database.test | 187 ++++++++++++++++++ 2 files changed, 187 insertions(+), 72 deletions(-) delete mode 100644 tests/fullstack-test2/ddl/alter_drop_database.test create mode 100644 tests/fullstack-test2/ddl/flashback/flashback_database.test diff --git a/tests/fullstack-test2/ddl/alter_drop_database.test b/tests/fullstack-test2/ddl/alter_drop_database.test deleted file mode 100644 index 8484da77dfd..00000000000 --- a/tests/fullstack-test2/ddl/alter_drop_database.test +++ /dev/null @@ -1,72 +0,0 @@ -# Copyright 2023 PingCAP, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -=> DBGInvoke __enable_schema_sync_service('false') ->> DBGInvoke __init_fail_point() - -mysql> drop database if exists d1; -mysql> drop database if exists d1_new; -mysql> create database d1; - -=> DBGInvoke __refresh_schemas() - -=> DBGInvoke mapped_database_exists(d1) -┌─mapped_database_exists(d1)───┐ -│ true │ -└──────────────────────────────┘ - -mysql> create table d1.t1 (a int); -mysql> alter table d1.t1 set tiflash replica 1; -mysql> insert into d1.t1 values(1); - -func> wait_table d1 t1 - -mysql> alter table d1.t1 add column b int; - ->> DBGInvoke __enable_fail_point(pause_before_apply_raft_cmd) - -# exactly write until fail point "pause_before_apply_raft_cmd" to be disable -mysql> insert into d1.t1 values(2,2); - -mysql> drop database d1; - -=> DBGInvoke __refresh_schemas() - -# make write cmd take effect ->> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd) - -# the `t1` is still mark as tombstone ->> select tidb_database,tidb_name,is_tombstone,tidb_table_id from system.tables where is_tombstone = 0 and tidb_database = 'd1' and tidb_name='t1'; -# check the row is written to the storage or not -mysql> flashback database d1 to d1_new -mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t1 order by a; -+------+------+ -| a | b | -+------+------+ -| 1 | NULL | -| 2 | 2 | -+------+------+ - -=> DBGInvoke __refresh_schemas() -# ensure the flashbacked table and database 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 d1_new.t1 order by a; -+------+------+ -| a | b | -+------+------+ -| 1 | NULL | -| 2 | 2 | -+------+------+ diff --git a/tests/fullstack-test2/ddl/flashback/flashback_database.test b/tests/fullstack-test2/ddl/flashback/flashback_database.test new file mode 100644 index 00000000000..dd541b5d2a4 --- /dev/null +++ b/tests/fullstack-test2/ddl/flashback/flashback_database.test @@ -0,0 +1,187 @@ +# Copyright 2023 PingCAP, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +## case 1, normal flashback without failpoints + +mysql> drop database if exists d1; +mysql> drop database if exists d1_new; + +# non-partition table +mysql> create database d1; +mysql> create table d1.t3 (a int); +mysql> insert into d1.t3 values(1); +# partition table +mysql> create table d1.t4(id INT NOT NULL,name VARCHAR(30)) PARTITION BY RANGE (id) ( PARTITION p0 VALUES LESS THAN (50),PARTITION p1 VALUES LESS THAN (100)); +mysql> insert into d1.t4 values(1, 'abc'),(2, 'cde'),(53, 'efg'); + +mysql> alter table d1.t3 set tiflash replica 1; +mysql> alter table d1.t4 set tiflash replica 1; +func> wait_table d1 t3 t4 + +mysql> alter table d1.t3 add column b int; +mysql> insert into d1.t3 values(2,2); +mysql> alter table d1.t4 add column b int; + +mysql> drop database d1; + +mysql> flashback database d1 to d1_new +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t3 order by a; ++------+------+ +| a | b | ++------+------+ +| 1 | NULL | +| 2 | 2 | ++------+------+ +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t4 order by id; ++----+------+------+ +| id | name | b | ++----+------+------+ +| 1 | abc | NULL | +| 2 | cde | NULL | +| 53 | efg | NULL | ++----+------+------+ + +# ensure the flashbacked table and database is not mark as tombstone +>> DBGInvoke __enable_schema_sync_service('true') +>> DBGInvoke __gc_schemas(18446744073709551615) + +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t3 order by a; ++------+------+ +| a | b | ++------+------+ +| 1 | NULL | +| 2 | 2 | ++------+------+ +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t4 order by id; ++----+------+------+ +| id | name | b | ++----+------+------+ +| 1 | abc | NULL | +| 2 | cde | NULL | +| 53 | efg | NULL | ++----+------+------+ + +=> DBGInvoke __enable_schema_sync_service('false') +>> DBGInvoke __init_fail_point() + +mysql> drop database if exists d1; +mysql> drop database if exists d1_new; + +## case 2, non-partition table +mysql> create database d1; + +=> DBGInvoke __refresh_schemas() + +=> DBGInvoke mapped_database_exists(d1) +┌─mapped_database_exists(d1)───┐ +│ true │ +└──────────────────────────────┘ + +mysql> create table d1.t1 (a int); +mysql> alter table d1.t1 set tiflash replica 1; +mysql> insert into d1.t1 values(1); + +func> wait_table d1 t1 + +mysql> alter table d1.t1 add column b int; + +>> DBGInvoke __enable_fail_point(pause_before_apply_raft_cmd) + +# exactly write until fail point "pause_before_apply_raft_cmd" to be disable +mysql> insert into d1.t1 values(2,2); + +mysql> drop database d1; + +=> DBGInvoke __refresh_schemas() + +# make write cmd take effect +>> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd) + +# the `t1` is still mark as tombstone +>> select tidb_database,tidb_name,is_tombstone,tidb_table_id from system.tables where is_tombstone = 0 and tidb_database = 'd1' and tidb_name='t1'; +# check the row is written to the storage or not +mysql> flashback database d1 to d1_new +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t1 order by a; ++------+------+ +| a | b | ++------+------+ +| 1 | NULL | +| 2 | 2 | ++------+------+ + +=> DBGInvoke __refresh_schemas() +# ensure the flashbacked table and database 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 d1_new.t1 order by a; ++------+------+ +| a | b | ++------+------+ +| 1 | NULL | +| 2 | 2 | ++------+------+ + +mysql> drop database if exists d1; +mysql> drop database if exists d1_new; + +## case 3, partition table +mysql> create database d1; + +=> DBGInvoke __refresh_schemas() + +mysql> create table d1.t2(id INT NOT NULL,name VARCHAR(30)) PARTITION BY RANGE (id) ( PARTITION p0 VALUES LESS THAN (50),PARTITION p1 VALUES LESS THAN (100)); +mysql> insert into d1.t2 values(1, 'abc'),(2, 'cde'),(53, 'efg'); + +>> DBGInvoke __enable_fail_point(pause_before_apply_raft_cmd) +mysql> alter table d1.t2 set tiflash replica 1; +mysql> alter table d1.t2 add column b int; + +mysql> drop database d1; + +=> DBGInvoke __refresh_schemas() + +# make write cmd take effect +>> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd) + +# check the row is written to the storage or not +mysql> flashback database d1 to d1_new + +func> wait_table d1_new t2 +mysql> set session tidb_isolation_read_engines='tiflash'; select * from d1_new.t2 order by id; ++----+------+------+ +| id | name | b | ++----+------+------+ +| 1 | abc | NULL | +| 2 | cde | NULL | +| 53 | efg | NULL | ++----+------+------+ + +=> DBGInvoke __refresh_schemas() +# ensure the flashbacked table and database 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 d1_new.t2 order by id; ++----+------+------+ +| id | name | b | ++----+------+------+ +| 1 | abc | NULL | +| 2 | cde | NULL | +| 53 | efg | NULL | ++----+------+------+ + +# cleanup +mysql> drop database if exists d1; +mysql> drop database if exists d1_new; From 8be7640d518a41eed41328262606a04843f21da7 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 27 Nov 2023 15:58:45 +0800 Subject: [PATCH 03/10] Remove useless changes --- dbms/src/TiDB/Schema/SchemaGetter.cpp | 37 ++------------------------- 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaGetter.cpp b/dbms/src/TiDB/Schema/SchemaGetter.cpp index 703133bfef4..26dae26bb82 100644 --- a/dbms/src/TiDB/Schema/SchemaGetter.cpp +++ b/dbms/src/TiDB/Schema/SchemaGetter.cpp @@ -115,9 +115,7 @@ struct TxnStructure static String hGet(KeyspaceSnapshot & snap, const String & key, const String & field) { String encode_key = encodeHashDataKey(key, field); - // LOG_INFO(Logger::get(), "hGet, encode_key={}", encode_key); - String value = snap.Get(encode_key); - return value; + return snap.Get(encode_key); } static String mvccGet(KeyspaceSnapshot & snap, const String & key, const String & field) @@ -145,24 +143,6 @@ struct TxnStructure return target_value; } - static std::vector> mvccGetAll( - KeyspaceSnapshot & snap, - const String & key, - const String & field) - { - auto encode_key = encodeHashDataKey(key, field); - // LOG_INFO(Logger::get(), "mvccGetAll, encode_key={}", encode_key); - auto mvcc_info = snap.mvccGet(encode_key); - const auto & values = mvcc_info.values(); - - std::vector> res; - for (const auto & value_pair : values) - { - res.emplace_back(value_pair.value(), value_pair.start_ts()); - } - return res; - } - // For convinient, we only return values. static std::vector> hGetAll(KeyspaceSnapshot & snap, const String & key) { @@ -181,19 +161,6 @@ struct TxnStructure } return res; } - - static std::vector> scan(KeyspaceSnapshot & snap, const String & key) - { - auto tikv_key_end = pingcap::kv::prefixNext(key); - auto scanner = snap.Scan(key, tikv_key_end); - std::vector> res; - while (scanner.valid) - { - res.push_back(std::make_pair(scanner.key(), scanner.value())); - scanner.next(); - } - return res; - } }; AffectedOption::AffectedOption(Poco::JSON::Object::Ptr json) @@ -285,7 +252,7 @@ std::optional SchemaGetter::getSchemaDiff(Int64 ver) LOG_WARNING(log, "The schema diff is empty, schema_version={} key={}", ver, key); return std::nullopt; } - LOG_DEBUG(log, "Get SchemaDiff from TiKV, schema_version={} data={}", ver, data); + LOG_TRACE(log, "Get SchemaDiff from TiKV, schema_version={} data={}", ver, data); SchemaDiff diff; diff.deserialize(data); return diff; From 35608e8b4c1a0c236f6d5982c1838fcaff500a0f Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 27 Nov 2023 16:11:13 +0800 Subject: [PATCH 04/10] Unify the naming --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 53 ++++++++++++++------------ dbms/src/TiDB/Schema/SchemaBuilder.h | 16 ++++---- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index 1957048552c..2d3c15104a0 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -259,12 +259,17 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) { case SchemaActionType::CreateSchema: { - applyCreateSchema(diff.schema_id); + applyCreateDatabase(diff.schema_id); break; } case SchemaActionType::DropSchema: { - applyDropSchema(diff.schema_id); + applyDropDatabase(diff.schema_id); + break; + } + case SchemaActionType::ActionRecoverSchema: + { + applyRecoverDatabase(diff.schema_id); break; } case SchemaActionType::CreateTables: @@ -295,11 +300,6 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff) applyRecoverTable(diff.schema_id, diff.table_id); break; } - case SchemaActionType::ActionRecoverSchema: - { - applyRecoverDatabase(diff.schema_id); - break; - } case SchemaActionType::DropTable: case SchemaActionType::DropView: { @@ -874,19 +874,19 @@ String createDatabaseStmt(Context & context, const DBInfo & db_info, const Schem } template -bool SchemaBuilder::applyCreateSchema(DatabaseID schema_id) +bool SchemaBuilder::applyCreateDatabase(DatabaseID database_id) { - auto db_info = getter.getDatabase(schema_id); - if (unlikely(db_info == nullptr)) + auto db_info = getter.getDatabase(database_id); + if (db_info == nullptr) { return false; } - applyCreateSchema(db_info); + applyCreateDatabaseByInfo(db_info); return true; } template -void SchemaBuilder::applyCreateSchema(const TiDB::DBInfoPtr & db_info) +void SchemaBuilder::applyCreateDatabaseByInfo(const TiDB::DBInfoPtr & db_info) { GET_METRIC(tiflash_schema_internal_ddl_count, type_create_db).Increment(); LOG_INFO(log, "Create database {} begin, database_id={}", name_mapper.debugDatabaseName(*db_info), db_info->id); @@ -911,6 +911,11 @@ void SchemaBuilder::applyRecoverDatabase(DatabaseID database auto db_info = getter.getDatabase(database_id); if (db_info == nullptr) { + LOG_INFO( + log, + "Recover database is ignored because database is not exist in TiKV," + " database_id={}", + database_id); return; } LOG_INFO(log, "Recover database begin, database_id={}", database_id); @@ -920,7 +925,7 @@ void SchemaBuilder::applyRecoverDatabase(DatabaseID database { LOG_INFO( log, - "Recover database {} ignored because instance is not exists, may have been physically dropped, " + "Recover database is ignored because instance is not exists, may have been physically dropped, " "database_id={}", db_name, database_id); @@ -944,29 +949,29 @@ void SchemaBuilder::applyRecoverDatabase(DatabaseID database } template -void SchemaBuilder::applyDropSchema(DatabaseID schema_id) +void SchemaBuilder::applyDropDatabase(DatabaseID database_id) { - TiDB::DBInfoPtr db_info = databases.getDBInfo(schema_id); + TiDB::DBInfoPtr db_info = databases.getDBInfo(database_id); if (unlikely(db_info == nullptr)) { - LOG_INFO(log, "Try to drop database but not found, may has been dropped, database_id={}", schema_id); + LOG_INFO(log, "Try to drop database but not found, may has been dropped, database_id={}", database_id); return; } { //TODO: it seems may need a lot time, maybe we can do it in a background thread - auto table_ids = table_id_map.findTablesByDatabaseID(schema_id); + auto table_ids = table_id_map.findTablesByDatabaseID(database_id); for (auto table_id : table_ids) - applyDropTable(schema_id, table_id); + applyDropTable(database_id, table_id); } - applyDropSchema(name_mapper.mapDatabaseName(schema_id, keyspace_id)); + applyDropDatabaseByName(name_mapper.mapDatabaseName(database_id, keyspace_id)); - databases.eraseDBInfo(schema_id); + databases.eraseDBInfo(database_id); } template -void SchemaBuilder::applyDropSchema(const String & db_name) +void SchemaBuilder::applyDropDatabaseByName(const String & db_name) { GET_METRIC(tiflash_schema_internal_ddl_count, type_drop_db).Increment(); LOG_INFO(log, "Tombstone database begin, db_name={}", db_name); @@ -1247,7 +1252,7 @@ void SchemaBuilder::syncAllSchema() { break; } - applyCreateSchema(db_info); + applyCreateDatabaseByInfo(db_info); { std::unique_lock created_db_set_lock(created_db_set_mutex); created_db_set.emplace(name_mapper.mapDatabaseName(*db_info)); @@ -1345,7 +1350,7 @@ void SchemaBuilder::syncAllSchema() } if (created_db_set.count(it->first) == 0 && !isReservedDatabase(context, it->first)) { - applyDropSchema(it->first); + applyDropDatabaseByName(it->first); LOG_INFO(log, "Database {} dropped during sync all schemas", it->first); } } @@ -1517,7 +1522,7 @@ void SchemaBuilder::dropAllSchema() { continue; } - applyDropSchema(db.first); + applyDropDatabaseByName(db.first); LOG_INFO(log, "Database {} dropped during drop all schemas", db.first); } diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.h b/dbms/src/TiDB/Schema/SchemaBuilder.h index 20cdb154a6e..8e84fbeb2df 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.h +++ b/dbms/src/TiDB/Schema/SchemaBuilder.h @@ -64,29 +64,29 @@ struct SchemaBuilder bool applyTable(DatabaseID database_id, TableID logical_table_id, TableID physical_table_id, bool force); private: - void applyDropSchema(DatabaseID schema_id); + void applyDropDatabase(DatabaseID database_id); /// Parameter db_name should be mapped. - void applyDropSchema(const String & db_name); + void applyDropDatabaseByName(const String & db_name); - bool applyCreateSchema(DatabaseID schema_id); - void applyCreateSchema(const TiDB::DBInfoPtr & db_info); + bool applyCreateDatabase(DatabaseID database_id); + void applyCreateDatabaseByInfo(const TiDB::DBInfoPtr & db_info); void applyRecoverDatabase(DatabaseID database_id); + void applyCreateTable(DatabaseID database_id, TableID table_id); void applyCreateStorageInstance( const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info, bool is_tombstone); void applyDropTable(DatabaseID database_id, TableID table_id); + /// Parameter schema_name should be mapped. + void applyDropPhysicalTable(const String & db_name, TableID table_id); void applyRecoverTable(DatabaseID database_id, TiDB::TableID table_id); void applyRecoverLogicalTable(const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info); bool tryRecoverPhysicalTable(const TiDB::DBInfoPtr & db_info, const TiDB::TableInfoPtr & table_info); - /// Parameter schema_name should be mapped. - void applyDropPhysicalTable(const String & db_name, TableID table_id); - void applyPartitionDiff(DatabaseID database_id, TableID table_id); void applyPartitionDiffOnLogicalTable( const TiDB::DBInfoPtr & db_info, @@ -107,8 +107,6 @@ struct SchemaBuilder void applySetTiFlashReplica(DatabaseID database_id, TableID table_id); - void applyCreateTable(DatabaseID database_id, TableID table_id); - void applyExchangeTablePartition(const SchemaDiff & diff); }; From 776fdf42dc8fada46ce659fcc28e01e7d5f8220e Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 27 Nov 2023 17:20:42 +0800 Subject: [PATCH 05/10] Add test case --- dbms/src/Databases/DatabaseTiFlash.cpp | 5 +++++ dbms/src/Databases/test/gtest_database.cpp | 24 ++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/dbms/src/Databases/DatabaseTiFlash.cpp b/dbms/src/Databases/DatabaseTiFlash.cpp index db80683e212..05382aa11a8 100644 --- a/dbms/src/Databases/DatabaseTiFlash.cpp +++ b/dbms/src/Databases/DatabaseTiFlash.cpp @@ -719,6 +719,11 @@ void DatabaseTiFlash::alterTombstone(const Context & context, Timestamp tombston // After all done, set the tombstone tombstone = tombstone_; + // Overwrite db_info if not null + if (new_db_info) + { + db_info = new_db_info; + } } void DatabaseTiFlash::drop(const Context & context) diff --git a/dbms/src/Databases/test/gtest_database.cpp b/dbms/src/Databases/test/gtest_database.cpp index 7ff716fc458..5b5062b1d84 100644 --- a/dbms/src/Databases/test/gtest_database.cpp +++ b/dbms/src/Databases/test/gtest_database.cpp @@ -944,6 +944,7 @@ try )", }; + size_t case_no = 0; for (const auto & statement : statements) { { @@ -973,6 +974,13 @@ try db->alterTombstone(*ctx, tso, nullptr); EXPECT_TRUE(db->isTombstone()); EXPECT_EQ(db->getTombstone(), tso); + if (case_no != 0) + { + auto db_tiflash = std::dynamic_pointer_cast(db); + ASSERT_NE(db_tiflash, nullptr); + auto db_info = db_tiflash->getDatabaseInfo(); + ASSERT_EQ(db_info.name, "test_db"); // not changed + } // Try restore from disk db = detachThenAttach(*ctx, db_name, std::move(db), log); @@ -985,10 +993,26 @@ try NullspaceID); db->alterTombstone(*ctx, 0, new_db_info); EXPECT_FALSE(db->isTombstone()); + if (case_no != 0) + { + auto db_tiflash = std::dynamic_pointer_cast(db); + ASSERT_NE(db_tiflash, nullptr); + auto db_info = db_tiflash->getDatabaseInfo(); + ASSERT_EQ(db_info.name, "test_new_db"); // changed by the `new_db_info` + } // Try restore from disk db = detachThenAttach(*ctx, db_name, std::move(db), log); EXPECT_FALSE(db->isTombstone()); + if (case_no != 0) + { + auto db_tiflash = std::dynamic_pointer_cast(db); + ASSERT_NE(db_tiflash, nullptr); + auto db_info = db_tiflash->getDatabaseInfo(); + ASSERT_EQ(db_info.name, "test_new_db"); // changed by the `new_db_info` + } + + case_no += 1; } } CATCH From 6383e4c86529bef4ad5afcff09e357bd2c4a1421 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 1 Dec 2023 17:36:27 +0800 Subject: [PATCH 06/10] Add hint --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index 2d3c15104a0..44da0497e8c 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -49,6 +49,7 @@ #include #include #include +#include "common/defines.h" namespace DB { @@ -877,7 +878,7 @@ template bool SchemaBuilder::applyCreateDatabase(DatabaseID database_id) { auto db_info = getter.getDatabase(database_id); - if (db_info == nullptr) + if (unlikely(db_info == nullptr)) { return false; } @@ -909,7 +910,7 @@ template void SchemaBuilder::applyRecoverDatabase(DatabaseID database_id) { auto db_info = getter.getDatabase(database_id); - if (db_info == nullptr) + if (unlikely(db_info == nullptr)) { LOG_INFO( log, @@ -921,7 +922,7 @@ void SchemaBuilder::applyRecoverDatabase(DatabaseID database LOG_INFO(log, "Recover database begin, database_id={}", database_id); auto db_name = name_mapper.mapDatabaseName(database_id, keyspace_id); auto db = context.tryGetDatabase(db_name); - if (!db) + if (unlikely(!db)) { LOG_INFO( log, From 94d4ebc5ea4877ce301707a6b255832a331b836c Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 1 Dec 2023 17:41:38 +0800 Subject: [PATCH 07/10] Format files --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index 44da0497e8c..f11bd7cbf26 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -49,7 +50,6 @@ #include #include #include -#include "common/defines.h" namespace DB { From 481b236cb290c265a883043599ebde16bf65129f Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Fri, 1 Dec 2023 18:38:37 +0800 Subject: [PATCH 08/10] Fix typo --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index f11bd7cbf26..2b311fc6c0b 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -415,7 +415,7 @@ void SchemaBuilder::applySetTiFlashReplica(DatabaseID databa return; } - // Recover the table if tombstoned + // Recover the table if tombstone if (storage->isTombstone()) { applyRecoverLogicalTable(db_info, table_info); @@ -574,7 +574,7 @@ void SchemaBuilder::applyPartitionDiffOnLogicalTable( { LOG_INFO( log, - "No partition changes, paritions_size={} {} with database_id={}, table_id={}", + "No partition changes, partitions_size={} {} with database_id={}, table_id={}", new_part_id_set.size(), name_mapper.debugCanonicalName(*db_info, *table_info), db_info->id, @@ -582,7 +582,7 @@ void SchemaBuilder::applyPartitionDiffOnLogicalTable( return; } - // Copy the local table info and update fileds on the copy + // Copy the local table info and update fields 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; From 6ceeb01eb431ee35511f44cc57fdfe1032f53801 Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 4 Dec 2023 10:44:08 +0800 Subject: [PATCH 09/10] Refine log level --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index 2b311fc6c0b..cc2eaae6b4e 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -924,7 +924,7 @@ void SchemaBuilder::applyRecoverDatabase(DatabaseID database auto db = context.tryGetDatabase(db_name); if (unlikely(!db)) { - LOG_INFO( + LOG_ERROR( log, "Recover database is ignored because instance is not exists, may have been physically dropped, " "database_id={}", From 221708d7cc0285ed3023e539f7ef7d302b76385b Mon Sep 17 00:00:00 2001 From: JaySon-Huang Date: Mon, 4 Dec 2023 11:34:10 +0800 Subject: [PATCH 10/10] Add tombstone timestamp to logging --- dbms/src/TiDB/Schema/SchemaBuilder.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dbms/src/TiDB/Schema/SchemaBuilder.cpp b/dbms/src/TiDB/Schema/SchemaBuilder.cpp index cc2eaae6b4e..f7f8ef8aea5 100644 --- a/dbms/src/TiDB/Schema/SchemaBuilder.cpp +++ b/dbms/src/TiDB/Schema/SchemaBuilder.cpp @@ -996,7 +996,7 @@ void SchemaBuilder::applyDropDatabaseByName(const String & d auto tombstone = tmt_context.getPDClient()->getTS(); db->alterTombstone(context, tombstone, /*new_db_info*/ nullptr); // keep the old db_info - LOG_INFO(log, "Tombstone database end, db_name={}", db_name); + LOG_INFO(log, "Tombstone database end, db_name={} tombstone={}", db_name, tombstone); } std::tuple parseColumnsFromTableInfo(const TiDB::TableInfo & table_info) @@ -1164,6 +1164,7 @@ void SchemaBuilder::applyDropPhysicalTable(const String & db name_mapper.debugTableName(storage->getTableInfo()), table_id); + const UInt64 tombstone_ts = tmt_context.getPDClient()->getTS(); // TODO:try to optimize alterCommands AlterCommands commands; { @@ -1174,17 +1175,18 @@ void SchemaBuilder::applyDropPhysicalTable(const String & db // 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 = tmt_context.getPDClient()->getTS(); + command.tombstone = tombstone_ts; commands.emplace_back(std::move(command)); } auto alter_lock = storage->lockForAlter(getThreadNameAndID()); storage->updateTombstone(alter_lock, commands, db_name, storage->getTableInfo(), name_mapper, context); LOG_INFO( log, - "Tombstone table {}.{} end, table_id={}", + "Tombstone table {}.{} end, table_id={} tombstone={}", db_name, name_mapper.debugTableName(storage->getTableInfo()), - table_id); + table_id, + tombstone_ts); } template