Skip to content

Commit b7c78db

Browse files
skyzhskyzh
authored
p4: add missing test, add hints for dbgtxnmgr (#661)
* p4: add missing test, add hints for dbgtxnmgr Signed-off-by: Alex Chi <[email protected]> * more comments Signed-off-by: Alex Chi <[email protected]> * fix fmt Signed-off-by: Alex Chi <[email protected]> * update scan test Signed-off-by: Alex Chi <[email protected]> * fix terrier bench precision Signed-off-by: Alex Chi <[email protected]> --------- Signed-off-by: Alex Chi <[email protected]>
1 parent 09bed9d commit b7c78db

File tree

5 files changed

+76
-18
lines changed

5 files changed

+76
-18
lines changed

‎src/execution/execution_common.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,26 @@ void TxnMgrDbg(const std::string &info, TransactionManager *txn_mgr, const Table
1919
TableHeap *table_heap) {
2020
// always use stderr for printing logs...
2121
fmt::println(stderr, "debug_hook: {}", info);
22-
// noop
22+
23+
fmt::println(
24+
stderr,
25+
"You see this line of text because you have not implemented `TxnMgrDbg`. You should do this once you have "
26+
"finished task 2. Implementing this helper function will save you a lot of time for debugging in later tasks.");
27+
28+
// We recommend implementing this function as traversing the table heap and print the version chain. An example output
29+
// of our reference solution:
30+
//
31+
// debug_hook: before verify scan
32+
// RID=0/0 ts=txn8 tuple=(1, <NULL>, <NULL>)
33+
// txn8@0 (2, _, _) ts=1
34+
// RID=0/1 ts=3 tuple=(3, <NULL>, <NULL>)
35+
// txn5@0 <del> ts=2
36+
// txn3@0 (4, <NULL>, <NULL>) ts=1
37+
// RID=0/2 ts=4 <del marker> tuple=(<NULL>, <NULL>, <NULL>)
38+
// txn7@0 (5, <NULL>, <NULL>) ts=3
39+
// RID=0/3 ts=txn6 <del marker> tuple=(<NULL>, <NULL>, <NULL>)
40+
// txn6@0 (6, <NULL>, <NULL>) ts=2
41+
// txn3@1 (7, _, _) ts=1
2342
}
2443

2544
} // namespace bustub

‎test/txn/txn_common.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,5 +488,6 @@ void QueryIndex(BustubInstance &instance, const std::string &txn_var_name, Trans
488488
}
489489

490490
using IntResult = std::vector<std::vector<int>>;
491+
using AnyResult = std::vector<std::vector<std::string>>;
491492

492493
} // namespace bustub

