Skip to content

Commit

Permalink
Add enable_checkpointing control for checkpoint threads
Browse files Browse the repository at this point in the history
Implemented an atomic flag to manage the state of checkpointing, ensuring that checkpoint threads are not redundantly initiated or terminated. This change adds a safety check in the `RunCheckPointThread` and `StopFlushThread` methods to prevent multiple threads from starting or stopping concurrently. Adjusted related configurations and tests to support and validate this new behavior.
  • Loading branch information
loloxwg committed Dec 2, 2024
1 parent 128432e commit 2c1627f
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 2 deletions.
4 changes: 3 additions & 1 deletion src/common/bustub_instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,9 @@ BustubInstance::~BustubInstance() {
if (enable_logging) {
log_manager_->StopFlushThread();
}
checkpoint_manager_->StopFlushThread();
if (enable_checkpointing) {
checkpoint_manager_->StopFlushThread();
}
delete execution_engine_;
delete catalog_;
delete checkpoint_manager_;
Expand Down
1 change: 1 addition & 0 deletions src/common/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
namespace bustub {

std::atomic<bool> enable_logging(false);
std::atomic<bool> enable_checkpointing(false);

std::chrono::duration<int64_t> log_timeout = std::chrono::seconds(1);
std::chrono::duration<int64_t> checkpoint_timeout = std::chrono::seconds(2);
Expand Down
1 change: 1 addition & 0 deletions src/include/common/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ extern std::atomic<bool> enable_logging;
/** If ENABLE_LOGGING is true, the log should be flushed to disk every LOG_TIMEOUT. */
extern std::chrono::duration<int64_t> log_timeout;

extern std::atomic<bool> enable_checkpointing;
extern std::chrono::duration<int64_t> checkpoint_timeout;

static constexpr int INVALID_PAGE_ID = -1; // invalid page id
Expand Down
6 changes: 6 additions & 0 deletions src/recovery/checkpoint_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ void CheckpointManager::EndCheckpoint() {
}

void CheckpointManager::RunCheckPointThread() {
if (enable_checkpointing) {
return;
}

enable_checkpointing = true;
checkpoint_thread_ = new std::thread([&] {
while (true) {
std::this_thread::sleep_for(checkpoint_timeout);
Expand All @@ -42,6 +47,7 @@ void CheckpointManager::RunCheckPointThread() {
}

void CheckpointManager::StopFlushThread() {
enable_checkpointing = false;
checkpoint_thread_->join();
delete checkpoint_thread_;
checkpoint_thread_ = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion test/recovery/recovery_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ TEST_F(RecoveryTest, CheckpointTest) {
auto val_1 = tuple.GetValue(&schema, 1);

// set log time out very high so that flush doesn't happen before checkpoint is performed
log_timeout = std::chrono::seconds(3);
log_timeout = std::chrono::seconds(2);

// insert a ton of tuples
Transaction *txn1 = bustub_instance->txn_manager_->Begin();
Expand Down

0 comments on commit 2c1627f

Please sign in to comment.