MOD-16382 Add MR_ClusterRefreshTopology for event-driven OSS topology refresh by gabsow · Pull Request #100 · RedisGears/LibMR

gabsow · 2026-06-18T08:06:41Z

Summary

Adds MR_ClusterRefreshTopology() so a consumer module can refresh the LibMR cluster view in response to a Redis server event, instead of requiring an operator to call a manual refresh command (e.g. TIMESERIES.REFRESHCLUSTER) on every primary after each reshard.

What it does

MR_ClusterRefreshTopology() schedules the existing MR_RefreshClusterData() on the LibMR event loop (the thread-safe place to mutate topology), with no blocked client to reply to — the difference from the command-driven MR_ClusterRefresh.
It is gated by clusterCtx.isOss, so it is a no-op outside OSS cluster mode (under Enterprise the DMC drives topology via CLUSTERSET), mirroring the OSS-only registration of REFRESHCLUSTER. Safe to call unconditionally from an event callback.
Declared in cluster.h next to the other MR_Cluster* entry points.

The local redismodule.h copy is updated with the RedisModuleEvent_ClusterTopologyChange definitions (id 20) so LibMR consumers compile against the new event; this mirrors redis/redis#15350 and will be reconciled by the normal header sync.

Used by the RedisTimeSeries consumer PR (auto-refresh on cluster topology change). Verified end-to-end on a 3-node OSS cluster: topology auto-refreshes on formation and reshard with no manual command.

Depends on / mirrors: redis/redis#15350
Relates-to: MOD-9152, RED-148990

Draft — gated on the core event being finalized.

🤖 Generated with Claude Code

Note

Medium Risk
Changes OSS cluster slot routing and when inter-shard connections are torn down; incorrect reconcile could misroute cross-shard traffic until a full refresh runs.

Overview
Adds MR_ClusterRefreshTopology() so consumer modules can refresh LibMR’s cluster view from RedisModuleEvent_ClusterTopologyChange (and related server events) without a manual REFRESHCLUSTER on every primary. The call is OSS-only (clusterCtx.isOss), ignores advisory change_flags, and queues work on the LibMR event loop so it is safe from a Redis server-event callback.

Reconciliation is MR_UpdateClusterSlots(): it builds a master/slot view from the same cluster module APIs as short-form CLUSTERSET (BuildClusterApiView). When the master set and addresses are unchanged, MR_TryApplyTopologyInPlace() clears and repoints slots[] while keeping existing shard connections; otherwise it falls back to the existing full MR_RefreshClusterData() teardown/rebuild.

