chore(transaction): Don't call GetLocalMask from blocking controller (#…

…2715) chore: remove reliance on GetLocalMask in BlockingController
dragonflydb · Mar 12, 2024 · 4a9f816 · 4a9f816
1 parent 9ccf2b9
commit 4a9f816
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 17 deletions.
diff --git a/src/server/blocking_controller.cc b/src/server/blocking_controller.cc
@@ -120,16 +120,10 @@ bool BlockingController::DbWatchTable::AddAwakeEvent(string_view key) {
 // Removes tx from its watch queues if tx appears there.
 void BlockingController::FinalizeWatched(ArgSlice args, Transaction* tx) {
   DCHECK(tx);
+  VLOG(1) << "FinalizeBlocking [" << owner_->shard_id() << "]" << tx->DebugId();
 
-  ShardId sid = owner_->shard_id();
-
-  VLOG(1) << "FinalizeBlocking [" << sid << "]" << tx->DebugId();
-
-  uint16_t local_mask = tx->GetLocalMask(sid);
-  bool is_awakened = local_mask & Transaction::AWAKED_Q;
-
-  if (is_awakened)
-    awakened_transactions_.erase(tx);
+  bool removed = awakened_transactions_.erase(tx);
+  DCHECK(!removed || (tx->GetLocalMask(owner_->shard_id()) & Transaction::AWAKED_Q));
 
   auto dbit = watched_dbs_.find(tx->GetDbIndex());
 
@@ -143,9 +137,8 @@ void BlockingController::FinalizeWatched(ArgSlice args, Transaction* tx) {
   // in case those keys still exist.
   for (string_view key : args) {
     bool removed_awakened = wt.UnwatchTx(key, tx);
-    if (removed_awakened) {
-      CHECK(is_awakened) << tx->DebugId() << " " << key << " " << local_mask;
-    }
+    CHECK(!removed_awakened || removed)
+        << tx->DebugId() << " " << key << " " << tx->GetLocalMask(owner_->shard_id());
   }
 
   if (wt.queue_map.empty()) {

diff --git a/src/server/engine_shard_set.cc b/src/server/engine_shard_set.cc
@@ -550,7 +550,7 @@ void EngineShard::PollExecution(const char* context, Transaction* trans) {
     // If the transaction concluded, it must remove itself from the tx queue.
     // Otherwise it is required to stay there to keep the relative order.
     if (is_ooo && !trans->IsMulti())
-      DCHECK_EQ(keep, trans->GetLocalTxqPos(sid) != TxQueue::kEnd);
+      DCHECK_EQ(keep, trans->DEBUG_GetTxqPosInShard(sid) != TxQueue::kEnd);
   }
 }
 

diff --git a/src/server/transaction.h b/src/server/transaction.h
@@ -270,10 +270,6 @@ class Transaction {
   // Get OpArgs for specific shard
   OpArgs GetOpArgs(EngineShard* shard) const;
 
-  uint32_t GetLocalTxqPos(ShardId sid) const {
-    return shard_data_[SidToId(sid)].pq_pos;
-  }
-
   TxId txid() const {
     return txid_;
   }
@@ -356,6 +352,10 @@ class Transaction {
   // Print in-dept failure state for debugging.
   std::string DEBUG_PrintFailState(ShardId sid) const;
 
+  uint32_t DEBUG_GetTxqPosInShard(ShardId sid) const {
+    return shard_data_[SidToId(sid)].pq_pos;
+  }
+
  private:
   // Holds number of locks for each IntentLock::Mode: shared and exlusive.
   struct LockCnt {