MOD-16399 Skip the rebuild when CLUSTERSET carries an unchanged topology

[gabsow]

Summary

• Handle only long-form CLUSTERSET.
• Before parsing or building cluster structures, compare the incoming arguments with the command stored on the current cluster.
• Ignore the command name and MYID; every topology-bearing argument must match.
• Preserve the current cluster, connections, and run ID for an identical command. Any changed argument follows the existing full rebuild path.
• Leave short-form CLUSTERSET, Redis core topology events, and the existing internal cluster comparison unchanged from master.

The final diff changes two files: 25 production lines plus focused test coverage. The previously published commit history is preserved.

Tests

• Identical long-form no-op test: passed.
• testMassiveClusterSet: 1,000 alternating changed topologies passed.
• Full standalone test_network.py: 19 tests completed, 0 failed.
• RedisTimeSeries integration coverage: RedisTimeSeries#2088.

---

Note

Medium Risk
Touches cluster topology application in libMR; incorrect equality could skip needed rebuilds or miss updates, though comparison is narrow and tests cover identical vs changed args.

Overview
Long-form CLUSTERSET no longer tears down and rebuilds cluster state when the incoming topology matches what is already stored. Before parsing, IsSameLongFormClusterSet compares every argument except MYID (index 6) against the saved clusterSetCommand, using RedisModule_StringPtrLen lengths so embedded NUL bytes are compared correctly. A match logs and returns OK without changing connections or the cluster run ID; any differing argument still goes through SetClusterDataLongForm. Short-form CLUSTERSET and other topology paths are unchanged.

Tests add testIdenticalLongFormClusterSetIsNoOp (stable run ID on repeat, rebuild on byte-level arg change and shard-id change) and adjust testMassiveClusterSet to alternate mock shard IDs so repeated updates still exercise rebuilds.

^{Reviewed by Cursor Bugbot for commit 4d1759e. Bugbot is set up for automated code reviews on this repo. Configure here.}

[galcohen-redislabs]

Well.. for the same reasons we should do the same for the short-form (which probably means we need to refactor SetClusterDataShortForm() such that the topology data (or at least the part that's relevant for us) is returned and could be released and be compared to the one generated by the next short-form clusterset).
Fortunately, we can skip the case of comparing short to long form topology results, since no version of a module should get both formats.

[gabsow]

Done in 6f7b604. Rather than returning the intermediate node structures from SetClusterDataShortForm, the derived master set is serialized into a canonical snapshot string (one <id> <ip>:<port> <slot-ranges> line per master, sorted by node id so the nodes-list iteration order doesn't matter), stored on the Cluster, and compared against the snapshot freshly derived for the next short-form command. The AUTH password is intentionally not part of the snapshot — it arrives in the arguments, which are compared separately (so a password change still rebuilds). Long-vs-short comparison is naturally excluded by the argc check, per your note.

Verified empirically on a live 2-node OSS cluster: initial short form rebuilds, identical re-send skips, re-send after a CLUSTER SETSLOT move rebuilds (fresh snapshot differs from the stored one), identical re-send skips again. Full pytest suite still passes (34 run, 0 failed).

[gabsow]

Restacked per the review direction on #100 (and today's discussion): this PR now sits on top of #100 and moves both CLUSTERSET forms onto the same reconcile mechanism the topology-change event uses, instead of a separate string-snapshot compare.

What changed vs. the previous head:

• The short-form ShortFormTopologySnapshot string compare is gone — the short form now reduces the server's cluster state to the shared comparable view (TopologyViewEntry via BuildClusterApiView, from MOD-16382 Add MR_ClusterRefreshTopology for event-driven OSS topology refresh #100) and reconciles semantically.
• The long form keeps the identical-args fast path, and any changed args are first reconciled through the same mechanism: a reshard that only moves slot ranges between the same shards is now applied in place — connections, in-flight executions and the run id survive (previously any non-identical payload rebuilt). A shard entering/leaving the set, or changing its address or credentials, still tears down and rebuilds.
• Tests updated to the new spec + new coverage: moved-boundary → in-place (run id + live connection survive); re-send of moved args → no-op; changed shard id / changed password → rebuild; testMassiveClusterSet now alternates the shard id so it still stresses 1000 genuine rebuilds.

Validated: RTS test_asm 5/5 (incl. short-form path) and the MOD-16399 flow test (identical → run id kept; changed address → rebuilt) against this head.

Note: until #100 merges, this PR's diff-vs-master shows #100's commits too — review just the top commits (57a2dd1, e8a3578, fafda41).

[gabsow]

CI hang root-caused (docker repro): the rewritten testMassiveClusterSet alternation started at shard id '2' — argument-identical to the topology __enter__ had just applied — so the identical-args skip made iteration 0 a no-op and GetConnection() blocked forever on its timeout-less queue (the 45-min silent cancel). Fixed in 1b5d6c3: alternation starts at '3', and GetConnection now has a finite timeout so any such regression fails fast instead of wedging the job. The skip behaved correctly — the test's phase was wrong.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Reviewed by Cursor Bugbot for commit c23cc6a. Configure here.}