The vendored redismodule.h gains RedisModuleEvent_ClusterTopologyChange, topology change reason flags, and RedisModuleClusterTopologyChangeInfo so LibMR consumers can compile against the new event (aligned with redis/redis#15350).

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

Subscribe to the new RedisModuleEvent_ClusterTopologyChange server event and refresh the LibMR cluster view whenever the OSS cluster topology changes (startup, resharding, failover). This removes the need for operators to call the undocumented internal TIMESERIES.REFRESHCLUSTER on every primary after cluster creation and every reshard, which was a frequent cause of keyless multi-shard commands (TS.MGET, TS.MRANGE, TS.MREVRANGE, TS.QUERYINDEX) not aggregating across shards. The callback simply calls the new LibMR MR_ClusterRefreshTopology(), which is a no-op outside OSS cluster mode, so the subscription is unconditional and safe. On servers that predate the event the subscription returns an (ignored) error and the module behaves exactly as before. Bumps the LibMR and RedisModulesSDK submodules for the new API and event id. Depends-on: redis/redis#15350, RedisGears/LibMR#100, RedisModulesSDK#89 Relates-to: MOD-9152, RED-148990 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

gabsow · 2026-06-23T12:32:49Z

Verified end-to-end against redis/redis#15350 on a live 4-shard scale-out (3-shard OSS cluster + RedisTimeSeries, then add a 4th shard and reshard 4096 slots with key migration):

node	raw topology events	actual refreshes
7811 (src)	4097	2
7812	8193	2
7813	8193	2
7814 (new)	4093	1

Cross-shard TS.QUERYINDEX/TS.MRANGE returned the complete result (24/24, including series migrated to the brand-new shard) on every node, with no TIMESERIES.REFRESHCLUSTER. The trailing debounce collapses the per-slot CLUSTER SETSLOT event storm (~8k events) into 1-2 refreshes, avoiding thousands of connection teardowns.

Subscribe to the new RedisModuleEvent_ClusterTopologyChange server event and refresh the LibMR cluster view whenever the OSS cluster topology changes (startup, resharding, failover). This removes the need for operators to call the undocumented internal TIMESERIES.REFRESHCLUSTER on every primary after cluster creation and every reshard, which was a frequent cause of keyless multi-shard commands (TS.MGET, TS.MRANGE, TS.MREVRANGE, TS.QUERYINDEX) not aggregating across shards. The callback calls the new LibMR MR_ClusterRefreshTopology(), which trailing- debounces the per-slot event storm of a reshard into a single refresh and is a no-op outside OSS cluster mode. On servers that predate the event the subscription returns an (ignored) error and the module behaves exactly as before. Tests (tests/flow/test_asm.py): - add test_auto_refresh_on_topology_change: with the initial REFRESHCLUSTER skipped, asserts the module auto-discovers the cluster and keeps cross-shard queries complete across an ASM migration, with no manual command. Gated to skip on servers without the event. - update test_short_form_clusterset: its "module starts unaware" premise no longer holds once the event fires (the module auto-refreshes on startup), so the unaware assertion now runs only when auto-refresh is absent; the short-form CLUSTERSET path is still fully exercised on servers without the event. Bumps the LibMR and RedisModulesSDK submodules for the new API and event id. Depends-on: redis/redis#15350, RedisGears/LibMR#100, RedisModulesSDK#89 Relates-to: MOD-9152, RED-148990 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Expose MR_ClusterRefreshTopology(): refreshes the LibMR cluster view in response to a Redis server event instead of requiring a manual REFRESHCLUSTER on every primary. Gated by clusterCtx.isOss so it is a no-op outside OSS cluster mode. A reshard issues CLUSTER SETSLOT per slot, so the topology-change event can fire thousands of times in a burst; refreshing per event would tear down and rebuild every inter-shard connection thousands of times (MR_RefreshClusterData frees the whole cluster). So the refresh is trailing-debounced via an event-loop task: each event only bumps a counter, and a single refresh runs once the counter has been stable for one debounce window. Verified on a live 4-shard reshard: ~8000 events collapse to 1-2 refreshes while cross-shard queries stay complete. Also vendors the RedisModuleEvent_ClusterTopologyChange definitions (event id 20, subevents, info struct) into the local redismodule.h so consumers can subscribe; this mirrors redis/redis#15350 and will be reconciled by the normal redismodule.h sync. Relates-to: MOD-9152, RED-148990 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Subscribe to the new RedisModuleEvent_ClusterTopologyChange server event and refresh the LibMR cluster view whenever the OSS cluster topology changes (startup, resharding, failover). This removes the need for operators to call the undocumented internal TIMESERIES.REFRESHCLUSTER on every primary after cluster creation and every reshard, which was a frequent cause of keyless multi-shard commands (TS.MGET, TS.MRANGE, TS.MREVRANGE, TS.QUERYINDEX) not aggregating across shards. The callback calls the new LibMR MR_ClusterRefreshTopology(), which trailing- debounces the per-slot event storm of a reshard into a single refresh and is a no-op outside OSS cluster mode. On servers that predate the event the subscription returns an (ignored) error and the module behaves exactly as before. Tests (tests/flow/test_asm.py): - add test_auto_refresh_on_topology_change: with the initial REFRESHCLUSTER skipped, asserts the module auto-discovers the cluster and keeps cross-shard queries complete across an ASM migration, with no manual command. Gated to skip on servers without the event. - update test_short_form_clusterset: its "module starts unaware" premise no longer holds once the event fires (the module auto-refreshes on startup), so the unaware assertion now runs only when auto-refresh is absent; the short-form CLUSTERSET path is still fully exercised on servers without the event. Bumps the LibMR and RedisModulesSDK submodules for the new API and event id. Depends-on: redis/redis#15350, RedisGears/LibMR#100, RedisModulesSDK#89 Relates-to: MOD-9152, RED-148990 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…esh on in-place reshard MR_ClusterRefreshTopology takes the change_flags reason bitmask from RedisModuleEvent_ClusterTopologyChange and picks the cheapest refresh that stays correct: - FLAG_NODE / FLAG_ROLE / FLAG_STATE (a node joined/left, a role flip, or an OK/FAIL transition -- the set of primaries may have changed) -> full MR_RefreshClusterData (reconnect to the new set of primaries). - FLAG_SLOT only (an in-place reshard: slots moved between primaries we are already connected to) -> new MR_UpdateClusterSlots, which repoints slot->node routing while reusing the existing connections, so in-flight fan-out / cross-shard queries are not aborted and slot-routed queries stay correct mid-reshard. The debounce accumulates the reason flags across the coalesced window and is race-safe: whichever refresh observes a rebuild-worthy flag does the full rebuild, so a membership/role/state change is never masked by the slot-map fast path. MR_UpdateClusterSlots self-upgrades to a full rebuild if an unknown shard appears. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…the reason flags MR_UpdateClusterSlots now reconciles against a fresh CLUSTER SLOTS and rebuilds (which aborts in-flight cross-shard executions) only when the set of slot-serving primaries actually changed -- a primary entered (unknown shard) or left (distinct-count shrank). Otherwise it repoints the slot map in place, preserving connections and in-flight commands. So an over-broad or spurious topology event -- a replica re-pointing, an OK<->FAIL flip, a slotless node joining -- no longer drops in-flight multi-key commands. The debounced handler always calls this single reconcile path; the event's reason flags become advisory (change_flags is no longer consulted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

gabsow · 2026-07-13T07:51:35Z

@galcohen-redislabs — design decision for you: do we keep the topology-refresh debounce, and in what shape?

What it is: every topology event just bumps a counter; the actual reconcile (MR_UpdateClusterSlots: GIL lock + CLUSTER SLOTS + slot-map repoint) runs only after 100 ms pass with no new events (MR_TOPOLOGY_DEBOUNCE_MS, trailing-edge).

Why it exists: to collapse event bursts into one reconcile. Historically it also prevented N connection teardowns — but the primary-set diff in this PR now handles that (rebuild only when a primary enters/leaves), so today the debounce only prevents reconcile churn, not command drops.

Measured event volume per action (live 3x3, patched core):

action	events per node	debounce benefit
ASM range migration (101 slots)	exactly 1, at takeover; 0 on abort	none — pure +100 ms convergence delay
failover / fail-back	1–2	~none
add replica (MEET + REPLICATE)	2–3, seconds apart	~none (separate quiet windows)
legacy per-slot SETSLOT reshard	~1 per slot per master	real: N reconciles → 1

The cost of keeping it as-is: after an ASM takeover we keep routing on the stale map for up to 100 ms — cross-shard commands hitting moved slots get the transient unavailable-slots error until the reconcile runs. Correctness holds (verified in test_asm.py — only whitelisted transient errors, results converge), but the window is avoidable.

Options:

Leading + trailing (my recommendation): reconcile immediately on the first event of a burst, plus one trailing reconcile after 100 ms of quiet. ASM/failover converge with zero added latency; a legacy burst costs ~2 reconciles instead of N. ~10-line change.
Trailing-only (current): simplest, but pays 100 ms on every ASM migration for a burst that, per the measurements, doesn't exist there.
No debounce: fastest convergence everywhere; a legacy 1,000-slot rebalance would run ~1,000 GIL-locked CLUSTER SLOTS reconciles over the reshard window.

Your call — happy to implement whichever you pick.

Review feedback (keep it as simple as possible): every topology-change event now schedules one reconcile task directly on the event loop. MR_UpdateClusterSlots already makes a redundant reconcile harmless (one CLUSTER SLOTS read, connection-preserving unless the primary set changed), so the debounce only saved reconcile churn during legacy per-slot resharding bursts, at the cost of delaying convergence by the quiet window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The in-place reconcile matched primaries by node id only, so a primary restarting at a new address with the same id (a pod reschedule that keeps nodes.conf) kept its stale ip/port and the reconnect loop redialed the dead address forever. Compare the address from the fresh CLUSTER SLOTS reply (port via RedisModule_GetClusterNodeInfo, matching MR_RefreshClusterData) and escalate to a full rebuild on mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

gabsow · 2026-07-13T09:12:19Z

Per the review call (keep it simple): b35bab1 drops the debounce entirely — every event now schedules one reconcile directly on the event loop; the cluster.h comment is updated to match the primary-set-diff behavior.

Self-review also caught a gap in the in-place path: a primary restarting at a new address with the same node id (pod reschedule keeping nodes.conf) kept its stale ip/port, so the reconnect loop would redial the dead address forever. d3830c1 compares the address from the fresh CLUSTER SLOTS (port via RedisModule_GetClusterNodeInfo, same as MR_RefreshClusterData) and escalates to a full rebuild on mismatch.

test_asm 5/5 green on OSS cluster with the patched core after both commits.

galcohen-redislabs · 2026-07-13T11:25:05Z

+ * In OSS mode CurrCluster->slots is the routing source of truth (the SendMsgType_BySlot
+ * path) and fan-out (SendMsgType_All) iterates the nodes dict; both stay correct here.
+ * Per-node slotRanges are only consumed by the Enterprise CLUSTERSET build and the
+ * debug RG.INFOCLUSTER reply, so they are intentionally left untouched. */


I don't see any point in this long and mostly confusing comment (what's an in-place reshard? why mention oss-mode? or in-flight multi-key commands?)

