PageStorage: Fix unstable rename test

dbms/src/Databases/test/gtest_database.cpp

@@ -97,16 +97,10 @@ class DatabaseTiFlashTest : public ::testing::Test
     static void recreateMetadataPath()
     {
         String path = TiFlashTestEnv::getContext().getPath();
-
         auto p = path + "/metadata/";
-        if (Poco::File file(p); file.exists())
-            file.remove(true);
-        Poco::File{p}.createDirectories();
-
+        TiFlashTestEnv::tryRemovePath(p, /*recreate=*/true);
         p = path + "/data/";
-        if (Poco::File file(p); file.exists())
-            file.remove(true);
-        Poco::File{p}.createDirectories();
+        TiFlashTestEnv::tryRemovePath(p, /*recreate=*/true);
     }
 
 protected:

dbms/src/Debug/MockSchemaGetter.h

@@ -23,30 +23,33 @@ namespace DB
 {
 struct MockSchemaGetter
 {
-    TiDB::DBInfoPtr getDatabase(DatabaseID db_id) { return MockTiDB::instance().getDBInfoByID(db_id); }
+    static TiDB::DBInfoPtr getDatabase(DatabaseID db_id) { return MockTiDB::instance().getDBInfoByID(db_id); }
 
-    Int64 getVersion() { return MockTiDB::instance().getVersion(); }
+    static Int64 getVersion() { return MockTiDB::instance().getVersion(); }
 
-    std::optional<SchemaDiff> getSchemaDiff(Int64 version)
+    static std::optional<SchemaDiff> getSchemaDiff(Int64 version)
     {
         return MockTiDB::instance().getSchemaDiff(version);
     }
 
-    bool checkSchemaDiffExists(Int64 version)
+    static bool checkSchemaDiffExists(Int64 version)
     {
         return MockTiDB::instance().checkSchemaDiffExists(version);
     }
 
-    TiDB::TableInfoPtr getTableInfo(DatabaseID, TableID table_id) { return MockTiDB::instance().getTableInfoByID(table_id); }
+    static TiDB::TableInfoPtr getTableInfo(DatabaseID, TableID table_id)
+    {
+        return MockTiDB::instance().getTableInfoByID(table_id);
+    }
 
-    std::vector<TiDB::DBInfoPtr> listDBs()
+    static std::vector<TiDB::DBInfoPtr> listDBs()
     {
         std::vector<TiDB::DBInfoPtr> res;
         const auto & databases = MockTiDB::instance().getDatabases();
-        for (auto it = databases.begin(); it != databases.end(); it++)
+        for (const auto & database : databases)
         {
-            auto db_id = it->second;
-            auto db_name = it->first;
+            auto db_id = database.second;
+            auto db_name = database.first;
             TiDB::DBInfoPtr db_ptr = std::make_shared<TiDB::DBInfo>(TiDB::DBInfo());
             db_ptr->id = db_id;
             db_ptr->name = db_name;
@@ -55,15 +58,15 @@ struct MockSchemaGetter
         return res;
     }
 
-    std::vector<TiDB::TableInfoPtr> listTables(Int64 db_id)
+    static std::vector<TiDB::TableInfoPtr> listTables(Int64 db_id)
     {
         auto tables_by_id = MockTiDB::instance().getTables();
         std::vector<TiDB::TableInfoPtr> res;
-        for (auto it = tables_by_id.begin(); it != tables_by_id.end(); it++)
+        for (auto & it : tables_by_id)
         {
-            if (it->second->dbID() == db_id)
+            if (it.second->dbID() == db_id)
             {
-                res.push_back(std::make_shared<TiDB::TableInfo>(TiDB::TableInfo(it->second->table_info)));
+                res.push_back(std::make_shared<TiDB::TableInfo>(TiDB::TableInfo(it.second->table_info)));
             }
         }
         return res;

dbms/src/Debug/MockTiDB.cpp

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <Common/Exception.h>
 #include <DataTypes/DataTypeDecimal.h>
 #include <DataTypes/DataTypeEnum.h>
 #include <DataTypes/DataTypeMyDate.h>
@@ -31,6 +32,8 @@
 #include <Storages/Transaction/TiDB.h>
 #include <Storages/Transaction/TypeMapping.h>
 
+#include <mutex>
+
 namespace DB
 {
 namespace ErrorCodes
@@ -350,27 +353,40 @@ Field getDefaultValue(const ASTPtr & default_value_ast)
     return Field();
 }
 
-void MockTiDB::newPartition(const String & database_name, const String & table_name, TableID partition_id, Timestamp tso, bool is_add_part)
+TableID MockTiDB::newPartition(TableID belong_logical_table, const String & partition_name, Timestamp tso, bool is_add_part)
 {
     std::lock_guard lock(tables_mutex);
 
-    TablePtr table = getTableByNameInternal(database_name, table_name);
-    TableInfo & table_info = table->table_info;
+    TablePtr logical_table = getTableByID(belong_logical_table);
+    TableID partition_id = table_id_allocator++; // allocate automatically
 
-    const auto & part_def = find_if(
-        table_info.partition.definitions.begin(),
-        table_info.partition.definitions.end(),
-        [&partition_id](PartitionDefinition & part_def) { return part_def.id == partition_id; });
-    if (part_def != table_info.partition.definitions.end())
-        throw Exception("Mock TiDB table " + database_name + "." + table_name + " already has partition " + std::to_string(partition_id),
-                        ErrorCodes::LOGICAL_ERROR);
+    return newPartitionImpl(logical_table, partition_id, partition_name, tso, is_add_part);
+}
+
+TableID MockTiDB::newPartition(const String & database_name, const String & table_name, TableID partition_id, Timestamp tso, bool is_add_part)
+{
+    std::lock_guard lock(tables_mutex);
+
+    TablePtr logical_table = getTableByNameInternal(database_name, table_name);
+    return newPartitionImpl(logical_table, partition_id, toString(partition_id), tso, is_add_part);
+}
+
+TableID MockTiDB::newPartitionImpl(const TablePtr & logical_table, TableID partition_id, const String & partition_name, Timestamp tso, bool is_add_part)
+{
+    TableInfo & table_info = logical_table->table_info;
+    RUNTIME_CHECK_MSG(!logical_table->existPartitionID(partition_id),
+                      "Mock TiDB table {}.{} already has partition {}, table_info={}",
+                      logical_table->database_name,
+                      logical_table->table_name,
+                      partition_id,
+                      table_info.serialize());
 
     table_info.is_partition_table = true;
     table_info.partition.enable = true;
     table_info.partition.num++;
     PartitionDefinition partition_def;
     partition_def.id = partition_id;
-    partition_def.name = std::to_string(partition_id);
+    partition_def.name = partition_name;
     table_info.partition.definitions.emplace_back(partition_def);
     table_info.update_timestamp = tso;
 
@@ -380,11 +396,12 @@ void MockTiDB::newPartition(const String & database_name, const String & table_n
 
         SchemaDiff diff;
         diff.type = SchemaActionType::AddTablePartition;
-        diff.schema_id = table->database_id;
-        diff.table_id = table->id();
+        diff.schema_id = logical_table->database_id;
+        diff.table_id = logical_table->id();
         diff.version = version;
         version_diff[version] = diff;
     }
+    return partition_id;
 }
 
 void MockTiDB::dropPartition(const String & database_name, const String & table_name, TableID partition_id)
@@ -631,6 +648,13 @@ TablePtr MockTiDB::getTableByNameInternal(const String & database_name, const St
     return it->second;
 }
 
+TablePtr MockTiDB::getTableByID(TableID table_id)
+{
+    if (auto it = tables_by_id.find(table_id); it != tables_by_id.end())
+        return it->second;
+    throw Exception(fmt::format("Mock TiDB table does not exists, table_id={}", table_id), ErrorCodes::UNKNOWN_TABLE);
+}
+
 TiDB::TableInfoPtr MockTiDB::getTableInfoByID(TableID table_id)
 {
     auto it = tables_by_id.find(table_id);

dbms/src/Debug/MockTiDB.h

@@ -37,23 +37,32 @@ class MockTiDB : public ext::Singleton<MockTiDB>
     public:
         Table(const String & database_name, DatabaseID database_id, const String & table_name, TiDB::TableInfo && table_info);
 
-        TableID id() { return table_info.id; }
-        DatabaseID dbID() { return database_id; }
+        TableID id() const { return table_info.id; }
+        DatabaseID dbID() const { return database_id; }
 
         ColumnID allocColumnID() { return ++col_id; }
 
-        bool isPartitionTable() { return table_info.is_partition_table; }
+        bool isPartitionTable() const { return table_info.is_partition_table; }
 
-        std::vector<TableID> getPartitionIDs()
+        std::vector<TableID> getPartitionIDs() const
         {
             std::vector<TableID> partition_ids;
             std::for_each(
                 table_info.partition.definitions.begin(),
                 table_info.partition.definitions.end(),
-                [&](const TiDB::PartitionDefinition & part_def) { partition_ids.emplace_back(part_def.id); });
+                [&](const auto & part_def) { partition_ids.emplace_back(part_def.id); });
             return partition_ids;
         }
 
+        bool existPartitionID(TableID part_id) const
+        {
+            const auto & part_def = find_if(
+                table_info.partition.definitions.begin(),
+                table_info.partition.definitions.end(),
+                [&part_id](const auto & part_def) { return part_def.id == part_id; });
+            return part_def != table_info.partition.definitions.end();
+        }
+
         TiDB::TableInfo table_info;
 
     private:
@@ -89,7 +98,8 @@ class MockTiDB : public ext::Singleton<MockTiDB>
 
     DatabaseID newDataBase(const String & database_name);
 
-    void newPartition(const String & database_name, const String & table_name, TableID partition_id, Timestamp tso, bool);
+    TableID newPartition(const String & database_name, const String & table_name, TableID partition_id, Timestamp tso, bool);
+    TableID newPartition(TableID belong_logical_table, const String & partition_name, Timestamp tso, bool);
 
     void dropPartition(const String & database_name, const String & table_name, TableID partition_id);
 
@@ -135,13 +145,15 @@ class MockTiDB : public ext::Singleton<MockTiDB>
 
     std::unordered_map<TableID, TablePtr> getTables() { return tables_by_id; }
 
-    Int64 getVersion() { return version; }
+    Int64 getVersion() const { return version; }
 
     TableID newTableID() { return table_id_allocator++; }
 
 private:
+    TableID newPartitionImpl(const TablePtr & logical_table, TableID partition_id, const String & partition_name, Timestamp tso, bool is_add_part);
     TablePtr dropTableInternal(Context & context, const String & database_name, const String & table_name, bool drop_regions);
     TablePtr getTableByNameInternal(const String & database_name, const String & table_name);
+    TablePtr getTableByID(TableID table_id);
 
 private:
     std::mutex tables_mutex;

dbms/src/Server/RaftConfigParser.h

@@ -37,8 +37,13 @@ struct TiFlashRaftConfig
     std::unordered_set<std::string> ignore_databases{"system"};
     // Actually it is "flash.service_addr"
     std::string flash_server_addr;
+
+    // Use PageStorage V1 for kvstore or not.
+    // TODO: remove this config
     bool enable_compatible_mode = true;
 
+    bool for_unit_test = false;
+
     static constexpr TiDB::StorageEngine DEFAULT_ENGINE = TiDB::StorageEngine::DT;
     TiDB::StorageEngine engine = DEFAULT_ENGINE;
     TiDB::SnapshotApplyMethod snapshot_apply_method = TiDB::SnapshotApplyMethod::DTFile_Directory;

dbms/src/Storages/PathPool.cpp

@@ -230,8 +230,8 @@ void StoragePathPool::clearPSV2ObsoleteData()
 
 void StoragePathPool::rename(const String & new_database, const String & new_table)
 {
-    RUNTIME_CHECK(!new_database.empty() && !new_table.empty(), new_database, new_table);
-    RUNTIME_CHECK(!path_need_database_name);
+    RUNTIME_CHECK(!new_database.empty() && !new_table.empty(), database, table, new_database, new_table);
+    RUNTIME_CHECK(!path_need_database_name, database, table, new_database, new_table);
 
     // The directories for storing table data is not changed, just rename related names.
     std::lock_guard lock{mutex};

dbms/src/Storages/Transaction/PDTiKVClient.h

@@ -31,11 +31,6 @@
 
 #include <atomic>
 
-// We define a shared ptr here, because TMTContext / SchemaSyncer / IndexReader all need to
-// `share` the resource of cluster.
-using KVClusterPtr = std::shared_ptr<pingcap::kv::Cluster>;
-
-
 namespace DB
 {
 struct PDClientHelper

dbms/src/Storages/Transaction/TMTContext.cpp

@@ -27,6 +27,8 @@
 #include <TiDB/Schema/TiDBSchemaSyncer.h>
 #include <pingcap/pd/MockPDClient.h>
 
+#include <memory>
+
 namespace DB
 {
 // default batch-read-index timeout is 10_000ms.
@@ -38,6 +40,28 @@ const int64_t DEFAULT_WAIT_REGION_READY_TIMEOUT_SEC = 20 * 60;
 
 const int64_t DEFAULT_READ_INDEX_WORKER_TICK_MS = 10;
 
+static SchemaSyncerPtr createSchemaSyncer(bool exist_pd_addr, bool for_unit_test, const KVClusterPtr & cluster)
+{
+    if (exist_pd_addr)
+    {
+        // product env
+        // Get DBInfo/TableInfo from TiKV, and create table with names `t_${table_id}`
+        return std::static_pointer_cast<SchemaSyncer>(
+            std::make_shared<TiDBSchemaSyncer</*mock_getter*/ false, /*mock_mapper*/ false>>(cluster));
+    }
+    else if (!for_unit_test)
+    {
+        // mock test
+        // Get DBInfo/TableInfo from MockTiDB, and create table with its display names
+        return std::static_pointer_cast<SchemaSyncer>(
+            std::make_shared<TiDBSchemaSyncer</*mock_getter*/ true, /*mock_mapper*/ true>>(cluster));
+    }
+    // unit test.
+    // Get DBInfo/TableInfo from MockTiDB, but create table with names `t_${table_id}`
+    return std::static_pointer_cast<SchemaSyncer>(
+        std::make_shared<TiDBSchemaSyncer</*mock_getter*/ true, /*mock_mapper*/ false>>(cluster));
+}
+
 TMTContext::TMTContext(Context & context_, const TiFlashRaftConfig & raft_config, const pingcap::ClusterConfig & cluster_config)
     : context(context_)
     , kvstore(std::make_shared<KVStore>(context, raft_config.snapshot_apply_method))
@@ -47,9 +71,7 @@ TMTContext::TMTContext(Context & context_, const TiFlashRaftConfig & raft_config
     , cluster(raft_config.pd_addrs.empty() ? std::make_shared<pingcap::kv::Cluster>()
                                            : std::make_shared<pingcap::kv::Cluster>(raft_config.pd_addrs, cluster_config))
     , ignore_databases(raft_config.ignore_databases)
-    , schema_syncer(raft_config.pd_addrs.empty()
-                        ? std::static_pointer_cast<SchemaSyncer>(std::make_shared<TiDBSchemaSyncer</*mock*/ true>>(cluster))
-                        : std::static_pointer_cast<SchemaSyncer>(std::make_shared<TiDBSchemaSyncer</*mock*/ false>>(cluster)))
+    , schema_syncer(createSchemaSyncer(!raft_config.pd_addrs.empty(), raft_config.for_unit_test, cluster))
     , mpp_task_manager(std::make_shared<MPPTaskManager>(
           std::make_unique<MinTSOScheduler>(
               context.getSettingsRef().task_scheduler_thread_soft_limit,

dbms/src/Storages/Transaction/TMTContext.h

@@ -44,6 +44,10 @@ using GCManagerPtr = std::shared_ptr<GCManager>;
 
 struct TiFlashRaftConfig;
 
+// We define a shared ptr here, because TMTContext / SchemaSyncer / IndexReader all need to
+// `share` the resource of cluster.
+using KVClusterPtr = std::shared_ptr<pingcap::kv::Cluster>;
+
 class TMTContext : private boost::noncopyable
 {
 public:

dbms/src/TestUtils/TiFlashTestBasic.h

@@ -73,6 +73,10 @@ namespace tests
         FAIL();                                                                        \
     }
 
+/**
+  * GTest related helper functions
+  */
+
 /// helper functions for comparing DataType
 ::testing::AssertionResult DataTypeCompare(
     const char * lhs_expr,