From ee164e72cfe6a47b018c5a5bc1d96e838ff0c375 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 12 May 2022 14:41:08 +0800 Subject: [PATCH 01/10] Add a force transfrom kvstore from v2 to v3 --- .../Storages/Transaction/RegionPersister.cpp | 76 +++++++++++++++++-- .../Storages/Transaction/RegionPersister.h | 2 + 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 77901922e2f..918df5550e3 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -170,6 +170,53 @@ PS::V1::PageStorage::Config getV1PSConfig(const PS::V2::PageStorage::Config & co return c; } +void RegionPersister::forceTransformKVStoreV2toV3() +{ + assert(page_reader != nullptr); + assert(page_writer != nullptr); + + Pages pages_transform = {}; + auto meta_transform_acceptor = [&](const DB::Page & page) { + pages_transform.emplace_back(page); + }; + + page_reader->traverse(meta_transform_acceptor, /*only_v2*/ true, /*only_v3*/ false); + + WriteBatch write_batch_transform{KVSTORE_NAMESPACE_ID}; + + for (const auto & page_transform : pages_transform) + { + // Check pages have not contain field offset + // Also get the tag of page_id + const auto & page_transform_entry = page_reader->getPageEntry(page_transform.page_id); + if (!page_transform_entry.field_offsets.empty()) + { + throw Exception(fmt::format("Can't transfrom kvstore from V2 to V3, [page_id={}]", + page_transform.page_id), + ErrorCodes::LOGICAL_ERROR); + } + + write_batch_transform.putPage(page_transform.page_id, // + page_transform_entry.tag, + std::make_shared(page_transform.data.begin(), + page_transform.data.size()), + page_transform.data.size()); + } + + // Will rewrite into V3. + page_writer->write(std::move(write_batch_transform), nullptr); + + WriteBatch write_batch_del_v2{KVSTORE_NAMESPACE_ID}; + + for (const auto & page_transform : pages_transform) + { + write_batch_del_v2.delPage(page_transform.page_id); + } + + // DEL must call after rewrite. + page_writer->writeIntoV2(std::move(write_batch_del_v2), nullptr); +} + RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, PageStorage::Config config) { { @@ -247,17 +294,36 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, page_storage_v2->restore(); page_storage_v3->restore(); - if (page_storage_v2->getNumberOfPages() == 0) - { + auto changeToOnlyV3 = [&]() { page_storage_v2 = nullptr; - run_mode = PageStorageRunMode::ONLY_V3; page_writer = std::make_shared(run_mode, /*storage_v2_*/ nullptr, page_storage_v3); page_reader = std::make_shared(run_mode, ns_id, /*storage_v2_*/ nullptr, page_storage_v3, global_context.getReadLimiter()); - } - else + + run_mode = PageStorageRunMode::ONLY_V3; + }; + + if (const auto & kvstore_remain_pages = page_storage_v2->getNumberOfPages(); kvstore_remain_pages != 0) { page_writer = std::make_shared(run_mode, page_storage_v2, page_storage_v3); page_reader = std::make_shared(run_mode, ns_id, page_storage_v2, page_storage_v3, global_context.getReadLimiter()); + + forceTransformKVStoreV2toV3(); + const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); + LOG_FMT_INFO(log, "Current kvstore translate to V3 finished. [ns_id={}] [done={}] [remain_before_translate_pages={}], [remain_after_translate_pages={}]", // + ns_id, + kvstore_remain_pages_after_transform == 0, + kvstore_remain_pages, + kvstore_remain_pages_after_transform); + + if (kvstore_remain_pages_after_transform == 0) + { + changeToOnlyV3(); + } // else do nothing, still use MIX_MODE + } + else // no need do transform + { + LOG_FMT_INFO(log, "Current kvstore translate already done before restored."); + changeToOnlyV3(); } break; } diff --git a/dbms/src/Storages/Transaction/RegionPersister.h b/dbms/src/Storages/Transaction/RegionPersister.h index a4f611cbdbb..f2828add202 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.h +++ b/dbms/src/Storages/Transaction/RegionPersister.h @@ -61,6 +61,8 @@ class RegionPersister final : private boost::noncopyable private: #endif + void forceTransformKVStoreV2toV3(); + void doPersist(RegionCacheWriteElement & region_write_buffer, const RegionTaskLock & lock, const Region & region); void doPersist(const Region & region, const RegionTaskLock * lock); From e8619c4b17b34d43c6e9fcb277856a87ca7654c5 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 12 May 2022 20:01:42 +0800 Subject: [PATCH 02/10] update --- dbms/src/Storages/Transaction/RegionPersister.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 918df5550e3..49d0af6eabd 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -182,10 +182,10 @@ void RegionPersister::forceTransformKVStoreV2toV3() page_reader->traverse(meta_transform_acceptor, /*only_v2*/ true, /*only_v3*/ false); - WriteBatch write_batch_transform{KVSTORE_NAMESPACE_ID}; - + WriteBatch write_batch_del_v2{KVSTORE_NAMESPACE_ID}; for (const auto & page_transform : pages_transform) { + WriteBatch write_batch_transform{KVSTORE_NAMESPACE_ID}; // Check pages have not contain field offset // Also get the tag of page_id const auto & page_transform_entry = page_reader->getPageEntry(page_transform.page_id); @@ -201,15 +201,12 @@ void RegionPersister::forceTransformKVStoreV2toV3() std::make_shared(page_transform.data.begin(), page_transform.data.size()), page_transform.data.size()); - } - // Will rewrite into V3. - page_writer->write(std::move(write_batch_transform), nullptr); + // Will rewrite into V3 one by one. + // The region data is big. It is not a good idea to combine pages. + page_writer->write(std::move(write_batch_transform), nullptr); - WriteBatch write_batch_del_v2{KVSTORE_NAMESPACE_ID}; - - for (const auto & page_transform : pages_transform) - { + // Record del page_id write_batch_del_v2.delPage(page_transform.page_id); } From 111a3223ae24b63d206a26d1cebd29986d057b9a Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 13 May 2022 12:18:03 +0800 Subject: [PATCH 03/10] fix --- dbms/src/Storages/Transaction/RegionPersister.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 49d0af6eabd..38613c77355 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -291,7 +291,7 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, page_storage_v2->restore(); page_storage_v3->restore(); - auto changeToOnlyV3 = [&]() { + auto change_to_only_v3 = [&]() { page_storage_v2 = nullptr; page_writer = std::make_shared(run_mode, /*storage_v2_*/ nullptr, page_storage_v3); page_reader = std::make_shared(run_mode, ns_id, /*storage_v2_*/ nullptr, page_storage_v3, global_context.getReadLimiter()); @@ -314,13 +314,13 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, if (kvstore_remain_pages_after_transform == 0) { - changeToOnlyV3(); + change_to_only_v3(); } // else do nothing, still use MIX_MODE } else // no need do transform { LOG_FMT_INFO(log, "Current kvstore translate already done before restored."); - changeToOnlyV3(); + change_to_only_v3(); } break; } From 776802a7025f8ac295fa3e60ecbf95aa5ed215cc Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 13 May 2022 14:22:14 +0800 Subject: [PATCH 04/10] changed --- .../Storages/Transaction/RegionPersister.cpp | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 38613c77355..6a900933846 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -175,40 +175,35 @@ void RegionPersister::forceTransformKVStoreV2toV3() assert(page_reader != nullptr); assert(page_writer != nullptr); - Pages pages_transform = {}; - auto meta_transform_acceptor = [&](const DB::Page & page) { - pages_transform.emplace_back(page); - }; - - page_reader->traverse(meta_transform_acceptor, /*only_v2*/ true, /*only_v3*/ false); - WriteBatch write_batch_del_v2{KVSTORE_NAMESPACE_ID}; - for (const auto & page_transform : pages_transform) - { + auto meta_transform_acceptor = [&](const DB::Page & page) { WriteBatch write_batch_transform{KVSTORE_NAMESPACE_ID}; // Check pages have not contain field offset // Also get the tag of page_id - const auto & page_transform_entry = page_reader->getPageEntry(page_transform.page_id); + const auto & page_transform_entry = page_reader->getPageEntry(page.page_id); if (!page_transform_entry.field_offsets.empty()) { - throw Exception(fmt::format("Can't transfrom kvstore from V2 to V3, [page_id={}]", - page_transform.page_id), + throw Exception(fmt::format("Can't transfrom kvstore from V2 to V3, [page_id={}] {}", + page.page_id, + page_transform_entry.toDebugString()), ErrorCodes::LOGICAL_ERROR); } - write_batch_transform.putPage(page_transform.page_id, // + write_batch_transform.putPage(page.page_id, // page_transform_entry.tag, - std::make_shared(page_transform.data.begin(), - page_transform.data.size()), - page_transform.data.size()); + std::make_shared(page.data.begin(), + page.data.size()), + page.data.size()); // Will rewrite into V3 one by one. // The region data is big. It is not a good idea to combine pages. page_writer->write(std::move(write_batch_transform), nullptr); // Record del page_id - write_batch_del_v2.delPage(page_transform.page_id); - } + write_batch_del_v2.delPage(page.page_id); + }; + + page_reader->traverse(meta_transform_acceptor, /*only_v2*/ true, /*only_v3*/ false); // DEL must call after rewrite. page_writer->writeIntoV2(std::move(write_batch_del_v2), nullptr); From 885ab0f2447a34e456d9f0f62a1aed41b3e11261 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 13 May 2022 17:21:29 +0800 Subject: [PATCH 05/10] update --- .../Storages/Transaction/RegionPersister.cpp | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 6a900933846..1208a549b68 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -286,14 +286,6 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, page_storage_v2->restore(); page_storage_v3->restore(); - auto change_to_only_v3 = [&]() { - page_storage_v2 = nullptr; - page_writer = std::make_shared(run_mode, /*storage_v2_*/ nullptr, page_storage_v3); - page_reader = std::make_shared(run_mode, ns_id, /*storage_v2_*/ nullptr, page_storage_v3, global_context.getReadLimiter()); - - run_mode = PageStorageRunMode::ONLY_V3; - }; - if (const auto & kvstore_remain_pages = page_storage_v2->getNumberOfPages(); kvstore_remain_pages != 0) { page_writer = std::make_shared(run_mode, page_storage_v2, page_storage_v3); @@ -301,22 +293,28 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, forceTransformKVStoreV2toV3(); const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); - LOG_FMT_INFO(log, "Current kvstore translate to V3 finished. [ns_id={}] [done={}] [remain_before_translate_pages={}], [remain_after_translate_pages={}]", // + LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [remain_before_translate_pages={}], [remain_after_translate_pages={}]", // ns_id, kvstore_remain_pages_after_transform == 0, kvstore_remain_pages, kvstore_remain_pages_after_transform); - if (kvstore_remain_pages_after_transform == 0) + if (kvstore_remain_pages_after_transform != 0) { - change_to_only_v3(); - } // else do nothing, still use MIX_MODE + throw Exception("KVStore transform failed. Still have some data exist in V2", ErrorCodes::LOGICAL_ERROR); + } } else // no need do transform { LOG_FMT_INFO(log, "Current kvstore translate already done before restored."); - change_to_only_v3(); } + + // change run_mode to ONLY_V3 + page_storage_v2 = nullptr; + page_writer = std::make_shared(run_mode, /*storage_v2_*/ nullptr, page_storage_v3); + page_reader = std::make_shared(run_mode, ns_id, /*storage_v2_*/ nullptr, page_storage_v3, global_context.getReadLimiter()); + + run_mode = PageStorageRunMode::ONLY_V3; break; } } From f456c23986a2e764aa103128b5698b3fb7596fb4 Mon Sep 17 00:00:00 2001 From: JaySon Date: Fri, 13 May 2022 18:24:59 +0800 Subject: [PATCH 06/10] add log --- dbms/src/Storages/Transaction/RegionPersister.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 1208a549b68..b49e1d88a6a 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -291,6 +291,7 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, page_writer = std::make_shared(run_mode, page_storage_v2, page_storage_v3); page_reader = std::make_shared(run_mode, ns_id, page_storage_v2, page_storage_v3, global_context.getReadLimiter()); + LOG_FMT_INFO(logger, "Current kvstore transform to V3 begin [pages_before_transform={}]", kvstore_remain_pages); forceTransformKVStoreV2toV3(); const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [remain_before_translate_pages={}], [remain_after_translate_pages={}]", // From c81634ef34b947ac165b03b99ed928ea69db0720 Mon Sep 17 00:00:00 2001 From: JaySon Date: Fri, 13 May 2022 18:26:00 +0800 Subject: [PATCH 07/10] Apply suggestions from code review --- dbms/src/Storages/Transaction/RegionPersister.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index b49e1d88a6a..b9a7064d86a 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -294,7 +294,7 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, LOG_FMT_INFO(logger, "Current kvstore transform to V3 begin [pages_before_transform={}]", kvstore_remain_pages); forceTransformKVStoreV2toV3(); const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); - LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [remain_before_translate_pages={}], [remain_after_translate_pages={}]", // + LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [pages_before_transform ={}], [pages_after_transform ={}]", // ns_id, kvstore_remain_pages_after_transform == 0, kvstore_remain_pages, From 7fb3c31e21ced1978de3b160d49a6856323433b1 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Fri, 13 May 2022 18:26:32 +0800 Subject: [PATCH 08/10] Update dbms/src/Storages/Transaction/RegionPersister.cpp Co-authored-by: JaySon --- dbms/src/Storages/Transaction/RegionPersister.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index b9a7064d86a..9fa6031a1e1 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -294,7 +294,7 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, LOG_FMT_INFO(logger, "Current kvstore transform to V3 begin [pages_before_transform={}]", kvstore_remain_pages); forceTransformKVStoreV2toV3(); const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); - LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [pages_before_transform ={}], [pages_after_transform ={}]", // + LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [pages_before_transform ={}] [pages_after_transform ={}]", // ns_id, kvstore_remain_pages_after_transform == 0, kvstore_remain_pages, From 792f6f2ff4545c9d4a03aad6975c7fbc181327f7 Mon Sep 17 00:00:00 2001 From: JaySon Date: Fri, 13 May 2022 18:28:48 +0800 Subject: [PATCH 09/10] Apply suggestions from code review --- dbms/src/Storages/Transaction/RegionPersister.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 9fa6031a1e1..6493e0ad3fe 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -294,7 +294,7 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, LOG_FMT_INFO(logger, "Current kvstore transform to V3 begin [pages_before_transform={}]", kvstore_remain_pages); forceTransformKVStoreV2toV3(); const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); - LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [pages_before_transform ={}] [pages_after_transform ={}]", // + LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [pages_before_transform={}] [pages_after_transform={}]", // ns_id, kvstore_remain_pages_after_transform == 0, kvstore_remain_pages, From 8570779f40a46e5c470b8a61949251fb8a01639e Mon Sep 17 00:00:00 2001 From: JaySon Date: Fri, 13 May 2022 18:45:37 +0800 Subject: [PATCH 10/10] Apply suggestions from code review --- dbms/src/Storages/Transaction/RegionPersister.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/Transaction/RegionPersister.cpp b/dbms/src/Storages/Transaction/RegionPersister.cpp index 6493e0ad3fe..8e6ed6821df 100644 --- a/dbms/src/Storages/Transaction/RegionPersister.cpp +++ b/dbms/src/Storages/Transaction/RegionPersister.cpp @@ -291,7 +291,7 @@ RegionMap RegionPersister::restore(const TiFlashRaftProxyHelper * proxy_helper, page_writer = std::make_shared(run_mode, page_storage_v2, page_storage_v3); page_reader = std::make_shared(run_mode, ns_id, page_storage_v2, page_storage_v3, global_context.getReadLimiter()); - LOG_FMT_INFO(logger, "Current kvstore transform to V3 begin [pages_before_transform={}]", kvstore_remain_pages); + LOG_FMT_INFO(log, "Current kvstore transform to V3 begin [pages_before_transform={}]", kvstore_remain_pages); forceTransformKVStoreV2toV3(); const auto & kvstore_remain_pages_after_transform = page_storage_v2->getNumberOfPages(); LOG_FMT_INFO(log, "Current kvstore transfrom to V3 finished. [ns_id={}] [done={}] [pages_before_transform={}] [pages_after_transform={}]", //