Skip to content

fix(client): Use mget_nonatomic for RunStorage.read_many on RedisCluster - #846

Merged
chromy merged 2 commits into
mainfrom
chromy/2026-11-05-redis-cluster-mget-nonatomic
Nov 5, 2026
Merged

chromy merged 2 commits into
mainfrom
chromy/2026-11-05-redis-cluster-mget-nonatomic

Conversation

@chromy

@chromy chromy commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

mget in redis-py-cluster returns the same results as (the new) mget_nonatomic
method on clusters in redis >= 4.1.0.
Here we change our use of mget to call mget_nonatomic if defined.

We support:

  • 3.4.1 <= redis <= 3.5.3 via redis-py-cluster 2.1.x
  • redis >= 4.1.0 without redis-py-cluster

Versions of redis since 4.1.0 incorporated the cluster management code.
Currently we handle this in the code here:

if TYPE_CHECKING:
try:
# redis>=4.1 ships cluster support natively
from redis import RedisCluster
except ImportError: # pragma: no cover - redis<4 fallback via the `cluster` extra
from rediscluster import RedisCluster

We make use of mget here:

new_values = self._redis.mget([self._make_key(sk) for sk in storage_keys])
compat_values = self._redis.mget([self._make_key(ck) for ck in compat_keys])
legacy_values = self._redis.mget([self._make_key(lk) for lk in legacy_keys])

This can go to at least 4 backends:

  • redis <= 3.5.3 no-cluster → mget
  • redis-py-cluster 2.1.x cluster → mget (but with mget_nonatomic semantics)
  • redis >= 4.1.0 no-cluster → mget
  • redis >= 4.1.0 cluster → mget_nonatomic

Here is an LLM summary of the status of mget and mget_nonatomic across the various versions:

Client / version mget across slots mget_nonatomic
redis-py-cluster 1.x (on redis-py 2.x) Doesn't raise: one GET per key, not atomic (1.3.6 client.py:675) Doesn't exist
redis-py-cluster 2.x (on redis-py 3.x, last release 2.1.3) Same: one GET per key (2.1.3 client.py:1003) Doesn't exist
redis-py 4.0 No cluster client -
redis-py 4.1.0 (cluster support added in #1660) Raises RedisClusterException (cluster.py:803) Added: one MGET per slot, one after another (commands/cluster.py:26)
redis-py 4.3.0 Raises, async RedisCluster too (#2099) Async version added (commands/cluster.py:205)
redis-py 4.3.4 Raises Per-slot MGETs run as one pipeline (#2230)
redis-py 5.0.0 Raises (cluster.py:1002) Unchanged (commands/cluster.py:156)
redis-py 6.x / 7.x Raises Unchanged
redis-py 8.x (8.1.0) Raises (cluster.py:1494) Unchanged (commands/cluster.py:234)

@chromy
chromy force-pushed the chromy/2026-11-05-redis-cluster-mget-nonatomic branch 2 times, most recently from a4a694b to fb997f6 Compare November 5, 2026 13:42
@chromy
chromy marked this pull request as ready for review November 5, 2026 13:44
@chromy
chromy requested a review from a team as a code owner November 5, 2026 13:44
On redis>=4.1, RedisCluster.mget raises "MGET - all keys must map to the
same key slot" unless every key shares a hash slot. RunStorage.read_many
mgets one key per schedule, so the scheduler crashes on startup with more
than one schedule. The crash comes from moving from redis-py-cluster to
redis-py>=4.1, not from a redis-py major version bump.

Route those mgets through a helper. It calls mget_nonatomic when the client
has one (redis-py>=4.1 RedisCluster) and plain mget otherwise. That covers
StrictRedis on every version, where there are no slots, and redis-py-cluster,
whose mget already splits the keys. None of the existing code switches
between redis-py-cluster and redis-py at runtime; the only switch is the
TYPE_CHECKING import fallback. So this detects the method instead of
importing a class.

Add RedisCluster tests that route every slot to the single CI redis. They
live in their own module and skip when redis.cluster can't be imported, which
is the case on the locked dev version (redis 3.5.3). They pass locally on
redis 5.0.1.

## mget across redis-py versions

Non-cluster (redis.Redis / StrictRedis): mget sends a single MGET on every
version from 3.x to 8.x. mget_nonatomic has never existed there.

| Client / version | mget across slots | mget_nonatomic |
|---|---|---|
| redis-py-cluster 1.x (on redis-py 2.x) | Doesn't raise: one GET per key, not atomic ([1.3.6 client.py:675](https://github.com/Grokzen/redis-py-cluster/blob/1.3.6/rediscluster/client.py#L675)) | Doesn't exist |
| redis-py-cluster 2.x (on redis-py 3.x, last release 2.1.3) | Same: one GET per key ([2.1.3 client.py:1003](https://github.com/Grokzen/redis-py-cluster/blob/2.1.3/rediscluster/client.py#L1003)) | Doesn't exist |
| redis-py 4.0 | No cluster client | - |
| redis-py 4.1.0 (cluster support added in [#1660](redis/redis-py#1660)) | Raises RedisClusterException ([cluster.py:803](https://github.com/redis/redis-py/blob/v4.1.0/redis/cluster.py#L803)) | Added: one MGET per slot, one after another ([commands/cluster.py:26](https://github.com/redis/redis-py/blob/v4.1.0/redis/commands/cluster.py#L26)) |
| redis-py 4.3.0 | Raises, async RedisCluster too ([#2099](redis/redis-py#2099)) | Async version added ([commands/cluster.py:205](https://github.com/redis/redis-py/blob/v4.3.0/redis/commands/cluster.py#L205)) |
| redis-py 4.3.4 | Raises | Per-slot MGETs run as one pipeline ([#2230](redis/redis-py#2230)) |
| redis-py 5.0.0 | Raises ([cluster.py:1002](https://github.com/redis/redis-py/blob/v5.0.0/redis/cluster.py#L1002)) | Unchanged ([commands/cluster.py:156](https://github.com/redis/redis-py/blob/v5.0.0/redis/commands/cluster.py#L156)) |
| redis-py 6.x / 7.x | Raises | Unchanged |
| redis-py 8.x (8.1.0) | Raises ([cluster.py:1494](https://github.com/redis/redis-py/blob/v8.1.0/redis/cluster.py#L1494)) | Unchanged ([commands/cluster.py:234](https://github.com/redis/redis-py/blob/v8.1.0/redis/commands/cluster.py#L234)) |

## redis-py-cluster mget vs redis-py mget_nonatomic

Both return values in input-key order, with None for missing keys, and
neither is atomic across slots.

| | redis-py-cluster mget | redis-py mget_nonatomic |
|---|---|---|
| Commands sent | One GET per key | One MGET per hash slot |
| Round trips | N, one after another | 4.1.0-4.3.3: one per slot, one after another. 4.3.4+: a single pipeline |
| Atomicity | None, even within a slot | Atomic within each slot, not across slots |
| Replica reads | - | Can read from replicas when read_from_replicas is set ([v8.1.0 commands/cluster.py:207](https://github.com/redis/redis-py/blob/v8.1.0/redis/commands/cluster.py#L207)) |

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chromy
chromy force-pushed the chromy/2026-11-05-redis-cluster-mget-nonatomic branch from fb997f6 to 312dc21 Compare November 5, 2026 13:45

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 312dc21. Configure here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chromy
chromy merged commit 4e296cf into main Nov 5, 2026
28 checks passed
@chromy
chromy deleted the chromy/2026-11-05-redis-cluster-mget-nonatomic branch November 5, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants