fix bug: RefPage of PageEntriesVersionSetWithDelta (#246)

pingcap · Sep 26, 2019 · 071c0d6 · 071c0d6
1 parent 1d5ab64
commit 071c0d6
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 12 deletions.
diff --git a/dbms/src/Storages/Page/VersionSet/PageEntriesVersionSetWithDelta.cpp b/dbms/src/Storages/Page/VersionSet/PageEntriesVersionSetWithDelta.cpp
@@ -218,7 +218,7 @@ void DeltaVersionEditAcceptor::applyRef(PageEntriesEdit::EditRecord & rec)
     // if `page_id` is a ref-id, collapse the ref-path to actual PageId
     // eg. exist RefPage2 -> Page1, add RefPage3 -> RefPage2, collapse to RefPage3 -> Page1
     const PageId normal_page_id = view->resolveRefId(rec.ori_page_id);
-    auto         old_entry      = view->findNormalPageEntry(normal_page_id);
+    const auto   old_entry      = view->findNormalPageEntry(normal_page_id);
     if (likely(old_entry != nullptr))
     {
         // if RefPage{ref_id} already exist, release that ref first
@@ -234,7 +234,7 @@ void DeltaVersionEditAcceptor::applyRef(PageEntriesEdit::EditRecord & rec)
         // increase entry's ref-count
         auto new_entry = *old_entry;
         new_entry.ref += 1;
-        current_version->normal_pages[rec.page_id] = new_entry;
+        current_version->normal_pages[normal_page_id] = new_entry;
     }
     else
     {

diff --git a/dbms/src/Storages/Page/VersionSet/PageEntriesView.cpp b/dbms/src/Storages/Page/VersionSet/PageEntriesView.cpp
@@ -134,7 +134,7 @@ std::set<PageId> PageEntriesView::validNormalPageIds() const
         }
         for (auto & [page_id, entry] : node->normal_pages)
         {
-            if (entry.ref != 0)
+            if (!entry.isTombstone())
                 valid_normal_pages.insert(page_id);
         }
     }

diff --git a/dbms/src/Storages/Page/tests/gtest_page_map_version_set.cpp b/dbms/src/Storages/Page/tests/gtest_page_map_version_set.cpp
@@ -220,6 +220,23 @@ TYPED_TEST_P(PageMapVersionSet_test, ApplyEditWithReadLock3)
     EXPECT_EQ(versions.size(), 1UL);
 }
 
+namespace
+{
+std::set<PageId> getNormalPageIDs(const PageEntriesVersionSet::SnapshotPtr &s)
+{
+    std::set<PageId> ids;
+    for (auto iter = s->version()->pages_cbegin(); iter != s->version()->pages_cend(); iter++)
+        ids.insert(iter->first);
+    return ids;
+}
+
+std::set<PageId> getNormalPageIDs(const PageEntriesVersionSetWithDelta::SnapshotPtr &s)
+{
+    return s->version()->validNormalPageIds();
+}
+
+} // namespace
+
 TYPED_TEST_P(PageMapVersionSet_test, Restore)
 {
     TypeParam versions(this->config_);
@@ -278,19 +295,37 @@ TYPED_TEST_P(PageMapVersionSet_test, Restore)
     ASSERT_NE(entry3, nullptr);
     ASSERT_EQ(entry3->checksum, 3UL);
 
-    std::set<PageId> valid_normal_page_ids;
-    if constexpr (std::is_same_v<TypeParam, PageEntriesVersionSet>)
+    std::set<PageId> valid_normal_page_ids = getNormalPageIDs(s);
+    ASSERT_FALSE(valid_normal_page_ids.count(1) > 0);
+    ASSERT_FALSE(valid_normal_page_ids.count(2) > 0);
+    ASSERT_TRUE(valid_normal_page_ids.count(3) > 0);
+}
+
+TYPED_TEST_P(PageMapVersionSet_test, PutRefPage)
+{
+    TypeParam versions(this->config_);
     {
-        for (auto iter = s->version()->pages_cbegin(); iter != s->version()->pages_cend(); iter++)
-            valid_normal_page_ids.insert(iter->first);
+        PageEntriesEdit edit;
+        PageEntry e;
+        e.checksum = 0xf;
+        edit.put(2, e);
+        versions.apply(edit);
     }
-    else
+    auto s1 = versions.getSnapshot();
+    ASSERT_EQ(s1->version()->at(2).checksum, 0xfUL);
+
+    //  Put RefPage3 -> Page2
     {
-        valid_normal_page_ids = s->version()->validNormalPageIds();
+        PageEntriesEdit edit;
+        edit.ref(3, 2);
+        versions.apply(edit);
     }
-    ASSERT_EQ(valid_normal_page_ids.count(1), 0UL);
-    ASSERT_EQ(valid_normal_page_ids.count(2), 0UL);
-    ASSERT_EQ(valid_normal_page_ids.count(3), 1UL);
+    auto s2 = versions.getSnapshot();
+    ASSERT_EQ(s2->version()->at(3).checksum, 0xfUL);
+
+    std::set<PageId> valid_normal_page_ids = getNormalPageIDs(s2);
+    ASSERT_TRUE(valid_normal_page_ids.count(2) > 0);
+    ASSERT_FALSE(valid_normal_page_ids.count(3) > 0);
 }
 
 TYPED_TEST_P(PageMapVersionSet_test, GcConcurrencyDelPage)
@@ -637,6 +672,7 @@ REGISTER_TYPED_TEST_CASE_P(PageMapVersionSet_test,
                            GcConcurrencyDelPage,
                            GcPageMove,
                            GcConcurrencySetPage,
+                           PutRefPage,
                            UpdateOnRefPage,
                            UpdateOnRefPage2,
                            IsRefId,