‎test/txn/txn_executor_test.cpp

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ TEST(TxnExecutorTest, DISABLED_UpdateConflict) { // NOLINT
420420
auto txn0 = BeginTxn(*bustub, "txn0");
421421
WithTxn(txn0, ExecuteTxn(*bustub, _var, _txn, "INSERT INTO table1 VALUES (0, 0, 0)"));
422422
WithTxn(txn0, CommitTxn(*bustub, _var, _txn));
423+
auto txn_ref = BeginTxn(*bustub, "txn_ref");
423424
TxnMgrDbg("after initialize", bustub->txn_manager_.get(), table_info, table_info->table_.get());
424425
auto txn1 = BeginTxn(*bustub, "txn1");
425426
auto txn2 = BeginTxn(*bustub, "txn2");
@@ -429,11 +430,37 @@ TEST(TxnExecutorTest, DISABLED_UpdateConflict) { // NOLINT
429430
TxnMgrDbg("after txn tainted", bustub->txn_manager_.get(), table_info, table_info->table_.get());
430431
WithTxn(txn1, CommitTxn(*bustub, _var, _txn));
431432
TxnMgrDbg("after commit", bustub->txn_manager_.get(), table_info, table_info->table_.get());
433+
WithTxn(txn_ref, QueryShowResult(*bustub, _var, _txn, "SELECT * FROM table1", IntResult{{0, 0, 0}}));
432434
TableHeapEntryNoMoreThan(*bustub, table_info, 1);
433435
}
434436
{
435437
fmt::println(stderr, "--- UpdateConflict2: complex case with version chain ---");
436-
// TODO(chi)
438+
auto bustub = std::make_unique<BustubInstance>();
439+
Execute(*bustub, "CREATE TABLE table1(a int, b int, c int)");
440+
auto table_info = bustub->catalog_->GetTable("table1");
441+
auto txn0 = BeginTxn(*bustub, "txn0");
442+
WithTxn(txn0, ExecuteTxn(*bustub, _var, _txn, "INSERT INTO table1 VALUES (0, 0, 0), (1, 1, 1)"));
443+
WithTxn(txn0, CommitTxn(*bustub, _var, _txn));
444+
TxnMgrDbg("after initialize", bustub->txn_manager_.get(), table_info, table_info->table_.get());
445+
auto txn1 = BeginTxn(*bustub, "txn1");
446+
auto txn2 = BeginTxn(*bustub, "txn2");
447+
auto txn3 = BeginTxn(*bustub, "txn3");
448+
auto txn4 = BeginTxn(*bustub, "txn4");
449+
auto txn_ref = BeginTxn(*bustub, "txn_ref");
450+
WithTxn(txn1, ExecuteTxn(*bustub, _var, _txn, "UPDATE table1 SET b = 233 WHERE a = 0"));
451+
WithTxn(txn1, CommitTxn(*bustub, _var, _txn));
452+
WithTxn(txn2, ExecuteTxn(*bustub, _var, _txn, "UPDATE table1 SET b = 2333 WHERE a = 1"));
453+
TxnMgrDbg("after updates", bustub->txn_manager_.get(), table_info, table_info->table_.get());
454+
WithTxn(txn3, ExecuteTxnTainted(*bustub, _var, _txn, "UPDATE table1 SET b = 2 WHERE a = 0"));
455+
TxnMgrDbg("after txn3 tainted", bustub->txn_manager_.get(), table_info, table_info->table_.get());
456+
WithTxn(txn4, ExecuteTxnTainted(*bustub, _var, _txn, "UPDATE table1 SET b = 2 WHERE a = 1"));
457+
TxnMgrDbg("after txn4 tainted", bustub->txn_manager_.get(), table_info, table_info->table_.get());
458+
WithTxn(txn2, CommitTxn(*bustub, _var, _txn));
459+
TxnMgrDbg("after commit", bustub->txn_manager_.get(), table_info, table_info->table_.get());
460+
WithTxn(txn_ref, QueryShowResult(*bustub, _var, _txn, "SELECT * FROM table1", IntResult{{0, 0, 0}, {1, 1, 1}}));
461+
auto txn5 = BeginTxn(*bustub, "txn5");
462+
WithTxn(txn5, QueryShowResult(*bustub, _var, _txn, "SELECT * FROM table1", IntResult{{0, 233, 0}, {1, 2333, 1}}));
463+
TableHeapEntryNoMoreThan(*bustub, table_info, 2);
437464
}
438465
}
439466

‎test/txn/txn_scan_test.cpp

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include "execution/execution_common.h"
12
#include "txn_common.h" // NOLINT
23

