p1 (bpm) flush api changes (#800)

* p1 flush changes

* Note why disk scheduler is shared ptr

Author: rmboyce

src/buffer/buffer_pool_manager.cpp

@@ -72,7 +72,7 @@ BufferPoolManager::BufferPoolManager(size_t num_frames, DiskManager *disk_manage
       next_page_id_(0),
       bpm_latch_(std::make_shared<std::mutex>()),
       replacer_(std::make_shared<LRUKReplacer>(num_frames, k_dist)),
-      disk_scheduler_(std::make_unique<DiskScheduler>(disk_manager)),
+      disk_scheduler_(std::make_shared<DiskScheduler>(disk_manager)),
       log_manager_(log_manager) {
   // Not strictly necessary...
   std::scoped_lock latch(*bpm_latch_);
@@ -274,11 +274,14 @@ auto BufferPoolManager::ReadPage(page_id_t page_id, AccessType access_type) -> R
 }
 
 /**
- * @brief Flushes a page's data out to disk.
+ * @brief Flushes a page's data out to disk unsafely.
  *
  * This function will write out a page's data to disk if it has been modified. If the given page is not in memory, this
  * function will return `false`.
  *
+ * You should not take a lock on the page in this function.
+ * This means that you should carefully consider when to toggle the `is_dirty_` bit.
+ *
  * ### Implementation
  *
  * You should probably leave implementing this function until after you have completed `CheckedReadPage` and
@@ -289,10 +292,47 @@ auto BufferPoolManager::ReadPage(page_id_t page_id, AccessType access_type) -> R
  * @param page_id The page ID of the page to be flushed.
  * @return `false` if the page could not be found in the page table, otherwise `true`.
  */
+auto BufferPoolManager::FlushPageUnsafe(page_id_t page_id) -> bool { UNIMPLEMENTED("TODO(P1): Add implementation."); }
+
+/**
+ * @brief Flushes a page's data out to disk safely.
+ *
+ * This function will write out a page's data to disk if it has been modified. If the given page is not in memory, this
+ * function will return `false`.
+ *
+ * You should take a lock on the page in this function to ensure that a consistent state is flushed to disk.
+ *
+ * ### Implementation
+ *
+ * You should probably leave implementing this function until after you have completed `CheckedReadPage`,
+ * `CheckedWritePage`, and `Flush` in the page guards, as it will likely be much easier to understand what to do.
+ *
+ * TODO(P1): Add implementation
+ *
+ * @param page_id The page ID of the page to be flushed.
+ * @return `false` if the page could not be found in the page table, otherwise `true`.
+ */
 auto BufferPoolManager::FlushPage(page_id_t page_id) -> bool { UNIMPLEMENTED("TODO(P1): Add implementation."); }
 
 /**
- * @brief Flushes all page data that is in memory to disk.
+ * @brief Flushes all page data that is in memory to disk unsafely.
+ *
+ * You should not take locks on the pages in this function.
+ * This means that you should carefully consider when to toggle the `is_dirty_` bit.
+ *
+ * ### Implementation
+ *
+ * You should probably leave implementing this function until after you have completed `CheckedReadPage`,
+ * `CheckedWritePage`, and `FlushPage`, as it will likely be much easier to understand what to do.
+ *
+ * TODO(P1): Add implementation
+ */
+void BufferPoolManager::FlushAllPagesUnsafe() { UNIMPLEMENTED("TODO(P1): Add implementation."); }
+
+/**
+ * @brief Flushes all page data that is in memory to disk safely.
+ *
+ * You should take locks on the pages in this function to ensure that a consistent state is flushed to disk.
  *
  * ### Implementation
  *

src/include/buffer/buffer_pool_manager.h

@@ -121,7 +121,9 @@ class BufferPoolManager {
   auto CheckedReadPage(page_id_t page_id, AccessType access_type = AccessType::Unknown) -> std::optional<ReadPageGuard>;
   auto WritePage(page_id_t page_id, AccessType access_type = AccessType::Unknown) -> WritePageGuard;
   auto ReadPage(page_id_t page_id, AccessType access_type = AccessType::Unknown) -> ReadPageGuard;
+  auto FlushPageUnsafe(page_id_t page_id) -> bool;
   auto FlushPage(page_id_t page_id) -> bool;
+  void FlushAllPagesUnsafe();
   void FlushAllPages();
   auto GetPinCount(page_id_t page_id) -> std::optional<size_t>;
 
@@ -151,8 +153,8 @@ class BufferPoolManager {
   /** @brief The replacer to find unpinned / candidate pages for eviction. */
   std::shared_ptr<LRUKReplacer> replacer_;
 
-  /** @brief A pointer to the disk scheduler. */
-  std::unique_ptr<DiskScheduler> disk_scheduler_;
+  /** @brief A pointer to the disk scheduler. Shared with the page guards for flushing. */
+  std::shared_ptr<DiskScheduler> disk_scheduler_;
 
   /**
    * @brief A pointer to the log manager.

src/include/storage/page/page_guard.h

@@ -15,6 +15,7 @@
 #include <memory>
 
 #include "buffer/buffer_pool_manager.h"
+#include "storage/disk/disk_scheduler.h"
 #include "storage/page/page.h"
 
 namespace bustub {
@@ -60,13 +61,14 @@ class ReadPageGuard {
     return reinterpret_cast<const T *>(GetData());
   }
   auto IsDirty() const -> bool;
+  void Flush();
   void Drop();
   ~ReadPageGuard();
 
  private:
   /** @brief Only the buffer pool manager is allowed to construct a valid `ReadPageGuard.` */
   explicit ReadPageGuard(page_id_t page_id, std::shared_ptr<FrameHeader> frame, std::shared_ptr<LRUKReplacer> replacer,
-                         std::shared_ptr<std::mutex> bpm_latch);
+                         std::shared_ptr<std::mutex> bpm_latch, std::shared_ptr<DiskScheduler> disk_scheduler);
 
   /** @brief The page ID of the page we are guarding. */
   page_id_t page_id_;
@@ -94,6 +96,13 @@ class ReadPageGuard {
    */
   std::shared_ptr<std::mutex> bpm_latch_;
 
+  /**
+   * @brief A shared pointer to the buffer pool's disk scheduler.
+   *
+   * Used when flushing pages to disk.
+   */
+  std::shared_ptr<DiskScheduler> disk_scheduler_;
+
   /**
    * @brief The validity flag for this `ReadPageGuard`.
    *
@@ -159,13 +168,14 @@ class WritePageGuard {
     return reinterpret_cast<T *>(GetDataMut());
   }
   auto IsDirty() const -> bool;
+  void Flush();
   void Drop();
   ~WritePageGuard();
 
  private:
   /** @brief Only the buffer pool manager is allowed to construct a valid `WritePageGuard.` */
   explicit WritePageGuard(page_id_t page_id, std::shared_ptr<FrameHeader> frame, std::shared_ptr<LRUKReplacer> replacer,
-                          std::shared_ptr<std::mutex> bpm_latch);
+                          std::shared_ptr<std::mutex> bpm_latch, std::shared_ptr<DiskScheduler> disk_scheduler);
 
   /** @brief The page ID of the page we are guarding. */
   page_id_t page_id_;
@@ -193,6 +203,13 @@ class WritePageGuard {
    */
   std::shared_ptr<std::mutex> bpm_latch_;
 
+  /**
+   * @brief A shared pointer to the buffer pool's disk scheduler.
+   *
+   * Used when flushing pages to disk.
+   */
+  std::shared_ptr<DiskScheduler> disk_scheduler_;
+
   /**
    * @brief The validity flag for this `WritePageGuard`.
    *

src/storage/page/page_guard.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "storage/page/page_guard.h"
+#include <memory>
 
 namespace bustub {
 
@@ -25,10 +26,16 @@ namespace bustub {
  * @param frame A shared pointer to the frame that holds the page we want to protect.
  * @param replacer A shared pointer to the buffer pool manager's replacer.
  * @param bpm_latch A shared pointer to the buffer pool manager's latch.
+ * @param disk_scheduler A shared pointer to the buffer pool manager's disk scheduler.
  */
 ReadPageGuard::ReadPageGuard(page_id_t page_id, std::shared_ptr<FrameHeader> frame,
-                             std::shared_ptr<LRUKReplacer> replacer, std::shared_ptr<std::mutex> bpm_latch)
-    : page_id_(page_id), frame_(std::move(frame)), replacer_(std::move(replacer)), bpm_latch_(std::move(bpm_latch)) {
+                             std::shared_ptr<LRUKReplacer> replacer, std::shared_ptr<std::mutex> bpm_latch,
+                             std::shared_ptr<DiskScheduler> disk_scheduler)
+    : page_id_(page_id),
+      frame_(std::move(frame)),
+      replacer_(std::move(replacer)),
+      bpm_latch_(std::move(bpm_latch)),
+      disk_scheduler_(std::move(disk_scheduler)) {
   UNIMPLEMENTED("TODO(P1): Add implementation.");
 }
 
@@ -92,6 +99,13 @@ auto ReadPageGuard::IsDirty() const -> bool {
   return frame_->is_dirty_;
 }
 
+/**
+ * @brief Flushes this page's data safely to disk.
+ *
+ * TODO(P1): Add implementation.
+ */
+void ReadPageGuard::Flush() { UNIMPLEMENTED("TODO(P1): Add implementation."); }
+
 /**
  * @brief Manually drops a valid `ReadPageGuard`'s data. If this guard is invalid, this function does nothing.
  *
@@ -123,10 +137,16 @@ ReadPageGuard::~ReadPageGuard() { Drop(); }
  * @param frame A shared pointer to the frame that holds the page we want to protect.
  * @param replacer A shared pointer to the buffer pool manager's replacer.
  * @param bpm_latch A shared pointer to the buffer pool manager's latch.
+ * @param disk_scheduler A shared pointer to the buffer pool manager's disk scheduler.
  */
 WritePageGuard::WritePageGuard(page_id_t page_id, std::shared_ptr<FrameHeader> frame,
-                               std::shared_ptr<LRUKReplacer> replacer, std::shared_ptr<std::mutex> bpm_latch)
-    : page_id_(page_id), frame_(std::move(frame)), replacer_(std::move(replacer)), bpm_latch_(std::move(bpm_latch)) {
+                               std::shared_ptr<LRUKReplacer> replacer, std::shared_ptr<std::mutex> bpm_latch,
+                               std::shared_ptr<DiskScheduler> disk_scheduler)
+    : page_id_(page_id),
+      frame_(std::move(frame)),
+      replacer_(std::move(replacer)),
+      bpm_latch_(std::move(bpm_latch)),
+      disk_scheduler_(std::move(disk_scheduler)) {
   UNIMPLEMENTED("TODO(P1): Add implementation.");
 }
 
@@ -198,6 +218,13 @@ auto WritePageGuard::IsDirty() const -> bool {
   return frame_->is_dirty_;
 }
 
+/**
+ * @brief Flushes this page's data safely to disk.
+ *
+ * TODO(P1): Add implementation.
+ */
+void WritePageGuard::Flush() { UNIMPLEMENTED("TODO(P1): Add implementation."); }
+
 /**
  * @brief Manually drops a valid `WritePageGuard`'s data. If this guard is invalid, this function does nothing.
  *