fix: should checkpoint the whole page

Author: zz-jason

source/concurrency-recovery/Recovery.hpp

@@ -91,7 +91,7 @@ inline bool Recovery::Run() {
 
   Analysis();
 
-  // Redo();
+  Redo();
 
   // if (Undo()) {
   //   return true;

source/storage/btree/core/BTreeGeneric.hpp

@@ -107,8 +107,10 @@ class BTreeGeneric : public leanstore::storage::BufferManagedTree {
   /// Called by buffer manager before eviction
   virtual SpaceCheckResult checkSpaceUtilization(BufferFrame& bf) override;
 
-  // pre: source buffer frame is shared latched
-  virtual void checkpoint(BufferFrame& bf, u8* dest) override;
+  /// Flush the page content in the buffer frame to disk
+  ///
+  /// NOTE: The source buffer frame should be shared latched
+  virtual void Checkpoint(BufferFrame& bf, void* dest) override;
 
   virtual void undo(const u8*, const u64) override {
     LOG(FATAL) << "undo is unsupported";
@@ -386,22 +388,23 @@ inline SpaceCheckResult BTreeGeneric::checkSpaceUtilization(BufferFrame& bf) {
   }
 }
 
-inline void BTreeGeneric::checkpoint(BufferFrame& bf, u8* dest) {
-  std::memcpy(dest, bf.page.mPayload, EFFECTIVE_PAGE_SIZE);
-  auto& destNode = *reinterpret_cast<BTreeNode*>(dest);
+inline void BTreeGeneric::Checkpoint(BufferFrame& bf, void* dest) {
+  std::memcpy(dest, &bf.page, PAGE_SIZE);
+  auto destPage = reinterpret_cast<Page*>(dest);
+  auto destNode = reinterpret_cast<BTreeNode*>(destPage->mPayload);
 
-  if (!destNode.mIsLeaf) {
+  if (!destNode->mIsLeaf) {
     // Replace all child swip to their page ID
-    for (u64 i = 0; i < destNode.mNumSeps; i++) {
-      if (!destNode.getChild(i).isEVICTED()) {
-        auto& childBf = destNode.getChild(i).asBufferFrameMasked();
-        destNode.getChild(i).evict(childBf.header.mPageId);
+    for (u64 i = 0; i < destNode->mNumSeps; i++) {
+      if (!destNode->getChild(i).isEVICTED()) {
+        auto& childBf = destNode->getChild(i).asBufferFrameMasked();
+        destNode->getChild(i).evict(childBf.header.mPageId);
       }
     }
     // Replace right most child swip to page id
-    if (!destNode.mRightMostChildSwip.isEVICTED()) {
-      auto& childBf = destNode.mRightMostChildSwip.asBufferFrameMasked();
-      destNode.mRightMostChildSwip.evict(childBf.header.mPageId);
+    if (!destNode->mRightMostChildSwip.isEVICTED()) {
+      auto& childBf = destNode->mRightMostChildSwip.asBufferFrameMasked();
+      destNode->mRightMostChildSwip.evict(childBf.header.mPageId);
     }
   }
 }

source/storage/buffer-manager/BufferManager.cpp

@@ -136,12 +136,10 @@ void BufferManager::writeAllBufferFrames() {
       auto& bf = mBfs[i];
       bf.header.mLatch.mutex.lock();
       if (!bf.isFree()) {
-        page.mBTreeId = bf.page.mBTreeId;
-        page.mMagicDebuging = bf.header.mPageId;
-        TreeRegistry::sInstance->checkpoint(bf.page.mBTreeId, bf,
-                                            page.mPayload);
-        s64 ret = pwrite(mPageFd, &page, PAGE_SIZE, bf.header.mPageId * PAGE_SIZE);
-        ENSURE(ret == PAGE_SIZE);
+        TreeRegistry::sInstance->Checkpoint(bf.page.mBTreeId, bf, &page);
+        s64 ret =
+            pwrite(mPageFd, &page, PAGE_SIZE, bf.header.mPageId * PAGE_SIZE);
+        DCHECK(ret == PAGE_SIZE);
       }
       bf.header.mLatch.mutex.unlock();
     }
@@ -152,9 +150,7 @@ void BufferManager::WriteBufferFrame(BufferFrame& bf) {
   Page page;
   bf.header.mLatch.mutex.lock();
   if (!bf.isFree()) {
-    page.mBTreeId = bf.page.mBTreeId;
-    page.mMagicDebuging = bf.header.mPageId;
-    TreeRegistry::sInstance->checkpoint(bf.page.mBTreeId, bf, page.mPayload);
+    TreeRegistry::sInstance->Checkpoint(bf.page.mBTreeId, bf, &page);
     s64 ret = pwrite(mPageFd, &page, PAGE_SIZE, bf.header.mPageId * PAGE_SIZE);
     ENSURE(ret == PAGE_SIZE);
   }

source/storage/buffer-manager/TreeRegistry.hpp

@@ -59,8 +59,8 @@ class BufferManagedTree {
     LOG(FATAL) << "BufferManagedTree::checkSpaceUtilization is unimplemented ";
   }
 
-  virtual void checkpoint(BufferFrame&, u8*) {
-    LOG(FATAL) << "BufferManagedTree::checkpoint is unimplemented";
+  virtual void Checkpoint(BufferFrame&, void*) {
+    LOG(FATAL) << "BufferManagedTree::Checkpoint is unimplemented";
   }
 
   virtual void undo(const u8*, const u64) {
@@ -228,13 +228,13 @@ class TreeRegistry {
   }
 
   // Pre: bf is shared/exclusive latched
-  inline void checkpoint(TREEID treeId, BufferFrame& bf, u8* dest) {
+  inline void Checkpoint(TREEID treeId, BufferFrame& bf, void* dest) {
     std::shared_lock sharedGuard(mMutex);
     auto it = mTrees.find(treeId);
     DLOG_IF(FATAL, it == mTrees.end())
         << "BufferManagedTree not find, treeId=" << treeId;
     auto& [tree, treeName] = it->second;
-    return tree->checkpoint(bf, dest);
+    return tree->Checkpoint(bf, dest);
   }
 
   // Recovery / SI

tests/BTreeVILoggingAndRecoveryTest.cpp

@@ -46,7 +46,7 @@ TEST_F(BTreeVILoggingAndRecoveryTest, SerializeAndDeserialize) {
   storage::btree::BTreeVI* btree;
 
   // prepare key-value pairs to insert
-  size_t numKVs(1);
+  size_t numKVs(10);
   std::vector<std::tuple<std::string, std::string>> kvToTest;
   for (size_t i = 0; i < numKVs; ++i) {
     std::string key("key_xxxxxxxxxxxx_" + std::to_string(i));
@@ -78,6 +78,12 @@ TEST_F(BTreeVILoggingAndRecoveryTest, SerializeAndDeserialize) {
     }
   });
 
+  cr::CRManager::sInstance->scheduleJobSync(0, [&]() {
+    rapidjson::Document doc(rapidjson::kObjectType);
+    leanstore::storage::btree::BTreeGeneric::ToJSON(*btree, &doc);
+    LOG(INFO) << "btree before destroy: " << leanstore::utils::JsonToStr(&doc);
+  });
+
   // meta file should be serialized during destructor.
   mLeanStore.reset(nullptr);
   FLAGS_recover = true;
@@ -87,6 +93,12 @@ TEST_F(BTreeVILoggingAndRecoveryTest, SerializeAndDeserialize) {
   EXPECT_TRUE(mLeanStore->GetBTreeVI(btreeName, &btree));
   EXPECT_NE(btree, nullptr);
 
+  cr::CRManager::sInstance->scheduleJobSync(0, [&]() {
+    rapidjson::Document doc(rapidjson::kObjectType);
+    leanstore::storage::btree::BTreeGeneric::ToJSON(*btree, &doc);
+    LOG(INFO) << "btree after recovery: " << leanstore::utils::JsonToStr(&doc);
+  });
+
   // lookup the restored btree
   cr::CRManager::sInstance->scheduleJobSync(0, [&]() {
     std::string copiedValue;