diff --git a/src/include/storage/page/page_guard.h b/src/include/storage/page/page_guard.h index 0e5761748..e2f885f02 100644 --- a/src/include/storage/page/page_guard.h +++ b/src/include/storage/page/page_guard.h @@ -102,6 +102,17 @@ class BasicPageGuard { return reinterpret_cast(GetDataMut()); } + /** The following methods are mainly used for testing */ + + auto GetPage() const -> Page * { return page_; } + + auto GetBPM() const -> BufferPoolManager * { return bpm_; } + + void SetNull() noexcept { + page_ = nullptr; + bpm_ = nullptr; + } + private: friend class ReadPageGuard; friend class WritePageGuard; @@ -118,6 +129,19 @@ class ReadPageGuard { ReadPageGuard(const ReadPageGuard &) = delete; auto operator=(const ReadPageGuard &) -> ReadPageGuard & = delete; + /** TODO(P2): Add implementation + * + * @brief Provide guard upgrade functionality, from ReadPageGuard to WritePageGuard + * + * You can call this function by: + * ReadPageGuard rg; + * WritePageGuard wg = rg.UpgradeWrite(); + * Note: You must not use the original ReadPageGuard after calling this function. + * + * @return WritePageGuard + */ + auto UpgradeWrite() -> WritePageGuard; + /** TODO(P2): Add implementation * * @brief Move constructor for ReadPageGuard @@ -166,6 +190,14 @@ class ReadPageGuard { return guard_.As(); } + /** The following methods are mainly used for testing */ + + auto GetPage() const -> Page * { return guard_.GetPage(); } + + auto GetBPM() const -> BufferPoolManager * { return guard_.GetBPM(); } + + void SetNull() noexcept { guard_.SetNull(); } + private: // You may choose to get rid of this and add your own private variables. BasicPageGuard guard_; @@ -233,6 +265,14 @@ class WritePageGuard { return guard_.AsMut(); } + /** The following methods are mainly used for testing */ + + auto GetPage() const -> Page * { return guard_.GetPage(); } + + auto GetBPM() const -> BufferPoolManager * { return guard_.GetBPM(); } + + void SetNull() noexcept { guard_.SetNull(); } + private: // You may choose to get rid of this and add your own private variables. BasicPageGuard guard_; diff --git a/src/storage/page/page_guard.cpp b/src/storage/page/page_guard.cpp index 343c210c4..f81a0e34e 100644 --- a/src/storage/page/page_guard.cpp +++ b/src/storage/page/page_guard.cpp @@ -3,6 +3,21 @@ namespace bustub { +auto BasicPageGuard::UpgradeRead() -> ReadPageGuard { + // TODO(p2): Your ReadPageGuard upgrade logic here + return {nullptr, nullptr}; +} + +auto BasicPageGuard::UpgradeWrite() -> WritePageGuard { + // TODO(p2): Your WritePageGuard upgrade logic here + return {nullptr, nullptr}; +} + +auto ReadPageGuard::UpgradeWrite() -> WritePageGuard { + // TODO(p2): Your WritePageGuard upgrade logic here + return {nullptr, nullptr}; +} + BasicPageGuard::BasicPageGuard(BasicPageGuard &&that) noexcept {} void BasicPageGuard::Drop() {} diff --git a/test/storage/b_plus_tree_concurrent_test.cpp b/test/storage/b_plus_tree_concurrent_test.cpp index 666c124a6..daa372053 100644 --- a/test/storage/b_plus_tree_concurrent_test.cpp +++ b/test/storage/b_plus_tree_concurrent_test.cpp @@ -57,7 +57,7 @@ void InsertHelper(BPlusTree, RID, GenericComparator<8>> *tree, con delete transaction; } -// helper function to seperate insert +// helper function to separate insert void InsertHelperSplit(BPlusTree, RID, GenericComparator<8>> *tree, const std::vector &keys, int total_threads, __attribute__((unused)) uint64_t thread_itr) { GenericKey<8> index_key; @@ -88,7 +88,7 @@ void DeleteHelper(BPlusTree, RID, GenericComparator<8>> *tree, con delete transaction; } -// helper function to seperate delete +// helper function to separate delete void DeleteHelperSplit(BPlusTree, RID, GenericComparator<8>> *tree, const std::vector &remove_keys, int total_threads, __attribute__((unused)) uint64_t thread_itr) { @@ -353,24 +353,24 @@ TEST(BPlusTreeConcurrentTest, DISABLED_MixTest2) { BPlusTree, RID, GenericComparator<8>> tree("foo_pk", page_id, bpm, comparator); // Add perserved_keys - std::vector perserved_keys; + std::vector preserved_keys; std::vector dynamic_keys; int64_t total_keys = 50; int64_t sieve = 5; for (int64_t i = 1; i <= total_keys; i++) { if (i % sieve == 0) { - perserved_keys.push_back(i); + preserved_keys.push_back(i); } else { dynamic_keys.push_back(i); } } - InsertHelper(&tree, perserved_keys, 1); + InsertHelper(&tree, preserved_keys, 1); // Check there are 1000 keys in there size_t size; auto insert_task = [&](int tid) { InsertHelper(&tree, dynamic_keys, tid); }; auto delete_task = [&](int tid) { DeleteHelper(&tree, dynamic_keys, tid); }; - auto lookup_task = [&](int tid) { LookupHelper(&tree, perserved_keys, tid); }; + auto lookup_task = [&](int tid) { LookupHelper(&tree, preserved_keys, tid); }; std::vector threads; std::vector> tasks; @@ -396,7 +396,7 @@ TEST(BPlusTreeConcurrentTest, DISABLED_MixTest2) { } } - ASSERT_EQ(size, perserved_keys.size()); + ASSERT_EQ(size, preserved_keys.size()); bpm->UnpinPage(HEADER_PAGE_ID, true); delete bpm; diff --git a/test/storage/page_guard_test.cpp b/test/storage/page_guard_test.cpp index 0c305cc06..e9c7991da 100644 --- a/test/storage/page_guard_test.cpp +++ b/test/storage/page_guard_test.cpp @@ -11,10 +11,12 @@ //===----------------------------------------------------------------------===// #include +#include #include #include #include "buffer/buffer_pool_manager.h" +#include "common/config.h" #include "storage/disk/disk_manager_memory.h" #include "storage/page/page_guard.h" @@ -22,9 +24,243 @@ namespace bustub { +// NOLINTNEXTLINE +TEST(PageGuardTest, DISABLED_GuardUpgradeBasicTest1) { + const size_t buffer_pool_size = 5; + const size_t k = 2; + + auto disk_manager = std::make_shared(); + auto bpm = std::make_shared(buffer_pool_size, disk_manager.get(), k); + + // Create two dummy pages from BPM + page_id_t p_1 = -1; + page_id_t p_2 = -1; + Page *dummy_page_1 = bpm->NewPage(&p_1); + Page *dummy_page_2 = bpm->NewPage(&p_2); + // Now the pin counts should both be 1 + EXPECT_EQ(dummy_page_1->GetPinCount(), 1); + EXPECT_EQ(dummy_page_2->GetPinCount(), 1); + EXPECT_NE(p_1, -1); + EXPECT_NE(p_2, -1); + EXPECT_NE(dummy_page_1, nullptr); + EXPECT_NE(dummy_page_2, nullptr); + + // Unpin the two pages + bpm->UnpinPage(p_1, false); + bpm->UnpinPage(p_2, false); + + // Create two dummy BasicPageGuards + auto b_pg_1 = bpm->FetchPageBasic(p_1); + auto b_pg_2 = bpm->FetchPageBasic(p_2); + + // Upgrade the first PageGuard to ReadPageGuard + ReadPageGuard rpg = b_pg_1.UpgradeRead(); + + // Sanity Check + EXPECT_EQ(dummy_page_1->GetPinCount(), 1); + EXPECT_EQ(b_pg_1.GetPage(), nullptr); + EXPECT_EQ(b_pg_1.GetBPM(), nullptr); + EXPECT_EQ(rpg.GetPage(), dummy_page_1); + EXPECT_EQ(rpg.GetBPM(), bpm.get()); + + // Drop the first ReadPageGuard + rpg.Drop(); + EXPECT_EQ(dummy_page_1->GetPinCount(), 0); + + // Upgrade the second PageGuard to WritePageGuard + WritePageGuard wpg = b_pg_2.UpgradeWrite(); + + // Sanity Check + EXPECT_EQ(dummy_page_2->GetPinCount(), 1); + EXPECT_EQ(b_pg_2.GetPage(), nullptr); + EXPECT_EQ(b_pg_2.GetBPM(), nullptr); + EXPECT_EQ(wpg.GetPage(), dummy_page_2); + EXPECT_EQ(wpg.GetBPM(), bpm.get()); + + // Drop the second WritePageGuard + wpg.Drop(); + EXPECT_EQ(dummy_page_2->GetPinCount(), 0); + + // Shut down the DiskManager + disk_manager->ShutDown(); +} + +// NOLINTNEXTLINE +TEST(PageGuardTest, DISABLED_GuardUpgradeBasicTest2) { + const size_t buffer_pool_size = 5; + const size_t k = 2; + + auto disk_manager = std::make_shared(); + auto bpm = std::make_shared(buffer_pool_size, disk_manager.get(), k); + + // Create one dummy pages + page_id_t p_id = -1; + Page *dummy_page = bpm->NewPage(&p_id); + // The pin count should be 1 + EXPECT_EQ(dummy_page->GetPinCount(), 1); + EXPECT_NE(p_id, -1); + EXPECT_NE(dummy_page, nullptr); + + // Unpin the page + bpm->UnpinPage(p_id, false); + + // Create one dummy BasicPageGuard + auto bpg = bpm->FetchPageBasic(p_id); + + // Upgrade the PageGuard to ReadPageGuard + ReadPageGuard rpg = bpg.UpgradeRead(); + + // Sanity Check + EXPECT_EQ(dummy_page->GetPinCount(), 1); + EXPECT_EQ(bpg.GetPage(), nullptr); + EXPECT_EQ(bpg.GetBPM(), nullptr); + EXPECT_EQ(rpg.GetPage(), dummy_page); + EXPECT_EQ(rpg.GetBPM(), bpm.get()); + + // Upgrade the rpg to WritePageGuard + WritePageGuard wpg = rpg.UpgradeWrite(); + + // Sanity Check + EXPECT_EQ(dummy_page->GetPinCount(), 1); + EXPECT_EQ(rpg.GetPage(), nullptr); + EXPECT_EQ(rpg.GetBPM(), nullptr); + EXPECT_EQ(wpg.GetPage(), dummy_page); + EXPECT_EQ(wpg.GetBPM(), bpm.get()); + + // Drop the WritePageGuard + wpg.Drop(); + EXPECT_EQ(dummy_page->GetPinCount(), 0); + + // Shut down the DiskManager + disk_manager->ShutDown(); +} + +// NOLINTNEXTLINE +TEST(PageGuardTest, DISABLED_GuardUpgradeConcurrentTest1) { + // This test should not block + const size_t buffer_pool_size = 20; + const size_t k = 2; + + auto disk_manager = std::make_shared(); + auto bpm = std::make_shared(buffer_pool_size, disk_manager.get(), k); + + // Create ten dummy pages, construct a page group + std::vector p_g; + std::vector p_vec; + + // Reserve space for p_g, since clang-tidy complains + p_g.reserve(10); + + // Initialize the page group + for (int i = 0; i < 10; ++i) { + page_id_t tmp_pid = -1; + Page *tmp_page = bpm->NewPage(&tmp_pid); + EXPECT_EQ(tmp_page->GetPinCount(), 1); + EXPECT_NE(tmp_pid, -1); + // Unpin the page + bpm->UnpinPage(tmp_pid, false); + p_g.push_back(tmp_pid); + p_vec.push_back(tmp_page); + } + + // Create a bpg group + std::vector bpg_g; + + // Create a rpg group + std::vector> rpg_g; + + // Reserve space for bpg_g & rpg_g, same as above + bpg_g.reserve(10); + rpg_g.reserve(10); + + // Initialize bpg_g + for (int i = 0; i < 10; ++i) { + bpg_g.push_back(bpm->FetchPageBasic(p_g[i])); + EXPECT_EQ(bpg_g.back().GetPage()->GetPinCount(), 1); + } + + // Create the thread vector + std::vector t_g; + + // Reserve space + t_g.reserve(10); + + // Launch ten threads, performing the upgrade + for (int i = 0; i < 10; ++i) { + // Capture i by value here, to avoid potential race condition + t_g.emplace_back([i, &bpg_g, &rpg_g]() { rpg_g.emplace_back(bpg_g[i].UpgradeRead(), i); }); + } + + // Wait til all the upgrades are finished + for (auto &t : t_g) { + t.join(); + } + + // Sanity check + for (auto &[rpg, index] : rpg_g) { + EXPECT_EQ(bpg_g[index].GetPage(), nullptr); + EXPECT_EQ(bpg_g[index].GetBPM(), nullptr); + EXPECT_EQ(rpg.GetPage(), p_vec[index]); + EXPECT_EQ(rpg.GetBPM(), bpm.get()); + // The pin count should be 1 for each page + EXPECT_EQ(rpg.GetPage()->GetPinCount(), 1); + // Drop the ReadPageGuard + rpg.Drop(); + EXPECT_EQ(p_vec[index]->GetPinCount(), 0); + } + + // Shutdown the Disk Manager + disk_manager->ShutDown(); +} + +TEST(PageGuardTest, DISABLED_GuardUpgradeConcurrentTest2) { + const size_t buffer_pool_size = 5; + const size_t k = 2; + + auto disk_manager = std::make_shared(); + auto bpm = std::make_shared(buffer_pool_size, disk_manager.get(), k); + + // Create one dummy page + page_id_t p_id = -1; + Page *dummy_page = bpm->NewPage(&p_id); + + // Unpin the page + bpm->UnpinPage(p_id, false); + + // Fetch a ReadPageGuard + ReadPageGuard rpg_1 = bpm->FetchPageRead(p_id); + + // Fetch a second time, assumes this is from another thread + ReadPageGuard rpg_2 = bpm->FetchPageRead(p_id); + + // Current pin count should be 1 + EXPECT_EQ(dummy_page->GetPinCount(), 2); + + // Update the first ReadPageGuard to WritePageGuard in a background thread + // Which should block at present + std::thread t([&]() { + WritePageGuard wpg = rpg_1.UpgradeWrite(); + EXPECT_EQ(wpg.GetPage(), dummy_page); + EXPECT_EQ(dummy_page->GetPinCount(), 1); + + // Drop the WritePageGuard + wpg.Drop(); + EXPECT_EQ(dummy_page->GetPinCount(), 0); + }); + + // Drop the second ReadPageGuard + rpg_2.Drop(); + EXPECT_EQ(rpg_2.GetPage(), nullptr); + + // This should not block + t.join(); + + // Sanity check + EXPECT_EQ(dummy_page->GetPinCount(), 0); +} + // NOLINTNEXTLINE TEST(PageGuardTest, DISABLED_SampleTest) { - const std::string db_name = "test.db"; const size_t buffer_pool_size = 5; const size_t k = 2; @@ -34,7 +270,7 @@ TEST(PageGuardTest, DISABLED_SampleTest) { page_id_t page_id_temp; auto *page0 = bpm->NewPage(&page_id_temp); - auto guarded_page = BasicPageGuard(bpm.get(), page0); + auto guarded_page = BasicPageGuard{bpm.get(), page0}; EXPECT_EQ(page0->GetData(), guarded_page.GetData()); EXPECT_EQ(page0->GetPageId(), guarded_page.PageId());