Skip to content

Commit dadb42b

Browse files
skyzhskyzh
authored
refactor(p4): improve test cases (#557)
* fix(p4): better test cases Signed-off-by: Alex Chi <[email protected]> * fix Signed-off-by: Alex Chi <[email protected]> * update tests Signed-off-by: Alex Chi <[email protected]> * fix lock manager fmt Signed-off-by: Alex Chi <[email protected]> * improve! Signed-off-by: Alex Chi <[email protected]> * clean up txn mgr Signed-off-by: Alex Chi <[email protected]> * fix Signed-off-by: Alex Chi <[email protected]> --------- Signed-off-by: Alex Chi <[email protected]>
1 parent 230b4ec commit dadb42b

File tree

10 files changed

+304
-258
lines changed

10 files changed

+304
-258
lines changed

‎src/common/bustub_instance.cpp‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,16 @@ BustubInstance::BustubInstance(const std::string &db_file_name) {
6262

6363
// Transaction (txn) related.
6464

65-
#ifdef __EMSCRIPTEN__
66-
lock_manager_ = new LockManager(false);
67-
#else
6865
lock_manager_ = new LockManager();
69-
#endif
7066

7167
txn_manager_ = new TransactionManager(lock_manager_, log_manager_);
7268

7369
lock_manager_->txn_manager_ = txn_manager_;
7470

71+
#ifndef __EMSCRIPTEN__
72+
lock_manager_->StartDeadlockDetection();
73+
#endif
74+
7575
// Checkpoint related.
7676
checkpoint_manager_ = new CheckpointManager(txn_manager_, log_manager_, buffer_pool_manager_);
7777

@@ -102,16 +102,16 @@ BustubInstance::BustubInstance() {
102102

103103
// Transaction (txn) related.
104104

105-
#ifdef __EMSCRIPTEN__
106-
lock_manager_ = new LockManager(false);
107-
#else
108105
lock_manager_ = new LockManager();
109-
#endif
110106

111107
txn_manager_ = new TransactionManager(lock_manager_, log_manager_);
112108

113109
lock_manager_->txn_manager_ = txn_manager_;
114110

111+
#ifndef __EMSCRIPTEN__
112+
lock_manager_->StartDeadlockDetection();
113+
#endif
114+
115115
// Checkpoint related.
116116
checkpoint_manager_ = new CheckpointManager(txn_manager_, log_manager_, buffer_pool_manager_);
117117

@@ -230,10 +230,11 @@ auto BustubInstance::ExecuteSqlTxn(const std::string &sql, ResultWriter &writer,
230230
binder.ParseAndSave(sql);
231231
l.unlock();
232232

233-
bool is_delete = false;
234-
235233
for (auto *stmt : binder.statement_nodes_) {
236234
auto statement = binder.BindStatement(stmt);
235+
236+
bool is_delete = false;
237+
237238
switch (statement->type_) {
238239
case StatementType::CREATE_STATEMENT: {
239240
const auto &create_stmt = dynamic_cast<const CreateStatement &>(*statement);
@@ -261,6 +262,7 @@ auto BustubInstance::ExecuteSqlTxn(const std::string &sql, ResultWriter &writer,
261262
continue;
262263
}
263264
case StatementType::DELETE_STATEMENT:
265+
case StatementType::UPDATE_STATEMENT:
264266
is_delete = true;
265267
default:
266268
break;

‎src/concurrency/lock_manager.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ auto LockManager::UnlockRow(Transaction *txn, const table_oid_t &oid, const RID
3030
return true;
3131
}
3232

33+
void LockManager::UnlockAll() {
34+
// You probably want to unlock all table and txn locks here.
35+
}
36+
3337
void LockManager::AddEdge(txn_id_t t1, txn_id_t t2) {}
3438

3539
void LockManager::RemoveEdge(txn_id_t t1, txn_id_t t2) {}

‎src/concurrency/transaction_manager.cpp‎

Lines changed: 7 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -22,102 +22,23 @@
2222
#include "storage/table/table_heap.h"
2323
namespace bustub {
2424

25-
std::unordered_map<txn_id_t, Transaction *> TransactionManager::txn_map = {};
26-
std::shared_mutex TransactionManager::txn_map_mutex = {};
27-
28-
auto TransactionManager::Begin(Transaction *txn, IsolationLevel isolation_level) -> Transaction * {
29-
// Acquire the global transaction latch in shared mode.
30-
global_txn_latch_.RLock();
31-
32-
if (txn == nullptr) {
33-
txn = new Transaction(next_txn_id_++, isolation_level);
34-
}
35-
36-
if (enable_logging) {
37-
LogRecord record = LogRecord(txn->GetTransactionId(), txn->GetPrevLSN(), LogRecordType::BEGIN);
38-
lsn_t lsn = log_manager_->AppendLogRecord(&record);
39-
txn->SetPrevLSN(lsn);
40-
}
41-
42-
std::unique_lock<std::shared_mutex> l(txn_map_mutex);
43-
txn_map[txn->GetTransactionId()] = txn;
44-
return txn;
45-
}
46-
4725
void TransactionManager::Commit(Transaction *txn) {
48-
txn->SetState(TransactionState::COMMITTED);
49-
50-
// Perform all deletes before we commit.
51-
auto write_set = txn->GetWriteSet();
52-
while (!write_set->empty()) {
53-
// auto &item = write_set->back();
54-
// auto *table = item.table_;
55-
// if (item.wtype_ == WType::DELETE) {
56-
// // Note that this also releases the lock when holding the page latch.
57-
// table->ApplyDelete(item.rid_, txn);
58-
// }
59-
write_set->pop_back();
60-
}
61-
write_set->clear();
62-
6326
// Release all the locks.
6427
ReleaseLocks(txn);
65-
// Release the global transaction latch.
66-
global_txn_latch_.RUnlock();
28+
29+
txn->SetState(TransactionState::COMMITTED);
6730
}
6831

6932
void TransactionManager::Abort(Transaction *txn) {
70-
txn->SetState(TransactionState::ABORTED);
71-
// Rollback before releasing the lock.
72-
auto table_write_set = txn->GetWriteSet();
73-
while (!table_write_set->empty()) {
74-
// auto &item = table_write_set->back();
75-
// auto *table = item.table_;
76-
// if (item.wtype_ == WType::DELETE) {
77-
// table->RollbackDelete(item.rid_, txn);
78-
// } else if (item.wtype_ == WType::INSERT) {
79-
// // Note that this also releases the lock when holding the page latch.
80-
// table->ApplyDelete(item.rid_, txn);
81-
// } else if (item.wtype_ == WType::UPDATE) {
82-
// table->UpdateTuple(item.tuple_, item.rid_, txn);
83-
// }
84-
table_write_set->pop_back();
85-
}
86-
table_write_set->clear();
87-
// Rollback index updates
88-
auto index_write_set = txn->GetIndexWriteSet();
89-
while (!index_write_set->empty()) {
90-
auto &item = index_write_set->back();
91-
auto *catalog = item.catalog_;
92-
// Metadata identifying the table that should be deleted from.
93-
TableInfo *table_info = catalog->GetTable(item.table_oid_);
94-
IndexInfo *index_info = catalog->GetIndex(item.index_oid_);
95-
auto new_key = item.tuple_.KeyFromTuple(table_info->schema_, *(index_info->index_->GetKeySchema()),
96-
index_info->index_->GetKeyAttrs());
97-
if (item.wtype_ == WType::DELETE) {
98-
index_info->index_->InsertEntry(new_key, item.rid_, txn);
99-
} else if (item.wtype_ == WType::INSERT) {
100-
index_info->index_->DeleteEntry(new_key, item.rid_, txn);
101-
} else if (item.wtype_ == WType::UPDATE) {
102-
// Delete the new key and insert the old key
103-
index_info->index_->DeleteEntry(new_key, item.rid_, txn);
104-
auto old_key = item.old_tuple_.KeyFromTuple(table_info->schema_, *(index_info->index_->GetKeySchema()),
105-
index_info->index_->GetKeyAttrs());
106-
index_info->index_->InsertEntry(old_key, item.rid_, txn);
107-
}
108-
index_write_set->pop_back();
109-
}
110-
table_write_set->clear();
111-
index_write_set->clear();
33+
/* TODO: revert all the changes in write set */
11234

113-
// Release all the locks.
11435
ReleaseLocks(txn);
115-
// Release the global transaction latch.
116-
global_txn_latch_.RUnlock();
36+
37+
txn->SetState(TransactionState::ABORTED);
11738
}
11839

119-
void TransactionManager::BlockAllTransactions() { global_txn_latch_.WLock(); }
40+
void TransactionManager::BlockAllTransactions() { UNIMPLEMENTED("block is not supported now!"); }
12041

121-
void TransactionManager::ResumeTransactions() { global_txn_latch_.WUnlock(); }
42+
void TransactionManager::ResumeTransactions() { UNIMPLEMENTED("resume is not supported now!"); }
12243

12344
} // namespace bustub

‎src/include/concurrency/lock_manager.h‎

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <vector>
2424

2525
#include "common/config.h"
26+
#include "common/macros.h"
2627
#include "common/rid.h"
2728
#include "concurrency/transaction.h"
2829

@@ -76,15 +77,23 @@ class LockManager {
7677
/**
7778
* Creates a new lock manager configured for the deadlock detection policy.
7879
*/
79-
LockManager() {
80+
LockManager() = default;
81+
82+
void StartDeadlockDetection() {
83+
BUSTUB_ENSURE(txn_manager_ != nullptr, "txn_manager_ is not set.")
8084
enable_cycle_detection_ = true;
8185
cycle_detection_thread_ = new std::thread(&LockManager::RunCycleDetection, this);
8286
}
8387

8488
~LockManager() {
89+
UnlockAll();
90+
8591
enable_cycle_detection_ = false;
86-
cycle_detection_thread_->join();
87-
delete cycle_detection_thread_;
92+
93+
if (cycle_detection_thread_ != nullptr) {
94+
cycle_detection_thread_->join();
95+
delete cycle_detection_thread_;
96+
}
8897
}
8998

9099
/**
@@ -301,7 +310,19 @@ class LockManager {
301310
TransactionManager *txn_manager_;
302311

303312
private:
304-
/** Fall 2022 */
313+
/** Spring 2023 */
314+
/* You are allowed to modify all functions below. */
315+
auto UpgradeLockTable(Transaction *txn, LockMode lock_mode, const table_oid_t &oid) -> bool;
316+
auto UpgradeLockRow(Transaction *txn, LockMode lock_mode, const table_oid_t &oid, const RID &rid) -> bool;
317+
auto AreLocksCompatible(LockMode l1, LockMode l2) -> bool;
318+
auto CanTxnTakeLock(Transaction *txn, LockMode lock_mode) -> bool;
319+
void GrantNewLocksIfPossible(LockRequestQueue *lock_request_queue);
320+
auto CanLockUpgrade(LockMode curr_lock_mode, LockMode requested_lock_mode) -> bool;
321+
auto CheckAppropriateLockOnTable(Transaction *txn, const table_oid_t &oid, LockMode row_lock_mode) -> bool;
322+
auto FindCycle(txn_id_t source_txn, std::vector<txn_id_t> &path, std::unordered_set<txn_id_t> &on_path,
323+
std::unordered_set<txn_id_t> &visited, txn_id_t *abort_txn_id) -> bool;
324+
void UnlockAll();
325+
305326
/** Structure that holds lock requests for a given table oid */
306327
std::unordered_map<table_oid_t, std::shared_ptr<LockRequestQueue>> table_lock_map_;
307328
/** Coordination */
@@ -320,3 +341,30 @@ class LockManager {
320341
};
321342

322343
} // namespace bustub
344+
345+
template <>
346+
struct fmt::formatter<bustub::LockManager::LockMode> : formatter<std::string_view> {
347+
// parse is inherited from formatter<string_view>.
348+
template <typename FormatContext>
349+
auto format(bustub::LockManager::LockMode x, FormatContext &ctx) const {
350+
string_view name = "unknown";
351+
switch (x) {
352+
case bustub::LockManager::LockMode::EXCLUSIVE:
353+
name = "EXCLUSIVE";
354+
break;
355+
case bustub::LockManager::LockMode::INTENTION_EXCLUSIVE:
356+
name = "INTENTION_EXCLUSIVE";
357+
break;
358+
case bustub::LockManager::LockMode::SHARED:
359+
name = "SHARED";
360+
break;
361+
case bustub::LockManager::LockMode::INTENTION_SHARED:
362+
name = "INTENTION_SHARED";
363+
break;
364+
case bustub::LockManager::LockMode::SHARED_INTENTION_EXCLUSIVE:
365+
name = "SHARED_INTENTION_EXCLUSIVE";
366+
break;
367+
}
368+
return formatter<string_view>::format(name, ctx);
369+
}
370+
};

‎src/include/concurrency/transaction_manager.h‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,21 @@ class TransactionManager {
4242
* @return an initialized transaction
4343
*/
4444
auto Begin(Transaction *txn = nullptr, IsolationLevel isolation_level = IsolationLevel::REPEATABLE_READ)
45-
-> Transaction *;
45+
-> Transaction * {
46+
if (txn == nullptr) {
47+
txn = new Transaction(next_txn_id_++, isolation_level);
48+
}
49+
50+
if (enable_logging) {
51+
LogRecord record = LogRecord(txn->GetTransactionId(), txn->GetPrevLSN(), LogRecordType::BEGIN);
52+
lsn_t lsn = log_manager_->AppendLogRecord(&record);
53+
txn->SetPrevLSN(lsn);
54+
}
55+
56+
std::unique_lock<std::shared_mutex> l(txn_map_mutex_);
57+
txn_map_[txn->GetTransactionId()] = txn;
58+
return txn;
59+
}
4660

4761
/**
4862
* Commits a transaction.
@@ -61,18 +75,18 @@ class TransactionManager {
6175
*/
6276

6377
/** The transaction map is a global list of all the running transactions in the system. */
64-
static std::unordered_map<txn_id_t, Transaction *> txn_map;
65-
static std::shared_mutex txn_map_mutex;
78+
std::unordered_map<txn_id_t, Transaction *> txn_map_;
79+
std::shared_mutex txn_map_mutex_;
6680

6781
/**
6882
* Locates and returns the transaction with the given transaction ID.
6983
* @param txn_id the id of the transaction to be found, it must exist!
7084
* @return the transaction with the given transaction id
7185
*/
72-
static auto GetTransaction(txn_id_t txn_id) -> Transaction * {
73-
std::shared_lock<std::shared_mutex> l(TransactionManager::txn_map_mutex);
74-
assert(TransactionManager::txn_map.find(txn_id) != TransactionManager::txn_map.end());
75-
auto *res = TransactionManager::txn_map[txn_id];
86+
auto GetTransaction(txn_id_t txn_id) -> Transaction * {
87+
std::shared_lock<std::shared_mutex> l(txn_map_mutex_);
88+
assert(txn_map_.find(txn_id) != txn_map_.end());
89+
auto *res = txn_map_[txn_id];
7690
assert(res != nullptr);
7791
return res;
7892
}
@@ -137,9 +151,6 @@ class TransactionManager {
137151
std::atomic<txn_id_t> next_txn_id_{0};
138152
LockManager *lock_manager_ __attribute__((__unused__));
139153
LogManager *log_manager_ __attribute__((__unused__));
140-
141-
/** The global transaction latch is used for checkpointing. */
142-
ReaderWriterLatch global_txn_latch_;
143154
};
144155

145156
} // namespace bustub

‎src/storage/table/table_heap.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,16 @@ auto TableHeap::InsertTuple(const TupleMeta &meta, const Tuple &tuple, LockManag
5353
auto npg = bpm_->NewPage(&next_page_id);
5454
BUSTUB_ENSURE(next_page_id != INVALID_PAGE_ID, "cannot allocate page");
5555

56-
// Don't do lock crabbing here: TSAN reports, also as last_page_id_ is only updated
57-
// later, this page won't be accessed.
5856
page->SetNextPageId(next_page_id);
57+
58+
auto next_page = reinterpret_cast<TablePage *>(npg->GetData());
59+
next_page->Init();
60+
5961
page_guard.Drop();
6062

63+
// acquire latch here as TSAN complains. Given we only have one insertion thread, this is fine.
6164
npg->WLatch();
6265
auto next_page_guard = WritePageGuard{bpm_, npg};
63-
auto next_page = next_page_guard.AsMut<TablePage>();
64-
next_page->Init();
6566

6667
last_page_id_ = next_page_id;
6768
page_guard = std::move(next_page_guard);

0 commit comments

Comments
 (0)