34
namespace bustub {
@@ -137,7 +138,7 @@ TEST(TxnScanTest, DISABLED_ScanTest) { // NOLINT
137138
auto txn_store_2 = bustub->txn_manager_->Begin();
138139
ASSERT_EQ(txn_store_2->GetReadTs(), 1);
139140
prev_log_3 = txn_store_2->AppendUndoLog(
140-
UndoLog{false, {true, true, true}, Tuple{{Int(4), DoubleNull(), BoolNull()}, schema.get()}, 1, {}});
141+
UndoLog{false, {true, true, true}, Tuple{{Int(4), Double(4), Bool(true)}, schema.get()}, 1, {}});
141142
prev_log_6 =
142143
txn_store_2->AppendUndoLog(UndoLog{false, {true, false, false}, Tuple{{Int(7)}, modify_schema.get()}, 1, {}});
143144
bustub->txn_manager_->Commit(txn_store_2);
@@ -166,7 +167,7 @@ TEST(TxnScanTest, DISABLED_ScanTest) { // NOLINT
166167
auto txn_store_4 = bustub->txn_manager_->Begin();
167168
ASSERT_EQ(txn_store_4->GetReadTs(), 3);
168169
prev_log_4 = txn_store_4->AppendUndoLog(
169-
UndoLog{false, {true, true, true}, Tuple{{Int(5), DoubleNull(), BoolNull()}, schema.get()}, 3, {}});
170+
UndoLog{false, {true, true, true}, Tuple{{Int(5), Double(3), Bool(false)}, schema.get()}, 3, {}});
170171
bustub->txn_manager_->Commit(txn_store_4);
171172
ASSERT_EQ(txn_store_4->GetCommitTs(), 4);
172173
}
@@ -200,23 +201,33 @@ TEST(TxnScanTest, DISABLED_ScanTest) { // NOLINT
200201

201202
TxnMgrDbg("before verify scan", bustub->txn_manager_.get(), table_info, table_info->table_.get());
202203

203-
auto query = "SELECT a FROM maintable";
204+
auto query = "SELECT * FROM maintable";
204205
fmt::println(stderr, "A: Verify txn0");
205-
WithTxn(txn0, QueryShowResult(*bustub, _var, _txn, query, IntResult{}));
206+
WithTxn(txn0, QueryShowResult(*bustub, _var, _txn, query, AnyResult{}));
206207
fmt::println(stderr, "B: Verify txn1");
207-
WithTxn(txn1, QueryShowResult(*bustub, _var, _txn, query, IntResult{{2}, {4}, {7}}));
208+
WithTxn(txn1, QueryShowResult(*bustub, _var, _txn, query,
209+
AnyResult{
210+
{"2", "decimal_null", "boolean_null"},
211+
{"4", "4.000000", "true"},
212+
{"7", "decimal_null", "boolean_null"},
213+
}));
208214

209215
// hidden tests... this is the only hidden test case among task 1, 2, 3. We recommend you to implement `TxnMgrDbg`
210216
// function, draw the version chain out, and think of what should be read by each txn.
211217

218+
// though we don't have null and double / bool types in task 3 and onwards, we will test them in this test case.
219+
// you should think about types other than integer, and think of the case where the user updates / inserts
220+
// a column of null.
221+
222+
// auto query_int = "SELECT a FROM maintable";
212223
// fmt::println(stderr, "C: Verify txn2");
213-
// WithTxn(txn2, QueryHideResult(*bustub, _var, _txn, query, IntResult{}));
224+
// WithTxn(txn2, QueryHideResult(*bustub, _var, _txn, query, IntResult{})); // <- you will need to fill in the answer
214225
// fmt::println(stderr, "D: Verify txn3");
215-
// WithTxn(txn3, QueryHideResult(*bustub, _var, _txn, query, IntResult{}));
226+
// WithTxn(txn3, QueryHideResult(*bustub, _var, _txn, query, IntResult{})); // <- you will need to fill in the answer
216227
// fmt::println(stderr, "E: Verify txn4");
217-
// WithTxn(txn4, QueryHideResult(*bustub, _var, _txn, query, IntResult{}));
228+
// WithTxn(txn4, QueryHideResult(*bustub, _var, _txn, query, IntResult{})); // <- you will need to fill in the answer
218229
// fmt::println(stderr, "F: Verify txn5");
219-
// WithTxn(txn5, QueryHideResult(*bustub, _var, _txn, query, IntResult{}));
230+
// WithTxn(txn5, QueryHideResult(*bustub, _var, _txn, query, IntResult{})); // <- you will need to fill in the answer
220231
}
221232

222233
// NOLINTEND(bugprone-unchecked-optional-access))

‎tools/terrier_bench/terrier.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,12 @@ struct TerrierMetrics {
9696
auto now = ClockMs();
9797
auto elapsed = now - start_time_;
9898
if (elapsed - last_report_at_ > 1000) {
99-
fmt::print(
100-
stderr,
101-
"[{:5.2f}] {}: total_committed_txn={:<8} total_aborted_txn={:<8} throughput={:<8.3} avg_throughput={:<8.3}\n",
102-
elapsed / 1000.0, reporter_, committed_txn_cnt_, aborted_txn_cnt_,
103-
(committed_txn_cnt_ - last_committed_txn_cnt_) / static_cast<double>(elapsed - last_report_at_) * 1000,
104-
committed_txn_cnt_ / static_cast<double>(elapsed) * 1000);
99+
fmt::print(stderr,
100+
"[{:5.2f}] {}: total_committed_txn={:<8} total_aborted_txn={:<8} throughput={:<8.3f} "
101+
"avg_throughput={:<8.3f}\n",
102+
elapsed / 1000.0, reporter_, committed_txn_cnt_, aborted_txn_cnt_,
103+
(committed_txn_cnt_ - last_committed_txn_cnt_) / static_cast<double>(elapsed - last_report_at_) * 1000,
104+
committed_txn_cnt_ / static_cast<double>(elapsed) * 1000);
105105
last_report_at_ = elapsed;
106106
last_committed_txn_cnt_ = committed_txn_cnt_;
107107
}
@@ -467,7 +467,7 @@ auto main(int argc, char **argv) -> int {
467467

468468
size_t bustub_terrier_num = 10;
469469
size_t bustub_thread_cnt = 2;
470-
const size_t bpm_size = 4096; // ensure benchmark does not hit BPM
470+
const size_t bpm_size = 4096; // ensure benchmark does not hit BPM
471471

472472
try {
473473
program.parse_args(argc, argv);

0 commit comments

Comments
 (0)