Repository navigation
feat(cluster): add scanIterator and HSCAN/SSCAN/ZSCAN iterators - #3529
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66aa795d4c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ) { | ||
| let cursor = options?.cursor ?? '0'; | ||
| do { | ||
| const reply = await this.hScan(key, cursor, options); |
There was a problem hiding this comment.
When useReplicas: true and a hash, set, or sorted set needs multiple pages, each call here is routed independently as a read-only keyed command. _execute therefore invokes getSlotRandomNode, whose slot iterator cycles through the master and replicas, so the cursor returned by one Redis process is sent to a different process on the next page; because scan cursors are node-local, this can omit or duplicate entries. The same pattern is repeated in all five new keyed iterators, so select and pin one node client for the whole iteration (or force every page to the master).
Useful? React with 👍 / 👎.
| } finally { | ||
| // Free the cluster-wide cursor when the caller stops early (or on an | ||
| // error) instead of leaving it to the idle sweep. No-op once done. | ||
| this._self._slots.evictScanCursor(cursor.toString()); |
There was a problem hiding this comment.
When options.cursor is a live token from BLESS SCAN (or another scan-chained command), this.scan correctly rejects it because routeScan checks the cursor label, but this unconditional cleanup then removes that token from the shared scanCursors map. The unrelated scan can no longer continue, so invalid regular-SCAN input corrupts another operation's state; only evict a token after this iterator has acquired or minted a SCAN-labelled cursor.
Useful? React with 👍 / 👎.
Description
RedisClusteralready supports cluster-wideSCAN, but it has noscanIterator(), sofor await (const keys of cluster.scanIterator())fails with "cluster.scanIterator is not a function". This adds it, plus the single-key iterators, as discussed in the issue.scanIterator()is a thin loop overcluster.scan(), which already walks every master. It compares the cursor withcursor.toString() !== '0', so it stops under a Buffer type mapping too.finallyblock callsevictScanCursor, so the cursor is freed right away and not left to the idle sweep.ScanIteratorInterruptedErrorwith the original error ascause. Other errors pass through unchanged, as in the sentinel iterator.ScanIteratorInterruptedErrortakes an optional message so the cluster case doesn't say "Sentinel master change"; the default message is unchanged.hScanIterator,sScanIteratorandzScanIteratorloop over the existing cluster commands, but every page runs on the slot master. Scan cursors are only valid on the node that issued them, and withuseReplicasread-only commands rotate between the master and its replicas, which skips or repeats entries. I also addedhScanValuesIteratorandhScanNoValuesIteratorso the cluster client has the same iterators asRedisClient.docs/scan-iterators.mdand a link to it fromdocs/clustering.md.Closes #1877.
Validation
New cases in
cluster/index.spec.ts:breakleaves no cursor behindScanIteratorInterruptedErrorand is freedcursoris rejected and left aloneuseReplicas: 300 members, all returned, no page sent to a replicacluster/index.spec.tsandscan-cursor.spec.tspass on Linux with Docker (216 passing).tscandnpm run lintare clean.Checklist
npm testpass with this change (including linting)? Ran the cluster specs and lint, not the full suite.Note
Medium Risk
New cluster iteration APIs touch scan cursor lifecycle and command routing (
useReplicas/ master pinning); misbehavior could miss keys or leak cursors, though behavior mirrors existingcluster.scan()and standalone iterators.Overview
Adds
for awaitscan iterators toRedisCluster, matching the standalone client: cluster-widescanIterator()plus keyedhScanIterator,hScanValuesIterator,hScanNoValuesIterator,sScanIterator, andzScanIterator.Cluster-wide iteration loops on existing
cluster.scan()(all masters), compares cursors withtoString()so Buffer type mappings terminate, evicts the virtual cursor when the loop ends early or errors, and throwsScanIteratorInterruptedError(with a cluster-specific message) when the node bound to the cursor leaves the cluster. Keyed*SCANiterators use an internal_masterOnlyview (_forceMaster) so pages stay on the slot master even whenuseReplicasis enabled.ScanIteratorInterruptedErrornow accepts an optional custom message (default unchanged for Sentinel). Docs add cluster scan-iterator guidance inscan-iterators.mdandclustering.md. Integration tests cover full keyspace coverage, Buffer mappings, cursor cleanup, interruption, and replica routing forsScanIterator.Reviewed by Cursor Bugbot for commit c42ed72. Bugbot is set up for automated code reviews on this repo. Configure here.