Trimmed in 6118f92.

galcohen-redislabs · 2026-07-13T11:30:21Z

+    }
+
+    RedisModule_ThreadSafeContextLock(mr_staticCtx);
+    RedisModuleCallReply *allSlotsReply = RedisModule_Call(mr_staticCtx, "cluster", "c", "slots");


We already have the RedisModule_GetClusterNodeSlotRanges(). Why call (and parse) redis commands?

Right — switched to GetClusterNodesList/GetClusterNodeInfo/GetClusterNodeSlotRanges (the short form's source) in 6118f92. Also drops the CLUSTER SLOTS non-TLS-port workaround. Falls back to a full refresh when the API is unavailable.

galcohen-redislabs · 2026-07-13T11:34:56Z

+ * path) and fan-out (SendMsgType_All) iterates the nodes dict; both stay correct here.
+ * Per-node slotRanges are only consumed by the Enterprise CLUSTERSET build and the
+ * debug RG.INFOCLUSTER reply, so they are intentionally left untouched. */
+static void MR_UpdateClusterSlots(){


In the context of LibMR we only care about the set of live master nodes (i.e., since we simply call all of them and don't care for slots at all). Why do anything more complex than that?
Also - short-form CLUSTERSET already builds when we need, so better to refactor it (e.g., to return a data struct that represents the set of primary nodes and is comparable with other objects of that struct) than adding a new function.
(we might want a refined logic of "all master nodes except slotless ones" but honestly I think this is an unnecessary optimization; especially since slotless nodes are added to a cluster and immediately get some slots migrated towards them).

Done in 6118f92: the view is now a comparable struct (TopologyViewEntry) built by BuildClusterApiView() from the same source the short form uses, and the decision is a plain master-set compare — ids + addresses (an address change must reconnect, so it escalates). Slotless masters are skipped per your note. The slot-map repoint stays as the cheap apply step since LibMR's public API exposes slot routing (SendMsgBySlot/IsMySlot). #104 is being restacked to move both CLUSTERSET forms onto this same mechanism, as you suggested.

Per review: drop the CLUSTER SLOTS RM_Call+parse in favor of the same cluster module API the short-form CLUSTERSET builds from (GetClusterNodesList/GetClusterNodeInfo/GetClusterNodeSlotRanges), expressed as a comparable topology-view struct; the decision is a plain master-set compare (ids + addresses). Falls back to a full refresh when the API is unavailable. Trims the over-long comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

cursor

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

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

cursor · 2026-07-13T12:32:27Z

+    if (view)
+        array_free(view);
+    if (!applied)
+        MR_RefreshClusterData();


Empty view skips full refresh

Medium Severity

When BuildClusterApiView yields zero slot-bearing masters but CurrCluster already exists with an empty nodes dict, MR_TryApplyTopologyInPlace still returns success. MR_UpdateClusterSlots then skips MR_RefreshClusterData, so event-driven topology refresh never repopulates masters from the cluster API.

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

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

This was referenced Jun 18, 2026

MOD-16382 Auto-refresh cluster topology on OSS topology changes (drop manual REFRESHCLUSTER) RedisTimeSeries/RedisTimeSeries#2071

Draft

Add RedisModuleEvent_ClusterTopologyChange server event redis/redis#15350

Merged

gabsow force-pushed the feature/cluster-topology-auto-refresh branch from 9e84dce to 9edcb3d Compare June 23, 2026 12:31

gabsow changed the title ~~Add MR_ClusterRefreshTopology for event-driven OSS topology refresh~~ MOD-16382 Add MR_ClusterRefreshTopology for event-driven OSS topology refresh Jun 23, 2026

gabsow requested a review from galcohen-redislabs June 23, 2026 13:04

gabsow force-pushed the feature/cluster-topology-auto-refresh branch from 9edcb3d to fe9d618 Compare June 24, 2026 03:14

gabsow force-pushed the feature/cluster-topology-auto-refresh branch from fe9d618 to 74d5753 Compare June 24, 2026 10:17

gabsow force-pushed the feature/cluster-topology-auto-refresh branch from 095f5d7 to f481460 Compare June 25, 2026 06:09

gabsow marked this pull request as ready for review June 29, 2026 14:26

cursor Bot reviewed Jun 29, 2026

View reviewed changes

Comment thread src/cluster.c Outdated

gabsow and others added 2 commits July 13, 2026 12:00

galcohen-redislabs reviewed Jul 13, 2026

View reviewed changes

gabsow requested a review from galcohen-redislabs July 13, 2026 12:30

cursor Bot reviewed Jul 13, 2026

View reviewed changes

gabsow mentioned this pull request Jul 13, 2026

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

Merged

MOD-16382 Sync vendored NODE-flag comment with merged redis/redis header

6f4bc06

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

gabsow mentioned this pull request Jul 15, 2026

MOD-16382/16399 Unified topology reconcile on de-globalized structure (#104 + #105) #106

Draft

Navigation Menu

Uh oh!

MOD-16382 Add MR_ClusterRefreshTopology for event-driven OSS topology refresh - #100

MOD-16382 Add MR_ClusterRefreshTopology for event-driven OSS topology refresh#100
gabsow wants to merge 7 commits into
RedisGears:masterfrom
gabsow:feature/cluster-topology-auto-refresh

gabsow commented Jun 18, 2026 •

edited by cursor Bot

Loading

Uh oh!

gabsow commented Jun 23, 2026

Uh oh!

Uh oh!

gabsow commented Jul 13, 2026

Uh oh!

gabsow commented Jul 13, 2026

Uh oh!

galcohen-redislabs Jul 13, 2026

Uh oh!

gabsow Jul 13, 2026

Uh oh!

galcohen-redislabs Jul 13, 2026

Uh oh!

gabsow Jul 13, 2026

Uh oh!

galcohen-redislabs Jul 13, 2026

Uh oh!

gabsow Jul 13, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gabsow commented Jun 18, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What it does

Uh oh!

gabsow commented Jun 23, 2026

Uh oh!

Uh oh!

gabsow commented Jul 13, 2026

Uh oh!

gabsow commented Jul 13, 2026

Uh oh!

galcohen-redislabs Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

gabsow Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

galcohen-redislabs Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

gabsow Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

galcohen-redislabs Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

gabsow Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 13, 2026

Choose a reason for hiding this comment

Empty view skips full refresh

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gabsow commented Jun 18, 2026 •

edited by cursor Bot

Loading