From cd77ac309360cf9dccfddb303933e729dc9acc19 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Mon, 23 May 2022 18:38:02 +0800 Subject: [PATCH 1/3] PageDirectory: need do gc twice in restore. --- .../Storages/Page/V3/PageDirectoryFactory.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp b/dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp index 40b12b64f06..7403f9e83a0 100644 --- a/dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp +++ b/dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp @@ -41,6 +41,22 @@ PageDirectoryPtr PageDirectoryFactory::create(String storage_name, FileProviderP // try to run GC again on some entries that are already marked as invalid in BlobStore. dir->gcInMemEntries(); + // MVCC gc must do twice when restore + // Because the first time call MVCC GC may not clean the deleted entries if being reference. + // But it will clean up the deleted reference. + // ex. + // P 1 + // R 2 -> 1 + // D 1 + // D 2 + // First time call gcInMemEntries(): + // - P 1 entry 0 being_ref_count is not zero, won't be cleaned. + // - R 2 entry 0 being_ref_count is zero, deref the P 1, make it being_ref_count is 0. Also R2 will be clean up. + // + // Second time call gcInMemEntries(): + // - P 1 being_ref_count is 0, it will be clean. + dir->gcInMemEntries(); + if (blob_stats) { // After all entries restored to `mvcc_table_directory`, only apply From 54a3fedd82828b3130150c6c293ee7988cbb11a6 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Tue, 24 May 2022 12:03:22 +0800 Subject: [PATCH 2/3] add ut --- .../Page/V3/tests/gtest_page_storage.cpp | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/dbms/src/Storages/Page/V3/tests/gtest_page_storage.cpp b/dbms/src/Storages/Page/V3/tests/gtest_page_storage.cpp index 91dfcaac6a8..b4ef2e7d4d2 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_page_storage.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_page_storage.cpp @@ -1329,5 +1329,35 @@ try } CATCH + +TEST_F(PageStorageTest, testGCTwice) +try +{ + // Make it in log_1_0 + { + WriteBatch batch; + batch.putExternal(1, 0); + page_storage->write(std::move(batch)); + } + + page_storage = reopenWithConfig(config); + + // Make it in log_2_0 + { + WriteBatch batch; + batch.putExternal(1, 0); + batch.putRefPage(2, 1); + batch.delPage(1); + batch.delPage(2); + page_storage->write(std::move(batch)); + } + page_storage = reopenWithConfig(config); + + auto alive_ids = page_storage->getAliveExternalPageIds(TEST_NAMESPACE_ID); + ASSERT_EQ(alive_ids.size(), 0); +} +CATCH + + } // namespace PS::V3::tests } // namespace DB From aec978923fb14c3d777e7594e8f1bc18ebfaa51b Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Tue, 24 May 2022 15:06:17 +0800 Subject: [PATCH 3/3] add mvcc test --- .../Page/V3/tests/gtest_page_directory.cpp | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp b/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp index dfa33824473..a7d29dc2320 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp @@ -2399,6 +2399,35 @@ try } CATCH +TEST_F(PageDirectoryGCTest, testGCTwice) +try +{ + PageEntryV3 entry_50_1{.file_id = 1, .size = 7890, .tag = 0, .offset = 0x123, .checksum = 0x4567}; + PageEntryV3 entry_50_2{.file_id = 2, .size = 7890, .tag = 0, .offset = 0x123, .checksum = 0x4567}; + + auto restore_from_edit = [](const PageEntriesEdit & edit) { + auto ctx = ::DB::tests::TiFlashTestEnv::getContext(); + auto provider = ctx.getFileProvider(); + auto path = getTemporaryPath(); + PSDiskDelegatorPtr delegator = std::make_shared(path); + PageDirectoryFactory factory; + auto d = factory.createFromEdit(getCurrentTestName(), provider, delegator, edit); + return d; + }; + + { + PageEntriesEdit edit; + edit.put(50, entry_50_1); + edit.put(50, entry_50_2); + edit.ref(51, 50); + edit.del(50); + edit.del(51); + auto page_ids = restore_from_edit(edit)->getAllPageIds(); + ASSERT_EQ(page_ids.size(), 0); + } +} +CATCH + #undef INSERT_ENTRY_TO #undef INSERT_ENTRY #undef INSERT_ENTRY_ACQ_SNAP