Guarantee kvrocks doesn't delete checkpoint that is creating

apache · Mar 19, 2021 · ff4187b · ff4187b
1 parent a358ddd
commit ff4187b
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 6 deletions.
diff --git a/src/server.cc b/src/server.cc
@@ -507,7 +507,10 @@ void Server::cron() {
       time_t create_time = storage_->GetCheckpointCreateTime();
       time_t access_time = storage_->GetCheckpointAccessTime();
 
-      if (storage_->ExistCheckpoint()) {
+      // Maybe creating checkpoint costs much time if target dir is on another
+      // disk partition, so when we want to clean up checkpoint, we should guarantee
+      // that kvrocks is not creating checkpoint even if there is a checkpoint.
+      if (storage_->ExistCheckpoint() && storage_->IsCreatingCheckpoint() == false) {
         // TODO(shooterit): support to config the alive time of checkpoint
         if ((GetFetchFileThreadNum() == 0 && std::time(nullptr) - access_time > 30) ||
             (std::time(nullptr) - create_time > 24 * 60 * 60)) {

diff --git a/src/storage.cc b/src/storage.cc
@@ -44,6 +44,7 @@ Storage::Storage(Config *config)
       lock_mgr_(16) {
   InitCRC32Table();
   Metadata::InitVersionCounter();
+  SetCreatingCheckpoint(false);
   SetCheckpointCreateTime(0);
   SetCheckpointAccessTime(0);
 }
@@ -608,14 +609,14 @@ Status Storage::ReplDataManager::GetFullReplDataInfo(Storage *storage, std::stri
       LOG(WARNING) << "Fail to create checkpoint, error:" << s.ToString();
       return Status(Status::NotOK, s.ToString());
     }
-
-    // Set checkpoint time
-    storage->SetCheckpointCreateTime(std::time(nullptr));
-    storage->SetCheckpointAccessTime(std::time(nullptr));
+    std::unique_ptr<rocksdb::Checkpoint> checkpoint_guard(checkpoint);
 
     // Create checkpoint of rocksdb
-    std::unique_ptr<rocksdb::Checkpoint> checkpoint_guard(checkpoint);
+    storage->SetCreatingCheckpoint(true);
     s = checkpoint->CreateCheckpoint(data_files_dir);
+    storage->SetCheckpointCreateTime(std::time(nullptr));
+    storage->SetCheckpointAccessTime(std::time(nullptr));
+    storage->SetCreatingCheckpoint(false);
     if (!s.ok()) {
       LOG(WARNING) << "Fail to create checkpoint, error:" << s.ToString();
       return Status(Status::NotOK, s.ToString());

diff --git a/src/storage.h b/src/storage.h
@@ -94,6 +94,7 @@ class Storage {
     static Status CleanInvalidFiles(Storage *storage,
       const std::string &dir, std::vector<std::string> valid_files);
     struct CheckpointInfo {
+      std::atomic<bool>   is_creating;
       std::atomic<time_t> create_time;
       std::atomic<time_t> access_time;
     };
@@ -118,6 +119,8 @@ class Storage {
   };
 
   bool ExistCheckpoint() { return backup_env_->FileExists(config_->checkpoint_dir).ok(); }
+  void SetCreatingCheckpoint(bool yes_or_no)  { checkpoint_info_.is_creating = yes_or_no; }
+  bool IsCreatingCheckpoint() { return checkpoint_info_.is_creating; }
   void SetCheckpointCreateTime(time_t t)  { checkpoint_info_.create_time = t; }
   time_t GetCheckpointCreateTime()  { return checkpoint_info_.create_time; }
   void SetCheckpointAccessTime(time_t t)  { checkpoint_info_.access_time = t; }