MOD-16600 Serialize legacy topology replacement (#116) [8.8 backport]

[gabsow]

What

Clean cherry-pick of 25fe7a3 ("MOD-16600 Serialize legacy topology replacement", #116) onto a37b672 — the LibMR commit pinned by RedisTimeSeries 8.8.x, 8.6 and 8.4. No conflicts, and it brings its own regression coverage (tests/mr_test_module).

Base branch 8.8 is new: it is just a37b672, i.e. exactly what RTS 8.8 pins today, so this PR's diff is only #116. Happy to rename it (rts-8.8, 8.8-maintenance, …) or drop it if the team would rather carry 8.8 backports differently — I created it because a reviewable PR needs a base and RTS pins a bare commit.

Why 8.8 needs it

The assertion #116 prevents is still reproducing on the RE8.8 nightly. Build 100.0.20-7660 (2026-08-05), released TS 80802, shard redis-119:

21:22:23.033  <timeseries> cluster topology changed        (x5 within 4ms)
21:22:23.037  <timeseries> Got cluster set command (long form)
21:22:23.037  === REDIS BUG REPORT START
21:22:23.037  # ==> /workspace/deps/LibMR/src/mr.c:939 'false' is not true

Long-form CLUSTERSET → MR_SetClusterData → MR_ClusterFree() transiently reports a one-shard topology → a concurrent execution takes ExecutionFlag_Local → its StepType_InternalCommand reaches the worker path → assert. Holding the Redis lock across the free/rebuild stops that state ever being observable.

master has carried this since 2026-07-26, but RTS 8.8 / 8.6 / 8.4 all pin a37b672, which predates it. So MOD-16600 currently sits in Awaiting Verification against a lane that structurally cannot verify it — the RE8.8 nightly has never run the fix.

What this does not fix

The same support packages still show the MOD-15896 Enterprise/OSS misdetection: Detected redis oss (cluster-enabled=yes) on the ASM shards (×14 and ×24 in the two packages, versus ×180 correct enterprise (cluster-enabled=no) on the non-cluster DBs). On one shard that produced 1309 rejected timeseries.HELLO, 722 message was not sent because status is not connected, and 97 execution max idle reached.

That is a separate defect and it is unfixed on every line. So this backport should stop the crash but is not expected to make the ASM scale-under-traffic scenario pass on its own — same caveat recorded on MOD-16600 when #116 was opened.

Verification

• git cherry-pick of 25fe7a3 onto a37b672 applies with no conflicts; the resulting diff is byte-identical to MOD-16600 Serialize legacy topology replacement #116's (src/cluster.c 21 lines, plus its tests).
• Behaviour is MOD-16600 Serialize legacy topology replacement #116's, already reviewed and merged on master; nothing new is introduced here.
• Not run on a live Enterprise cluster — the acceptance test is the unchanged RE8.8 ASM nightly once RTS 8.8 bumps deps/LibMR to this commit.

Follow-up

RTS-side pin bumps on 8.8 (and 8.6 / 8.4 if wanted) are the second half; I'll open those once this has a merge commit to point at.

---

Note

Medium Risk
Changes synchronization around cluster topology updates on a path tied to production crashes; scope is limited to lock placement and test coverage, not new topology logic.

Overview
Fixes a race where long-form CLUSTERSET and REFRESHCLUSTER could leave clusterCtx in a transient one-shard state (MR_ClusterFree() sets clusterSize to 1) while the main thread still starts executions. Concurrent internal-command work could then be classified as local and hit an assertion in mr.c.

Production change (cluster.c): MR_ClusterRefreshFromCommand and MR_ClusterSetFromCommand now take RedisModule_ThreadSafeContextLock for the full MR_RefreshClusterData / MR_SetClusterData sequence (including the legacy free-then-rebuild path). Narrow locks around cluster slots / GetClusterNodeInfo inside refresh were removed so locking is owned at the event-loop handler level.

Tests: The lmrtest module gains lmrtest.internalcommand and a registered internal command so executions exercise the same routing path as production. New multi-shard tests overlap heavy CLUSTERSET or repeated REFRESHCLUSTER with internal commands to catch the bug without test-only delays in core code.

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

[gabsow]

Competing alternative opened as #121 — please treat these two as either/or, not both.

Tom's point was that a maintenance branch shouldn't be needed, and checking it out he's right: the only reason RTS 8.8 can't build against master is that MR_Init gained a topologyEvents parameter. Of the 39 MR_* symbols RTS 8.8 references, 32 are declared in our headers, and MR_Init is the single one whose declaration changed — nothing was removed.

#121 restores MR_Init to its old three-argument form and moves the new capability to MR_InitWithTopologyEvents, so RTS 8.8/8.6/8.4 build against master unchanged and every future LibMR fix reaches them by a pin bump instead of a cherry-pick.

If #121 lands, this PR should be closed and the 8.8 branch I created deleted. Keeping this one only makes sense if inheriting master's other post-a37b672 commits turns out to be unacceptable on a release line — #111/#115 change the inter-shard TLS gate, which is the one I'd actually scrutinise.

[gabsow]

Closing — the premise is gone. A maintenance branch was only needed because RTS 8.8 couldn't build against master, and that turned out to be one argument at the consumer's call site: pass topologyEvents = false in the same commit that bumps deps/LibMR. Verified safe (MR_ClusterInit's only diff between the pins is clusterCtx.topologyEvents = topologyEvents;), and on 8.8 the topology-event paths are unreachable anyway since core never emits the notification.

I also checked the other commits 8.8 would inherit by moving to master, since that was the real risk in this decision:

• MOD-16399 Skip the rebuild when CLUSTERSET carries an unchanged topology #104 (skip unchanged CLUSTERSET) and MOD-16956 Fix execution leak when an initiator execution is aborted after completing #108 (execution leak) — wanted on 8.8.
• MOD-16382 Adjustments for topology events #117 / MOD-16382 Preserve topology on gated CLUSTERSET #119 — topology-event adjustments, inert with the flag off.
• MOD-16951 Gate inter-shard TLS on tls-cluster for OSS cluster #111 / MOD-16951 fix checkTLS condition with tls-cluster config #115 — I flagged these as the thing to scrutinise. They gate inter-shard TLS on tls-cluster for the native cluster view paths only and explicitly keep the historical tls-port fallback for long-form CLUSTERSET. 8.8 exhibits the symptom they fix — the MOD-16600 support package for build 7660 shows 97 × execution max idle reached on redis-119. So this is a benefit on 8.8, not a risk.

Deleting the 8.8 branch I created for this PR; it was only ever a37b672, which is reachable from master, so nothing is lost.