From 86e12013f0f95013be4ace5ca56ed1121f95c3f6 Mon Sep 17 00:00:00 2001
From: adiholden <adi@dragonflydb.io>
Date: Wed, 26 Feb 2025 14:02:54 +0200
Subject: [PATCH] server: disable single shard tx optimization on scheduling
 (#4647)

Signed-off-by: adi_holden <adi@dragonflydb.io>
---
 .github/workflows/ci.yml  |  2 +-
 src/server/test_utils.cc  | 10 +++++++++-
 src/server/transaction.cc |  7 ++++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index ba911ccc3..5330c1a49 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -32,7 +32,7 @@ jobs:
           path: ~/.cache/pre-commit
           key: pre-commit|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}
       - name: Run pre-commit checks
-        run:
+        run: |
           source venv/bin/activate
           pre-commit run --show-diff-on-failure --color=always --from-ref HEAD^ --to-ref HEAD
         shell: bash
diff --git a/src/server/test_utils.cc b/src/server/test_utils.cc
index 34ef119da..69548b8af 100644
--- a/src/server/test_utils.cc
+++ b/src/server/test_utils.cc
@@ -265,7 +265,7 @@ void BaseFamilyTest::ResetService() {
       LOG(ERROR) << "Deadlock detected!!!!";
       absl::SetFlag(&FLAGS_alsologtostderr, true);
       fb2::Mutex m;
-      shard_set->pool()->AwaitFiberOnAll([&m](unsigned index, ProactorBase* base) {
+      shard_set->pool()->AwaitFiberOnAll([&m, this](unsigned index, ProactorBase* base) {
         ThisFiber::SetName("Watchdog");
         std::unique_lock lk(m);
         LOG(ERROR) << "Proactor " << index << ":\n";
@@ -293,6 +293,14 @@ void BaseFamilyTest::ResetService() {
                                      ->trans_locks) {
             LOG(ERROR) << "Key " << k_v.first << " " << k_v.second;
           }
+
+          LOG(ERROR) << "Transaction for shard " << es->shard_id();
+          for (auto& conn : connections_) {
+            auto* context = conn.second->cmd_cntx();
+            if (context->transaction && context->transaction->IsActive(es->shard_id())) {
+              LOG(ERROR) << context->transaction->DebugId(es->shard_id());
+            }
+          }
         }
       });
     }
diff --git a/src/server/transaction.cc b/src/server/transaction.cc
index 73de131b0..a34b775b6 100644
--- a/src/server/transaction.cc
+++ b/src/server/transaction.cc
@@ -556,6 +556,7 @@ string Transaction::DebugId(std::optional<ShardId> sid) const {
     absl::StrAppend(&res, ":", multi_->cmd_seq_num);
   }
   absl::StrAppend(&res, " {id=", trans_id(this));
+  absl::StrAppend(&res, " {cb_ptr=", absl::StrFormat("%p", static_cast<const void*>(cb_ptr_)));
   if (sid) {
     absl::StrAppend(&res, ",mask[", *sid, "]=", int(shard_data_[SidToId(*sid)].local_mask),
                     ",is_armed=", DEBUG_IsArmedInShard(*sid),
@@ -756,7 +757,11 @@ void Transaction::ScheduleInternal() {
 
     ScheduleContext schedule_ctx{this, optimistic_exec};
 
-    if (unique_shard_cnt_ == 1) {
+    // TODO: this optimization is disabled due to a issue #4648 revealing this code can
+    // lead to transaction not being scheduled.
+    // To reproduce the bug remove the false in the condition and run
+    // ./list_family_test --gtest_filter=*AwakeMulti on alpine machine
+    if (false && unique_shard_cnt_ == 1) {
       // Single shard optimization. Note: we could apply the same optimization
       // to multi-shard transactions as well by creating a vector of ScheduleContext.
       schedule_queues[unique_shard_id_].queue.Push(&schedule_ctx);