DOC-7017 Assert cumulative SCAN totals instead of pinning per-call counts in cmds_generic - #3900
Merged
Conversation
…unts in cmds_generic scan2's five SCAN calls no longer assert that the last call alone returns 18 matches; each client now sums matches across all five calls and asserts the cumulative total (19), which is what SCAN actually guarantees. Applied to redis-py, node-redis, ioredis, predis, jedis, lettuce-async, lettuce-reactive, and go-redis (rust already did this; NRedisStack's scan2 step is an empty stub, untouched). Verifying that fix surfaced a second copy of the same bug one step later: scan3's `SCAN ... TYPE zset` also assumed a single default-COUNT call would find both matches. It was invisible before because scan2 always crashed first — the crash was hiding a second, independent defect. Reproduced the scan3 failure 100% of the time on a fresh, empty Redis instance (not just leftover state from repeated local runs), so it's a real defect and not environmental noise. Fixed the same way: loop until the cursor is exhausted, accumulate, assert on the accumulated set. The lettuce-async/lettuce-reactive fix needed a second pass. The first attempt nested the accumulate-until-done loop's blocking `.join()`/`.block()` calls inside the existing `.thenCompose()`/`.flatMap()` async callback chain those files use for every other step. That deadlocks: the blocking call sits on the same Netty event-loop thread that needs to run to deliver the SCAN response, so the command hangs until Lettuce's own 1-minute command timeout kills it — passed locally as a hang, not a failure, so it needed a timing read to catch. Rewrote scan3 to drop out of the async chain entirely and use plain sequential blocking calls, matching the pattern scan2 already established in these same two files. Learned: a step masked by an earlier crash can hide its own independent bug — fixing the crash can unmask a different defect downstream, not just prove the fix works. Constraint: in lettuce-async/lettuce-reactive cmds_generic, never nest a blocking .join()/.block() call inside a thenCompose/thenAccept/flatMap/doOnNext callback — it deadlocks the event-loop thread that would deliver the response. Rejected: keeping scan3's async-chained thenCompose/thenAccept style and looping inside it | deadlocks Lettuce's event loop; use plain sequential blocking calls instead, as scan1/scan2 already do in both files. Gaps: only verified against local Redis 7.2.7, 3x per client; Redis 8.8 was unavailable. scan4 remains unverified end-to-end — every client that reaches it fails on HSCAN NOVALUES, a separate pre-existing version-gated (Redis 7.4+) issue this branch doesn't touch. Ticket: DOC-7017
Contributor
Contributor
🧠 Redis MemoryFound 5 related items from repository history (5 new this commit):
Memory updated at 548c720 |
Contributor
Author
|
Thanks for the (several) approvals, @dwdougherty :-) |
andy-stark-redis
deleted the
DOC-7017-cmds-generic-scan-2-s-five-call-split-assumption-is-not-guaranteed-flaky-blocks-full-script-runs
branch
August 28, 2026 13:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cmds_generic'sscan2step asserted that the last of five SCAN calls returns exactly 18 matches — an unguaranteed per-call split. Every client (redis-py, node-redis, ioredis, predis, jedis, lettuce-async, lettuce-reactive, go-redis) now asserts the cumulative total across all five calls (19) instead. Rust already did this; NRedisStack'sscan2step is an empty stub, untouched.scan3'sTYPE-filtered SCAN made the identical single-call assumption. Fixed the same way (loop until the cursor is exhausted, accumulate, assert on the total) across the same clients (predis has noscan3step; rust never uses SCAN there).console.asserton this check is now a real, throwing assertion.See the commit message for the full narrative, including a self-caught bug: the first pass at the lettuce-async/lettuce-reactive
scan3fix nested a blocking call inside an async callback and deadlocked the event loop for a full minute per run before being caught and rewritten.Test plan
cmds_genericexample throughbuild/example-test-harnessat least 3x against a live Redis instance;scan2andscan3pass reliably on every run for every client.scan4wasn't possible in this environment — local Redis was 7.2.7 andscan4'sHSCAN ... NOVALUESneeds 7.4+. That's a separate, pre-existing, version-gated issue this PR doesn't touch; every client fails there identically regardless of this change.Related: DOC-6968, DOC-7015.
Note
Low Risk
Changes are limited to documentation example tests and assertions; no production runtime or Redis server behavior is modified.
Overview
cmds_genericSCAN examples no longer assume fixed per-call result sizes fromMATCH/TYPE-filteredSCAN, which Redis does not guarantee.For
scan2, examples across eight clients (go-redis, ioredis, jedis, lettuce-async, lettuce-reactive, node-redis, predis, redis-py) now sum keys from every iteration (including the high-COUNTpass) and assert 19 total matches for*11*, instead of expecting the final call alone to return 18. Docs/comments call out that only the cumulative total is stable.For
scan3, the same clients (except predis, which has noscan3step) loop until the cursor is exhausted, accumulatingTYPE zsetkeys, instead of relying on a singleSCAN. Lettuce async/reactivescan3is rewritten as sequential blocking scans rather than nested reactive chains, avoiding blocking inside callbacks.node-redis replaces non-throwing
console.asserton thescan2check withassert.strictEqual.Reviewed by Cursor Bugbot for commit 548c720. Bugbot is set up for automated code reviews on this repo. Configure here.