fix: preserve correct command order in MULTI/EXEC transactions (#5954)

* fix: preserve correct command order in MULTI/EXEC transactions

* fix: review comments

* fix: broken test

Author: vyavdoshenko

src/server/main_service.cc

@@ -1620,7 +1620,11 @@ DispatchResult Service::InvokeCmd(const CommandId* cid, CmdArgList tail_args,
   // We are not sending any admin command in the monitor, and we do not want to
   // do any processing if we don't have any waiting connections with monitor
   // enabled on them - see https://redis.io/commands/monitor/
-  if (!ServerState::tlocal()->Monitors().Empty() && (cid->opt_mask() & CO::ADMIN) == 0) {
+  // For EXEC command specifically, we dispatch monitor after executing all queued commands
+  // to preserve correct ordering (MULTI, commands, EXEC) instead of (MULTI, EXEC, commands)
+  bool should_dispatch_monitor = !ServerState::tlocal()->Monitors().Empty() &&
+                                 (cid->opt_mask() & CO::ADMIN) == 0 && cid != exec_cid_;
+  if (should_dispatch_monitor) {
     DispatchMonitor(cntx, cid, tail_args);
   }
 
@@ -2585,6 +2589,13 @@ void Service::Exec(CmdArgList args, const CommandContext& cmd_cntx) {
   }
 
   cntx->cid = exec_cid_;
+
+  // Dispatch EXEC to monitor after all queued commands have been executed
+  // to preserve correct ordering (MULTI, commands, EXEC)
+  if (!ServerState::tlocal()->Monitors().Empty() && (exec_cid_->opt_mask() & CO::ADMIN) == 0) {
+    DispatchMonitor(cntx, exec_cid_, args);
+  }
+
   VLOG(2) << "Exec completed";
 }
 

tests/dragonfly/connection_test.py

@@ -120,7 +120,36 @@ async def test_monitor_command_multi(async_pool):
     expected = CollectedRedisMsg.all_from_src(*expected)
 
     # The order is random due to squashing
-    assert set(expected) == set(collected[2:])
+    assert set(expected) == set(collected[1:-1])
+
+
+"""
+Test MONITOR command preserves correct order for MULTI/EXEC sequences
+Regression test for https://github.com/dragonflydb/dragonfly/issues/5953
+"""
+
+
+@dfly_args({"proactor_threads": 4})
+async def test_monitor_command_multi_exec_order(async_pool):
+    monitor = CollectingMonitor(aioredis.Redis(connection_pool=async_pool))
+    await monitor.start()
+
+    c = aioredis.Redis(connection_pool=async_pool)
+    p = c.pipeline(transaction=True)
+    p.ping()
+    p.set("key1", "value1")
+    p.get("key1")
+    await p.execute()
+
+    collected = await monitor.stop()
+
+    # Verify the commands appear in the correct order: MULTI, PING, SET, GET, EXEC
+    assert len(collected) == 5
+    assert "MULTI" in collected[0].cmd.upper()
+    assert "PING" in collected[1].cmd.upper()
+    assert "SET" in collected[2].cmd.upper()
+    assert "GET" in collected[3].cmd.upper()
+    assert "EXEC" in collected[4].cmd.upper()
 
 
 """