Add persister compatibility mode test

dbms/src/Common/FailPoint.cpp

@@ -23,7 +23,9 @@ std::unordered_map<String, std::shared_ptr<FailPointChannel>> FailPointHelper::f
     M(region_exception_after_read_from_storage_some_error)   \
     M(region_exception_after_read_from_storage_all_error)    \
     M(exception_before_dmfile_remove_encryption)             \
-    M(exception_before_dmfile_remove_from_disk)
+    M(exception_before_dmfile_remove_from_disk)              \
+    M(force_enable_region_persister_compatibility_mode)      \
+    M(force_disable_region_persister_compatibility_mode)
 
 #define APPLY_FOR_FAILPOINTS_WITH_CHANNEL(M) M(pause_after_learner_read)
 

dbms/src/Storages/Page/PageStorage.cpp

@@ -72,8 +72,8 @@ PageFile::Version PageStorage::getMinDataVersion(const FileProviderPtr & file_pr
     if (page_files.empty())
         return PageFile::CURRENT_VERSION;
 
-    // Simply check the first PageFile is good enough
-    auto reader = const_cast<PageFile &>(*page_files.begin()).createMetaMergingReader();
+    // Simply check the last PageFile is good enough
+    auto reader = const_cast<PageFile &>(*page_files.rbegin()).createMetaMergingReader();
 
     PageFile::Version min_binary_version = PageFile::CURRENT_VERSION, temp_version;
     reader->moveNext(&temp_version);

dbms/src/Storages/Page/PageStorage.h

@@ -3,6 +3,7 @@
 #include <Storages/Page/Page.h>
 #include <Storages/Page/PageDefines.h>
 #include <Storages/Page/PageFile.h>
+#include <Storages/Page/VersionSet/PageEntriesVersionSet.h>
 #include <Storages/Page/VersionSet/PageEntriesVersionSetWithDelta.h>
 #include <Storages/Page/WriteBatch.h>
 

dbms/src/Storages/Transaction/RegionPersister.cpp

@@ -1,3 +1,4 @@
+#include <Common/FailPoint.h>
 #include <IO/MemoryReadWriteBuffer.h>
 #include <Interpreters/Context.h>
 #include <Storages/Page/PageStorage.h>
@@ -15,6 +16,12 @@ namespace ErrorCodes
 extern const int LOGICAL_ERROR;
 } // namespace ErrorCodes
 
