fix: Made a hack to avoid deadlock during SAVE

dragonflydb · Jun 19, 2024 · 11da5d3 · 11da5d3
1 parent 9301ebf
commit 11da5d3
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 6 deletions.
diff --git a/src/server/db_slice.h b/src/server/db_slice.h
@@ -470,8 +470,13 @@ class DbSlice {
   void PerformDeletion(PrimeIterator del_it, DbTable* table);
 
   // this is workaround to execute callbacks for db_slice and journal atomically
-  [[nodiscard]] auto GetChangeCbLock() const {
-    return std::shared_lock(cb_mu_);
+  void LockChangeCb() const {
+    return cb_mu_.lock();
+  }
+
+  // this is workaround to execute callbacks for db_slice and journal atomically
+  void UnlockChangeCb() const {
+    return cb_mu_.unlock();
   }
 
  private:

diff --git a/src/server/detail/save_stages_controller.cc b/src/server/detail/save_stages_controller.cc
@@ -254,7 +254,10 @@ void SaveStagesController::SaveDfs() {
 
   // Save shard files.
   auto cb = [this](Transaction* t, EngineShard* shard) {
+    // a hack to avoid deadlock in Transaction::RunCallback(...)
+    shard->db_slice().UnlockChangeCb();
     SaveDfsSingle(shard);
+    shard->db_slice().LockChangeCb();
     return OpStatus::OK;
   };
   trans_->ScheduleSingleHop(std::move(cb));
@@ -294,7 +297,10 @@ void SaveStagesController::SaveRdb() {
   }
 
   auto cb = [snapshot = snapshot.get()](Transaction* t, EngineShard* shard) {
+    // a hack to avoid deadlock in Transaction::RunCallback(...)
+    shard->db_slice().UnlockChangeCb();
     snapshot->StartInShard(shard);
+    shard->db_slice().LockChangeCb();
     return OpStatus::OK;
   };
   trans_->ScheduleSingleHop(std::move(cb));

diff --git a/src/server/transaction.cc b/src/server/transaction.cc
@@ -627,7 +627,7 @@ void Transaction::RunCallback(EngineShard* shard) {
   DCHECK_EQ(shard, EngineShard::tlocal());
 
   RunnableResult result;
-  auto change_callback_lock = shard->db_slice().GetChangeCbLock();
+  shard->db_slice().LockChangeCb();
   try {
     result = (*cb_ptr_)(this, shard);
 
@@ -665,8 +665,10 @@ void Transaction::RunCallback(EngineShard* shard) {
   // Log to journal only once the command finished running
   if ((coordinator_state_ & COORD_CONCLUDING) || (multi_ && multi_->concluding)) {
     LogAutoJournalOnShard(shard, result);
-    change_callback_lock.unlock();
+    shard->db_slice().UnlockChangeCb();
     MaybeInvokeTrackingCb();
+  } else {
+    shard->db_slice().UnlockChangeCb();
   }
 }
 
@@ -1249,11 +1251,11 @@ OpStatus Transaction::RunSquashedMultiCb(RunnableType cb) {
   DCHECK_EQ(unique_shard_cnt_, 1u);
 
   auto* shard = EngineShard::tlocal();
-  auto change_callback_lock = shard->db_slice().GetChangeCbLock();
+  shard->db_slice().LockChangeCb();
   auto result = cb(this, shard);
   shard->db_slice().OnCbFinish();
   LogAutoJournalOnShard(shard, result);
-  change_callback_lock.unlock();
+  shard->db_slice().UnlockChangeCb();
   MaybeInvokeTrackingCb();
 
   DCHECK_EQ(result.flags, 0);  // if it's sophisticated, we shouldn't squash it