From b34cd1d7c4dfe0cf3a84b21fa5c1b6e0583e0283 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Thu, 26 May 2022 12:46:47 +0800 Subject: [PATCH] Fix v3 can not find entry in mix mode. (#5001) close pingcap/tiflash#5000 --- dbms/src/Storages/Page/V3/PageDirectory.cpp | 22 +++++- .../V3/tests/gtest_page_storage_mix_mode.cpp | 76 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/Page/V3/PageDirectory.cpp b/dbms/src/Storages/Page/V3/PageDirectory.cpp index d944fe422d3..e9b754854b8 100644 --- a/dbms/src/Storages/Page/V3/PageDirectory.cpp +++ b/dbms/src/Storages/Page/V3/PageDirectory.cpp @@ -797,7 +797,17 @@ PageIDAndEntryV3 PageDirectory::get(PageIdV3Internal page_id, const PageDirector } } - throw Exception(fmt::format("Fail to get entry [page_id={}] [seq={}] [resolve_id={}] [resolve_ver={}]", page_id, snap->sequence, id_to_resolve, ver_to_resolve), ErrorCodes::PS_ENTRY_NO_VALID_VERSION); + // Only mix mode throw_on_not_exist is false. + // In mix mode, storage will create a snapshot contains V2 and V3. + // If we find a del entry in V3, we still need find it in V2. + if (throw_on_not_exist) + { + throw Exception(fmt::format("Fail to get entry [page_id={}] [seq={}] [resolve_id={}] [resolve_ver={}]", page_id, snap->sequence, id_to_resolve, ver_to_resolve), ErrorCodes::PS_ENTRY_NO_VALID_VERSION); + } + else + { + return PageIDAndEntryV3{page_id, PageEntryV3{.file_id = INVALID_BLOBFILE_ID}}; + } } std::pair PageDirectory::get(const PageIdV3Internals & page_ids, const PageDirectorySnapshotPtr & snap, bool throw_on_not_exist) const @@ -846,7 +856,15 @@ std::pair PageDirectory::get(const PageIdV3Internal break; // continue the resolving } } - throw Exception(fmt::format("Fail to get entry [page_id={}] [ver={}] [resolve_id={}] [resolve_ver={}] [idx={}]", page_id, init_ver_to_resolve, id_to_resolve, ver_to_resolve, idx), ErrorCodes::PS_ENTRY_NO_VALID_VERSION); + + if (throw_on_not_exist) + { + throw Exception(fmt::format("Fail to get entry [page_id={}] [ver={}] [resolve_id={}] [resolve_ver={}] [idx={}]", page_id, init_ver_to_resolve, id_to_resolve, ver_to_resolve, idx), ErrorCodes::PS_ENTRY_NO_VALID_VERSION); + } + else + { + return false; + } }; PageIDAndEntriesV3 id_entries; diff --git a/dbms/src/Storages/Page/V3/tests/gtest_page_storage_mix_mode.cpp b/dbms/src/Storages/Page/V3/tests/gtest_page_storage_mix_mode.cpp index f7ce3180172..97eac841018 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_page_storage_mix_mode.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_page_storage_mix_mode.cpp @@ -567,6 +567,82 @@ try } CATCH +TEST_F(PageStorageMixedTest, ReadWithSnapshotAfterMergeDelta) +try +{ + UInt64 tag = 0; + const size_t buf_sz = 1024; + char c_buff[buf_sz]; + for (size_t i = 0; i < buf_sz; ++i) + { + c_buff[i] = i % 0xff; + } + { + WriteBatch batch; + ReadBufferPtr buff = std::make_shared(c_buff, sizeof(c_buff)); + batch.putPage(1, tag, buff, buf_sz); + buff = std::make_shared(c_buff, sizeof(c_buff)); + batch.putPage(2, tag, buff, buf_sz, {20, 120, 400, 200, 15, 75, 170, 24}); + page_writer_v2->write(std::move(batch), nullptr); + } + ASSERT_EQ(reloadMixedStoragePool(), PageStorageRunMode::MIX_MODE); + const size_t buf_sz2 = 2048; + char c_buff2[buf_sz2] = {0}; + { + WriteBatch batch; + ReadBufferPtr buff2 = std::make_shared(c_buff2, sizeof(c_buff2)); + batch.putPage(3, tag, buff2, buf_sz2); + page_writer_mix->write(std::move(batch), nullptr); + } + // Thread A create snapshot for read + auto snapshot_mix_before_merge_delta = page_reader_mix->getSnapshot("ReadWithSnapshotAfterMergeDelta"); + { + auto page_reader_mix_with_snap = storage_pool_mix->newLogReader(nullptr, snapshot_mix_before_merge_delta); + const auto & page1 = page_reader_mix_with_snap.read(1); + const auto & page2 = page_reader_mix_with_snap.read(2); + const auto & page3 = page_reader_mix_with_snap.read(3); + ASSERT_PAGE_EQ(c_buff, buf_sz, page1, 1); + ASSERT_PAGE_EQ(c_buff, buf_sz, page2, 2); + ASSERT_PAGE_EQ(c_buff2, buf_sz2, page3, 3); + } + { + auto page_reader_mix_with_snap = storage_pool_mix->newLogReader(nullptr, true, "ReadWithSnapshotAfterMergeDelta"); + const auto & page1 = page_reader_mix_with_snap.read(1); + const auto & page2 = page_reader_mix_with_snap.read(2); + const auto & page3 = page_reader_mix_with_snap.read(3); + ASSERT_PAGE_EQ(c_buff, buf_sz, page1, 1); + ASSERT_PAGE_EQ(c_buff, buf_sz, page2, 2); + ASSERT_PAGE_EQ(c_buff2, buf_sz2, page3, 3); + } + // Thread B apply merge delta, create page 4, and delete the origin page 1, 3 + { + WriteBatch batch; + ReadBufferPtr buff2 = std::make_shared(c_buff2, sizeof(c_buff2)); + batch.putPage(4, tag, buff2, buf_sz2); + batch.delPage(1); + batch.delPage(3); + page_writer_mix->write(std::move(batch), nullptr); + } + // Thread A continue to read 1, 3 + { + auto page_reader_mix_with_snap = storage_pool_mix->newLogReader(nullptr, snapshot_mix_before_merge_delta); + // read 1, 3 with snapshot, should be success + const auto & page1 = page_reader_mix_with_snap.read(1); + const auto & page3 = page_reader_mix_with_snap.read(3); + ASSERT_PAGE_EQ(c_buff, buf_sz, page1, 1); + ASSERT_PAGE_EQ(c_buff2, buf_sz2, page3, 3); + ASSERT_THROW(page_reader_mix_with_snap.read(4), DB::Exception); + } + { + // Revert v3 + WriteBatch batch; + batch.delPage(3); + batch.delPage(4); + page_writer_mix->write(std::move(batch), nullptr); + } +} +CATCH + } // namespace PS::V3::tests } // namespace DB