Skip to content

Route Master/Replica SCAN continuations to the issuing node #3287 - #3843

Open
lsh1215 wants to merge 3 commits into
redis:mainfrom
lsh1215:3287-masterreplica-scan-readfrom
Open

Route Master/Replica SCAN continuations to the issuing node #3287#3843
lsh1215 wants to merge 3 commits into
redis:mainfrom
lsh1215:3287-masterreplica-scan-readfrom

Conversation

@lsh1215

@lsh1215 lsh1215 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #3287 — undefined SCAN-family results when ReadFrom spreads reads across nodes on a Master/Replica connection.

Problem

SCAN/HSCAN/SSCAN/ZSCAN cursors are node-local state: the cursor value is a physical position in that node's hashtable, so a continuation request (cursor != 0) must return to the node that issued it. On Master/Replica, every replica holds a full copy of the keyspace, so ReadFrom load-balances individual reads across nodes. That is correct for stateless reads, but for the stateful SCAN family it means MasterReplicaConnectionProvider re-selects a read node per call, and a continuation can land on a different node that interprets the cursor against its own table — yielding undefined results (silently missing/duplicated elements).

Measured with a 1000-field hashtable-encoded hash:

ReadFrom.MASTER  -> 1000 / 1000
ReadFrom.ANY     -> ~740-780 / 1000   (nondeterministic)

Fix

Carry the issuing node on the cursor and pin continuations to it (the opposite direction of the cluster "aggregate" scan — here we stick):

  • ScanCursor gains an optional, package-private source (RedisURI), exposed across packages via a new ScanCursorAccessor (following the existing OrderingReadFromAccessor pattern), so the public ScanCursor API is unchanged. Immutable INITIAL/FINISHED reject setSource.
  • RedisCommandBuilder propagates the source hint from the input cursor to the cursor that carries the response, across all scan builders (incl. streaming). Reusing the returned cursor object — as ScanIterator/ScanStream already do — carries it automatically.
  • MasterReplicaChannelWriter recognizes scan commands at the single routing choke point (covers sync/async/reactive): continuations pin to the issuing node; initial requests stamp the selected node onto the returned cursor.
  • MasterReplicaConnectionProvider adds a node-reporting overload and getPinnedConnectionAsync, which fails fast with a descriptive RedisException if the issuing node has left the topology, instead of silently re-routing (which would reintroduce undefined results).

Read load-balancing for non-scan reads and for initial scan selection is unchanged; the cluster scan path is untouched.

Tests

  • Unit: initial-stamp, continuation-pin, fail-fast, non-scan default route, MULTI/WRITE binding, batch stamping, and provider node-notification / pin / fail.
  • Integration (MasterReplicaStickyScanIntegrationTests): {SCAN, HSCAN, SSCAN, ZSCAN} × {sync, async, reactive} under ReadFrom.ANY must return every element, plus a ReadFrom.UPSTREAM control, against the standard docker-env master/replica pair.

Notes for reviewers (draft)

  • I chose the ScanCursorAccessor approach over a public ScanCursor.setSource to avoid widening the public API; happy to switch to public @since 7.7 accessors if you prefer.
  • findNodeByUri matches by host:port; if a failover reassigns the same host:port to a different physical node, the pin would target it — a pre-existing Master/Replica assumption (ConnectionKey), not introduced here.
  • Pinning of scan continuations dispatched inside a batch/transaction is out of scope (documented non-goal); the standard APIs dispatch scans as single commands.
  • Still draft: the integration matrix needs a run against the real docker-env before this is ready for review.

Make sure that:


Note

Medium Risk
Changes read routing for a core command family on Master/Replica connections; behavior is scoped to scan commands with solid tests, but incorrect pinning would still cause subtle data-iteration bugs.

Overview
Fixes #3287: under Master/Replica with ReadFrom load-balancing, SCAN/HSCAN/SSCAN/ZSCAN continuations could hit a different replica and return incomplete or duplicate results because cursors are node-local.

ScanCursor now carries an optional internal RedisURI source (via ScanCursorAccessor, public API unchanged). RedisCommandBuilder copies that hint from the request cursor onto the response cursor for all scan command builders (including streaming). MasterReplicaChannelWriter routes initial scans through read selection while stamping the chosen node on the cursor, and routes continuations through getPinnedConnectionAsync when a source is set; batched reads stamp unscanned cursors on the same selected node. MasterReplicaConnectionProvider adds a listener overload for reporting the selected node and fails fast if the pinned node left the topology.

Unit tests cover propagation, pinning, transactions, and batches; MasterReplicaStickyScanIntegrationTests exercises full iterations across sync/async/reactive under ReadFrom.ANY.

Reviewed by Cursor Bugbot for commit 7df3592. Bugbot is set up for automated code reviews on this repo. Configure here.

lsh1215 added 3 commits July 14, 2026 01:18
SCAN cursors are node-local state: a continuation request must be routed to the node that issued the cursor. ScanCursor now carries an optional source RedisURI so drivers can associate a cursor with its issuing node. The attribute is package-private and exposed across packages through ScanCursorAccessor (following the OrderingReadFromAccessor pattern), leaving the public ScanCursor API unchanged. The immutable INITIAL/FINISHED cursors reject setSource.
Scan command builders now copy the source-node hint from the input cursor to the cursor that carries the response, so reusing the returned cursor object for continuation calls retains the association (as ScanIterator and ScanStream already do). Applied uniformly across all SCAN/HSCAN/SSCAN/ZSCAN builders including streaming variants.
MasterReplicaChannelWriter now recognizes scan commands: continuation requests (a cursor carrying a source) are pinned to the issuing node, while the node selected for an initial request is stamped onto the returned cursor. MasterReplicaConnectionProvider gains a node-reporting overload and getPinnedConnectionAsync, which fails fast with a descriptive RedisException when the issuing node is no longer part of the topology instead of silently routing elsewhere. Read load-balancing for non-scan reads and for initial scan selection is unchanged. Adds unit coverage plus a {SCAN,HSCAN,SSCAN,ZSCAN} x {sync,async,reactive} integration matrix under ReadFrom.ANY with a ReadFrom.UPSTREAM control.
@lsh1215
lsh1215 marked this pull request as ready for review July 13, 2026 17:10
@a-TODO-rov

Copy link
Copy Markdown
Collaborator

Hey @lsh1215
Thanks for the contribution. I am reviewing your PR.

@a-TODO-rov

Copy link
Copy Markdown
Collaborator

Hey again @lsh1215
I am considering another approach that is more consistent with the way the driver handles this problem in cluster mode.
You are welcome to contribute to the discussion #3287 (comment)

@a-TODO-rov a-TODO-rov added the status: on-hold We cannot start working on this issue yet label Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: on-hold We cannot start working on this issue yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Undefined results when using ReadFrom with SCAN family commands

2 participants