tests/unit/cluster/cluster-shards.tcl (1)

284-298: 💤 Low value

Add a brief comment explaining why cluster saveconfig is required here.

The save-before-pause is the entire point of this fix, but the reason is non-obvious: under valgrind the SIGSTOP from pause_process can arrive before clusterBeforeSleep() flushes the deferred CLUSTER_TODO_SAVE_CONFIG write, so the restarted node would otherwise read a stale/new random shard id. A one-line comment preserves that intent for future readers.

📝 Suggested comment

     for {set i 0} {$i < 8} {incr i 4} {
         dict set node_ids $i [R $i cluster myshardid]
+        # Force a synchronous nodes.conf flush before SIGSTOP; otherwise the
+        # deferred shard-id save may not reach disk before pause_process (esp. under valgrind).
         R $i cluster saveconfig
         pause_process [srv [expr -1*$i] pid]
     }

As per coding guidelines: "Comment complex test scenarios to explain intent (test documentation)".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/cluster/cluster-shards.tcl` around lines 284 - 298, Add a one-line
comment above the loop that calls "cluster saveconfig" explaining that the
explicit save is required because under valgrind a SIGSTOP from pause_process
can occur before clusterBeforeSleep() flushes the deferred
CLUSTER_TODO_SAVE_CONFIG write; without this forced save the restarted node
(after restart_server) may read a stale or random shard id. Mention the race
with SIGSTOP/valgrind and reference cluster saveconfig, pause_process,
clusterBeforeSleep() and CLUSTER_TODO_SAVE_CONFIG so future readers understand
the intent.