+namespace FailPoints
+{
+extern const char force_enable_region_persister_compatibility_mode[];
+extern const char force_disable_region_persister_compatibility_mode[];
+}
+
 void RegionPersister::drop(RegionID region_id, const RegionTaskLock &)
 {
     if (page_storage)
@@ -135,6 +142,10 @@ RegionMap RegionPersister::restore(IndexReaderCreateFunc * func, PageStorage::Co
         auto detect_binary_version = PageStorage::getMinDataVersion(global_context.getFileProvider(), delegator);
         bool run_in_compatibility_mode
             = path_pool.isRaftStorageCapatibilityModeEnabled() && (detect_binary_version == PageFile::VERSION_BASE);
+
+        fiu_do_on(FailPoints::force_enable_region_persister_compatibility_mode, { run_in_compatibility_mode = true; });
+        fiu_do_on(FailPoints::force_disable_region_persister_compatibility_mode, { run_in_compatibility_mode = false; });
+
         if (!run_in_compatibility_mode)
         {
             config.num_write_slots = 4; // extend write slots to 4 at least

dbms/src/Storages/Transaction/tests/gtest_region_persister.cpp

@@ -2,30 +2,43 @@
 
 #define private public
 #include <Storages/Transaction/RegionManager.h>
+#include <Storages/Transaction/RegionPersister.h>
 #undef private
 
+#include <Common/FailPoint.h>
 #include <Common/Stopwatch.h>
 #include <IO/ReadBufferFromFile.h>
 #include <IO/WriteBufferFromFile.h>
 #include <Storages/Page/PageStorage.h>
 #include <Storages/Transaction/Region.h>
-#include <Storages/Transaction/RegionManager.h>
 #include <Storages/Transaction/TiKVRecordFormat.h>
 #include <test_utils/TiflashTestBasic.h>
 
 #include <ext/scope_guard.h>
 
 #include "region_helper.h"
 
-namespace DB::tests
+namespace DB
+{
+namespace FailPoints
+{
+extern const char force_enable_region_persister_compatibility_mode[];
+extern const char force_disable_region_persister_compatibility_mode[];
+} // namespace FailPoints
+
+namespace tests
 {
 
 class RegionPersister_test : public ::testing::Test
 {
 public:
     RegionPersister_test() : dir_path(TiFlashTestEnv::getTemporaryPath() + "/region_persister_tmp") {}
 
-    static void SetUpTestCase() { TiFlashTestEnv::setupLogger(); }
+    static void SetUpTestCase()
+    {
+        TiFlashTestEnv::setupLogger();
+        fiu_init(0); // init failpoint
+    }
 
     void SetUp() override { dropFiles(); }
 
@@ -53,7 +66,7 @@ static ::testing::AssertionResult PeerCompare( //
     const metapb::Peer & lhs,
     const metapb::Peer & rhs)
 {
-    if (lhs.id() == rhs.id() && lhs.is_learner() == rhs.is_learner())
+    if (lhs.id() == rhs.id() && lhs.role() == rhs.role())
         return ::testing::AssertionSuccess();
     else
         return ::testing::internal::EqFailure(lhs_expr, rhs_expr, lhs.ShortDebugString(), rhs.ShortDebugString(), false);
@@ -198,12 +211,13 @@ try
     size_t region_num = 100;
     RegionMap regions;
     TableID table_id = 100;
+
+    PageStorage::Config config;
+    config.file_roll_size = 128 * MB;
     {
         UInt64 diff = 0;
-        // PageStorage::Config config;
-        // config.file_roll_size = 128 * MB;
         RegionPersister persister(ctx, region_manager);
-        persister.restore();
+        persister.restore(nullptr, config);
 
         for (size_t i = 0; i < region_num; ++i)
         {
@@ -229,14 +243,14 @@ try
     RegionMap new_regions;
     {
         RegionPersister persister(ctx, region_manager);
-        new_regions = persister.restore();
+        new_regions = persister.restore(nullptr, config);
         size_t num_regions_missed = 0;
         for (size_t i = 0; i < region_num; ++i)
         {
             auto new_iter = new_regions.find(i);
             if (new_iter == new_regions.end())
             {
-                LOG_INFO(&Poco::Logger::get("RegionPersister_test"), "Region missed, id=" << i);
+                LOG_ERROR(&Poco::Logger::get("RegionPersister_test"), "Region missed, id=" << i);
                 ++num_regions_missed;
             }
             else
@@ -251,6 +265,117 @@ try
 }
 CATCH
 
+TEST_F(RegionPersister_test, persister_compatibility_mode)
+try
+{
+    std::string path = dir_path + "/compatibility_mode";
+
+    // Force to run in compatibility mode for the default region persister
+    FailPointHelper::enableFailPoint(FailPoints::force_enable_region_persister_compatibility_mode);
+    SCOPE_EXIT({ FailPointHelper::disableFailPoint(FailPoints::force_enable_region_persister_compatibility_mode); });
+    auto & ctx = TiFlashTestEnv::getContext(DB::Settings(),
+        Strings{
+            path,
+        });
+
+    size_t region_num = 500;
+    RegionMap regions;
+    TableID table_id = 100;
+
+    PageStorage::Config config;
+    config.file_roll_size = 2 * MB;
+    RegionManager region_manager;
+    DB::Timestamp tso = 0;
+    {
+        RegionPersister persister(ctx, region_manager);
+        // Force to run in compatibility mode
+        FailPointHelper::enableFailPoint(FailPoints::force_enable_region_persister_compatibility_mode);
+        persister.restore(nullptr, config);
+        ASSERT_EQ(persister.page_storage, nullptr);
+        ASSERT_NE(persister.stable_page_storage, nullptr);
+
+        for (size_t i = 0; i < region_num; ++i)
+        {
+            auto region = std::make_shared<Region>(createRegionMeta(i, table_id));
+            TiKVKey key = RecordKVFormat::genKey(table_id, i, tso++);
+            region->insert("default", TiKVKey::copyFrom(key), TiKVValue("value1"));
+            region->insert("write", TiKVKey::copyFrom(key), RecordKVFormat::encodeWriteCfValue('P', 0));
+            region->insert("lock", TiKVKey::copyFrom(key), RecordKVFormat::encodeLockCfValue('P', "", 0, 0));
+
+            persister.persist(*region);
+
+            regions.emplace(region->id(), region);
+        }
+    }
+
+    {
+        RegionPersister persister(ctx, region_manager);
+        // restore normally, should run in compatibility mode.
+        RegionMap new_regions = persister.restore(nullptr, config);
+        ASSERT_EQ(persister.page_storage, nullptr);
+        ASSERT_NE(persister.stable_page_storage, nullptr);
+        // Try to read
+        for (size_t i = 0; i < region_num; ++i)
+        {
+            auto new_iter = new_regions.find(i);
+            ASSERT_NE(new_iter, new_regions.end());
+            auto old_region = regions[i];
+            auto new_region = new_regions[i];
+            ASSERT_EQ(*new_region, *old_region);
+        }
+    }
+
+    size_t region_num_under_nromal_mode = 200;
+    {
+        RegionPersister persister(ctx, region_manager);
+        // Force to run in normal mode
+        FailPointHelper::enableFailPoint(FailPoints::force_disable_region_persister_compatibility_mode);
+        RegionMap new_regions = persister.restore(nullptr, config);
+        ASSERT_NE(persister.page_storage, nullptr);
+        ASSERT_EQ(persister.stable_page_storage, nullptr);
+        // Try to read
+        for (size_t i = 0; i < region_num; ++i)
+        {
+            auto new_iter = new_regions.find(i);
+            ASSERT_NE(new_iter, new_regions.end());
+            auto old_region = regions[i];
+            auto new_region = new_regions[i];
+            ASSERT_EQ(*new_region, *old_region);
+        }
+        // Try to write more regions under normal mode
+        for (size_t i = region_num; i < region_num + region_num_under_nromal_mode; ++i)
+        {
+            auto region = std::make_shared<Region>(createRegionMeta(i, table_id));
+            TiKVKey key = RecordKVFormat::genKey(table_id, i, tso++);
+            region->insert("default", TiKVKey::copyFrom(key), TiKVValue("value1"));
+            region->insert("write", TiKVKey::copyFrom(key), RecordKVFormat::encodeWriteCfValue('P', 0));
+            region->insert("lock", TiKVKey::copyFrom(key), RecordKVFormat::encodeLockCfValue('P', "", 0, 0));
+
+            persister.persist(*region);
+
+            regions.emplace(region->id(), region);
+        }
+    }
+
+    {
+        RegionPersister persister(ctx, region_manager);
+        // Restore normally, should run in normal mode.
+        RegionMap new_regions = persister.restore(nullptr, config);
+        ASSERT_NE(persister.page_storage, nullptr);
+        ASSERT_EQ(persister.stable_page_storage, nullptr);
+        // Try to read
+        for (size_t i = 0; i < region_num + region_num_under_nromal_mode; ++i)
+        {
+            auto new_iter = new_regions.find(i);
+            ASSERT_NE(new_iter, new_regions.end()) << " region:" << i;
+            auto old_region = regions[i];
+            auto new_region = new_regions[i];
+            ASSERT_EQ(*new_region, *old_region) << " region:" << i;
+        }
+    }
+}
+CATCH
+
 
 void RegionPersister_test::testFunc(const String & path, const PageStorage::Config & config, int region_num, bool is_gc, bool clean_up)
 {
@@ -391,4 +516,5 @@ TEST_F(RegionPersister_test, DISABLED_persister_sync_on_write)
     runTest(dir_path + "region_persist_storage_sow_true", true);
 }
 
-} // namespace DB::tests
+} // namespace tests
+} // namespace DB

dbms/src/test_utils/TiflashTestBasic.h

@@ -126,7 +126,7 @@ class TiFlashTestEnv
         {
             context.getTMTContext();
             auto paths = getPathPool(testdata_path);
-            context.setPathPool(paths.first, paths.second, Strings{}, false, context.getPathCapacity(), context.getFileProvider());
+            context.setPathPool(paths.first, paths.second, Strings{}, true, context.getPathCapacity(), context.getFileProvider());
         }
         catch (Exception & e)
         {
@@ -148,7 +148,7 @@ class TiFlashTestEnv
                 testdata_path.emplace_back(getTemporaryPath());
             context.initializePathCapacityMetric(testdata_path, 0);
             auto paths = getPathPool(testdata_path);
-            context.setPathPool(paths.first, paths.second, Strings{}, false, context.getPathCapacity(), context.getFileProvider());
+            context.setPathPool(paths.first, paths.second, Strings{}, true, context.getPathCapacity(), context.getFileProvider());
             context.createTMTContext({}, {"default"}, TiDB::StorageEngine::TMT, false);
 
             context.getTMTContext().restore();