Merge branch 'master' into feat-lock-upgrade

src/common/bustub_instance.cpp

@@ -1,3 +1,4 @@
+#include <memory>
 #include <optional>
 #include <shared_mutex>
 #include <string>
@@ -15,7 +16,6 @@
 #include "catalog/schema.h"
 #include "catalog/table_generator.h"
 #include "common/bustub_instance.h"
-#include "common/enums/statement_type.h"
 #include "common/exception.h"
 #include "common/util/string_util.h"
 #include "concurrency/lock_manager.h"
@@ -39,87 +39,90 @@
 namespace bustub {
 
 auto BustubInstance::MakeExecutorContext(Transaction *txn, bool is_modify) -> std::unique_ptr<ExecutorContext> {
- return std::make_unique<ExecutorContext>(txn, catalog_, buffer_pool_manager_, txn_manager_, lock_manager_, is_modify);
+ return std::make_unique<ExecutorContext>(txn, catalog_.get(), buffer_pool_manager_.get(), txn_manager_.get(),
+ lock_manager_.get(), is_modify);
 }
 
 BustubInstance::BustubInstance(const std::string &db_file_name) {
  enable_logging = false;
 
  // Storage related.
- disk_manager_ = new DiskManager(db_file_name);
+ disk_manager_ = std::make_unique<DiskManager>(db_file_name);
 
  // Log related.
- log_manager_ = new LogManager(disk_manager_);
+ log_manager_ = std::make_unique<LogManager>(disk_manager_.get());
 
  // We need more frames for GenerateTestTable to work. Therefore, we use 128 instead of the default
  // buffer pool size specified in `config.h`.
  try {
- buffer_pool_manager_ = new BufferPoolManager(128, disk_manager_, LRUK_REPLACER_K, log_manager_);
+ buffer_pool_manager_ =
+ std::make_unique<BufferPoolManager>(128, disk_manager_.get(), LRUK_REPLACER_K, log_manager_.get());
  } catch (NotImplementedException &e) {
  std::cerr << "BufferPoolManager is not implemented, only mock tables are supported." << std::endl;
  buffer_pool_manager_ = nullptr;
  }
 
  // Transaction (txn) related.
+ lock_manager_ = std::make_unique<LockManager>();
 
- lock_manager_ = new LockManager();
+ txn_manager_ = std::make_unique<TransactionManager>(lock_manager_.get(), log_manager_.get());
 
- txn_manager_ = new TransactionManager(lock_manager_, log_manager_);
-
- lock_manager_->txn_manager_ = txn_manager_;
+ lock_manager_->txn_manager_ = txn_manager_.get();
 
 #ifndef __EMSCRIPTEN__
  lock_manager_->StartDeadlockDetection();
 #endif
 
  // Checkpoint related.
- checkpoint_manager_ = new CheckpointManager(txn_manager_, log_manager_, buffer_pool_manager_);
+ checkpoint_manager_ =
+ std::make_unique<CheckpointManager>(txn_manager_.get(), log_manager_.get(), buffer_pool_manager_.get());
 
- // Catalog.
- catalog_ = new Catalog(buffer_pool_manager_, lock_manager_, log_manager_);
+ // Catalog related.
+ catalog_ = std::make_unique<Catalog>(buffer_pool_manager_.get(), lock_manager_.get(), log_manager_.get());
 
- // Execution engine.
- execution_engine_ = new ExecutionEngine(buffer_pool_manager_, txn_manager_, catalog_);
+ // Execution engine related.
+ execution_engine_ = std::make_unique<ExecutionEngine>(buffer_pool_manager_.get(), txn_manager_.get(), catalog_.get());
 }
 
 BustubInstance::BustubInstance() {
  enable_logging = false;
 
  // Storage related.
- disk_manager_ = new DiskManagerUnlimitedMemory();
+ disk_manager_ = std::make_unique<DiskManagerUnlimitedMemory>();
 
  // Log related.
- log_manager_ = new LogManager(disk_manager_);
+ log_manager_ = std::make_unique<LogManager>(disk_manager_.get());
 
  // We need more frames for GenerateTestTable to work. Therefore, we use 128 instead of the default
  // buffer pool size specified in `config.h`.
  try {
- buffer_pool_manager_ = new BufferPoolManager(128, disk_manager_, LRUK_REPLACER_K, log_manager_);
+ buffer_pool_manager_ =
+ std::make_unique<BufferPoolManager>(128, disk_manager_.get(), LRUK_REPLACER_K, log_manager_.get());
  } catch (NotImplementedException &e) {
  std::cerr << "BufferPoolManager is not implemented, only mock tables are supported." << std::endl;
  buffer_pool_manager_ = nullptr;
  }
 
  // Transaction (txn) related.
+ lock_manager_ = std::make_unique<LockManager>();
 
- lock_manager_ = new LockManager();
-
- txn_manager_ = new TransactionManager(lock_manager_, log_manager_);
+ txn_manager_ = std::make_unique<TransactionManager>(lock_manager_.get(), log_manager_.get());
 
- lock_manager_->txn_manager_ = txn_manager_;
+ lock_manager_->txn_manager_ = txn_manager_.get();
 
 #ifndef __EMSCRIPTEN__
  lock_manager_->StartDeadlockDetection();
 #endif
 
  // Checkpoint related.
- checkpoint_manager_ = new CheckpointManager(txn_manager_, log_manager_, buffer_pool_manager_);
+ checkpoint_manager_ =
+ std::make_unique<CheckpointManager>(txn_manager_.get(), log_manager_.get(), buffer_pool_manager_.get());
 
- // Catalog.
- catalog_ = new Catalog(buffer_pool_manager_, lock_manager_, log_manager_);
+ // Catalog related.
+ catalog_ = std::make_unique<Catalog>(buffer_pool_manager_.get(), lock_manager_.get(), log_manager_.get());
 
- // Execution engine.
- execution_engine_ = new ExecutionEngine(buffer_pool_manager_, txn_manager_, catalog_);
+ // Execution engine related.
+ execution_engine_ = std::make_unique<ExecutionEngine>(buffer_pool_manager_.get(), txn_manager_.get(), catalog_.get());
 }
 
 void BustubInstance::CmdDisplayTables(ResultWriter &writer) {
@@ -354,14 +357,6 @@ BustubInstance::~BustubInstance() {
  if (enable_logging) {
  log_manager_->StopFlushThread();
  }
- delete execution_engine_;
- delete catalog_;
- delete checkpoint_manager_;
- delete log_manager_;
- delete buffer_pool_manager_;
- delete lock_manager_;
- delete txn_manager_;
- delete disk_manager_;
 }
 
 } // namespace bustub

src/concurrency/transaction_manager.cpp

@@ -20,6 +20,7 @@
 #include "catalog/catalog.h"
 #include "common/macros.h"
 #include "storage/table/table_heap.h"
+
 namespace bustub {
 
 void TransactionManager::Commit(Transaction *txn) {

src/include/common/bustub_instance.h

@@ -250,17 +250,18 @@ class BustubInstance {
  */
  void GenerateMockTable();
 
- // TODO(chi): change to unique_ptr. Currently they're directly referenced by recovery test, so
+ // Currently the followings are directly referenced by recovery test, so
  // we cannot do anything on them until someone decides to refactor the recovery test.
 
- DiskManager *disk_manager_;
- BufferPoolManager *buffer_pool_manager_;
- LockManager *lock_manager_;
- TransactionManager *txn_manager_;
- LogManager *log_manager_;
- CheckpointManager *checkpoint_manager_;
- Catalog *catalog_;
- ExecutionEngine *execution_engine_;
+ std::unique_ptr<DiskManager> disk_manager_;
+ std::unique_ptr<BufferPoolManager> buffer_pool_manager_;
+ std::unique_ptr<LockManager> lock_manager_;
+ std::unique_ptr<TransactionManager> txn_manager_;
+ std::unique_ptr<LogManager> log_manager_;
+ std::unique_ptr<CheckpointManager> checkpoint_manager_;
+ std::unique_ptr<Catalog> catalog_;
+ std::unique_ptr<ExecutionEngine> execution_engine_;
+ /** Coordination for catalog */
  std::shared_mutex catalog_lock_;
 
  auto GetSessionVariable(const std::string &key) -> std::string {

src/include/concurrency/transaction.h

@@ -30,19 +30,26 @@ namespace bustub {
 
 /**
  * Transaction states for 2PL:
+ * Running transactions could be aborted during either `GROWING` or `SHRINKING` stage.
  *
  * _________________________
+ * | |
  * | v
  * GROWING -> SHRINKING -> COMMITTED ABORTED
- * |__________|________________________^
+ * | | ^
+ * |___________|________________________|
  *
  * Transaction states for Non-2PL:
+ * Running transactions could only be aborted during `GROWING` stage, since there is no `SHRINKING` stage.
+ *
  * __________
+ * | |
  * | v
  * GROWING -> COMMITTED ABORTED
- * |_________________________^
+ * | ^
+ * |_________________________|
  *
- **/
+ */
 enum class TransactionState { GROWING, SHRINKING, COMMITTED, ABORTED };
 
 /**
@@ -126,14 +133,17 @@ enum class AbortReason {
  * TransactionAbortException is thrown when state of a transaction is changed to ABORTED
  */
 class TransactionAbortException : public std::exception {
- txn_id_t txn_id_;
- AbortReason abort_reason_;
-
  public:
  explicit TransactionAbortException(txn_id_t txn_id, AbortReason abort_reason)
  : txn_id_(txn_id), abort_reason_(abort_reason) {}
+
+ /** @return this transaction id */
  auto GetTransactionId() -> txn_id_t { return txn_id_; }
+
+ /** @return the abort reason for this transaction */
  auto GetAbortReason() -> AbortReason { return abort_reason_; }
+
+ /** @return the detailed information of abort reason */
  auto GetInfo() -> std::string {
  switch (abort_reason_) {
  case AbortReason::LOCK_ON_SHRINKING:
@@ -155,10 +165,17 @@ class TransactionAbortException : public std::exception {
  return "Transaction " + std::to_string(txn_id_) + " aborted because attempted lock upgrade is incompatible\n";
  case AbortReason::ATTEMPTED_UNLOCK_BUT_NO_LOCK_HELD:
  return "Transaction " + std::to_string(txn_id_) + " aborted because attempted to unlock but no lock held \n";
+ default:
+ // Unknown AbortReason
+ throw bustub::Exception("Unknown abort reason for transaction " + std::to_string(txn_id_));
  }
- // Todo: Should fail with unreachable.
- return "";
+ // This is impossible
+ assert(false);
  }
+
+ private:
+ txn_id_t txn_id_;
+ AbortReason abort_reason_;
 };
 
 /**
@@ -248,22 +265,30 @@ class Transaction {
  return x_row_lock_set_;
  }
 
- /** @return the set of resources under a shared lock */
+ /** @return the set of table resources under a shared lock */
  inline auto GetSharedTableLockSet() -> std::shared_ptr<std::unordered_set<table_oid_t>> { return s_table_lock_set_; }
+
+ /** @return the set of table resources under a exclusive lock */
  inline auto GetExclusiveTableLockSet() -> std::shared_ptr<std::unordered_set<table_oid_t>> {
  return x_table_lock_set_;
  }
+
+ /** @return the set of table resources under a intention shared lock */
  inline auto GetIntentionSharedTableLockSet() -> std::shared_ptr<std::unordered_set<table_oid_t>> {
  return is_table_lock_set_;
  }
+
+ /** @return the set of table resources under a intention exclusive lock */
  inline auto GetIntentionExclusiveTableLockSet() -> std::shared_ptr<std::unordered_set<table_oid_t>> {
  return ix_table_lock_set_;
  }
+
+ /** @return the set of table resources under a shared intention exclusive lock */
  inline auto GetSharedIntentionExclusiveTableLockSet() -> std::shared_ptr<std::unordered_set<table_oid_t>> {
  return six_table_lock_set_;
  }
 
- /** @return true if rid (belong to table oid) is shared locked by this transaction */
+ /** @return true if the specified row (belong to table oid) is shared locked by this transaction */
  auto IsRowSharedLocked(const table_oid_t &oid, const RID &rid) -> bool {
  auto row_lock_set = s_row_lock_set_->find(oid);
  if (row_lock_set == s_row_lock_set_->end()) {
@@ -272,7 +297,7 @@ class Transaction {
  return row_lock_set->second.find(rid) != row_lock_set->second.end();
  }
 
- /** @return true if rid (belong to table oid) is exclusive locked by this transaction */
+ /** @return true if the specified row (belong to table oid) is exclusive locked by this transaction */
  auto IsRowExclusiveLocked(const table_oid_t &oid, const RID &rid) -> bool {
  auto row_lock_set = x_row_lock_set_->find(oid);
  if (row_lock_set == x_row_lock_set_->end()) {
@@ -281,22 +306,27 @@ class Transaction {
  return row_lock_set->second.find(rid) != row_lock_set->second.end();
  }
 
+ /** @return true if the table (specified by oid) is intention shared locked by this transaction */
  auto IsTableIntentionSharedLocked(const table_oid_t &oid) -> bool {
  return is_table_lock_set_->find(oid) != is_table_lock_set_->end();
  }
 
+ /** @return true if the table (specified by oid) is shared locked by this transaction */
  auto IsTableSharedLocked(const table_oid_t &oid) -> bool {
  return s_table_lock_set_->find(oid) != s_table_lock_set_->end();
  }
 
+ /** @return true if the table (specified by oid) is intention exclusive locked by this transaction */
  auto IsTableIntentionExclusiveLocked(const table_oid_t &oid) -> bool {
  return ix_table_lock_set_->find(oid) != ix_table_lock_set_->end();
  }
 
+ /** @return true if the table (specified by oid) is exclusive locked by this transaction */
  auto IsTableExclusiveLocked(const table_oid_t &oid) -> bool {
  return x_table_lock_set_->find(oid) != x_table_lock_set_->end();
  }
 
+ /** @return true if the table (specified by oid) is shared intention exclusive locked by this transaction */
  auto IsTableSharedIntentionExclusiveLocked(const table_oid_t &oid) -> bool {
  return six_table_lock_set_->find(oid) != six_table_lock_set_->end();
  }
@@ -340,6 +370,7 @@ class Transaction {
  /** The LSN of the last record written by the transaction. */
  lsn_t prev_lsn_;
 
+ /** The latch for this transaction */
  std::mutex latch_;
 
  /** Concurrent index: the pages that were latched during index operation. */

src/include/concurrency/transaction_manager.h

@@ -71,11 +71,10 @@ class TransactionManager {
  void Abort(Transaction *txn);
 
  /**
- * Global list of running transactions
+ * The transaction map is a global list of all the running transactions in the system.
  */
-
- /** The transaction map is a global list of all the running transactions in the system. */
  std::unordered_map<txn_id_t, Transaction *> txn_map_;
+ /** Coordination for the transaction map */
  std::shared_mutex txn_map_mutex_;
 
  /**