From 63939a84a04b611453d582e6b91fce9dd4937acb Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:46:53 -0500 Subject: [PATCH 01/10] fix: allow EVALSHA in ClusterPipeline (#2914) Remove EVALSHA from PIPELINE_BLOCKED_COMMANDS so scripts can be pipelined in cluster mode when keys map to one slot. Slot routing already existed in determine_slot(); update docs and add regression coverage. --- docs/lua_scripting.rst | 8 +++++++- redis/cluster.py | 1 - tests/test_cluster.py | 43 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/docs/lua_scripting.rst b/docs/lua_scripting.rst index bd7b9bc01d..7642e73602 100644 --- a/docs/lua_scripting.rst +++ b/docs/lua_scripting.rst @@ -112,4 +112,10 @@ The following commands are not supported: - ``EVAL_RO`` - ``EVALSHA_RO`` -Using scripting within pipelines in cluster mode is **not supported**. +``EVALSHA`` can be used inside a ``ClusterPipeline``. Keys must still map to +the same hash slot (or ``numkeys`` may be ``0``, in which case the command is +routed to a random primary). The script must already be loaded on the target +node, for example via ``SCRIPT LOAD`` on the cluster client, which loads the +script on all primaries. Other scripting helpers such as ``EVAL``, +``load_scripts``, and ``script_load_for_pipeline`` remain unsupported on +cluster pipelines. diff --git a/redis/cluster.py b/redis/cluster.py index 3ca318031a..ba74bce05a 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -3641,7 +3641,6 @@ def inner(*args, **kwargs): "CONFIG", "DBSIZE", "ECHO", - "EVALSHA", "FLUSHALL", "FLUSHDB", "INFO", diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 052134d04f..4e69279aa0 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -21,6 +21,7 @@ ) from redis.cluster import ( PRIMARY, + PIPELINE_BLOCKED_COMMANDS, REDIS_CLUSTER_HASH_SLOTS, REPLICA, ClusterNode, @@ -3835,6 +3836,35 @@ def test_disconnect_calls_disconnect_on_existing_connections(self, r): mock_node_conn.disconnect.assert_called_once() +def test_evalsha_not_in_pipeline_blocked_commands(): + """EVALSHA must be usable in ClusterPipeline (see #2914).""" + assert "EVALSHA" not in PIPELINE_BLOCKED_COMMANDS + + +def test_evalsha_can_be_queued_on_cluster_pipeline(): + """ + Queuing EVALSHA on a ClusterPipeline must not raise the historical + 'blocked when running redis in cluster mode' error. Slot selection for + EVALSHA is already handled by RedisCluster.determine_slot(). + """ + r = get_mocked_redis_client(host=default_host, port=default_port) + try: + pipe = r.pipeline() + sha = "a" * 40 + returned = pipe.evalsha(sha, 1, "foo", "bar") + assert returned is pipe + + queue = pipe._execution_strategy.command_queue + assert len(queue) == 1 + assert queue[0].args == ("EVALSHA", sha, 1, "foo", "bar") + + assert r.determine_slot("EVALSHA", sha, 1, "foo", "bar") == key_slot(b"foo") + zero_key_slot = r.determine_slot("EVALSHA", sha, 0) + assert 0 <= zero_key_slot < REDIS_CLUSTER_HASH_SLOTS + finally: + r.close() + + @pytest.mark.onlycluster class TestClusterPipeline: """ @@ -3870,6 +3900,19 @@ def test_blocked_arguments(self, r): str(ex.value).startswith("shard_hint is deprecated in cluster mode") is True ) + def test_evalsha_in_pipeline(self, r): + """ + EVALSHA is allowed in ClusterPipeline when keys map to one slot + and the script is already loaded on cluster primaries (#2914). + """ + multiply = "return redis.call('GET', KEYS[1]) * ARGV[1]" + sha = r.script_load(multiply) + # hash tag keeps the key on a known slot for the pipeline + r.set("{user}a", 2) + with r.pipeline() as pipe: + pipe.evalsha(sha, 1, "{user}a", 3) + assert pipe.execute() == [6] + def test_redis_cluster_pipeline(self, r): """ Test that we can use a pipeline with the RedisCluster class From a6d661732930043b09115a38934bab613b6ed4fd Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:54:27 -0500 Subject: [PATCH 02/10] ci: trigger fork CI for #2914 From 3bbd5d830f40b7c25a0f2e4a9f9fa1af7a016df5 Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:14:28 -0500 Subject: [PATCH 03/10] fix: route ClusterPipeline EVALSHA via determine_slot --- redis/cluster.py | 11 +++++++---- tests/test_cluster.py | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/redis/cluster.py b/redis/cluster.py index ba74bce05a..dc67a44498 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -4111,12 +4111,15 @@ def _send_cluster_commands( # in a list of pre-defined request policies command_flag = self.command_flags.get(command) if not command_flag: - # Fallback to default policy + # Fallback to default policy. + # Use determine_slot (not _get_command_keys) so EVAL / + # EVALSHA with numkeys=0 work on Redis <7, matching + # the async ClusterPipeline path. if not self._pipe.get_default_node(): - keys = None + slot = None else: - keys = self._pipe._get_command_keys(*c.args) - if not keys or len(keys) == 0: + slot = self._pipe.determine_slot(*c.args) + if slot is None: command_policies = CommandPolicies() else: command_policies = CommandPolicies( diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 4e69279aa0..41ef06ee0c 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -3859,8 +3859,29 @@ def test_evalsha_can_be_queued_on_cluster_pipeline(): assert queue[0].args == ("EVALSHA", sha, 1, "foo", "bar") assert r.determine_slot("EVALSHA", sha, 1, "foo", "bar") == key_slot(b"foo") - zero_key_slot = r.determine_slot("EVALSHA", sha, 0) - assert 0 <= zero_key_slot < REDIS_CLUSTER_HASH_SLOTS + finally: + r.close() + + +def test_evalsha_zero_keys_pipeline_execute_skips_get_command_keys(): + """ + Sync ClusterPipeline must route EVALSHA via determine_slot, not + COMMAND GETKEYS. Redis <7 fails on GETKEYS for EVALSHA with numkeys=0. + """ + r = get_mocked_redis_client(host=default_host, port=default_port) + try: + mock_all_nodes_resp(r, 42) + sha = "a" * 40 + with r.pipeline() as pipe: + pipe.evalsha(sha, 0) + with patch.object( + pipe, + "_get_command_keys", + side_effect=AssertionError( + "COMMAND GETKEYS must not be used for EVALSHA" + ), + ): + assert pipe.execute() == [42] finally: r.close() @@ -3913,6 +3934,16 @@ def test_evalsha_in_pipeline(self, r): pipe.evalsha(sha, 1, "{user}a", 3) assert pipe.execute() == [6] + def test_evalsha_zero_keys_in_pipeline(self, r): + """ + EVALSHA with numkeys=0 must execute in ClusterPipeline without + calling COMMAND GETKEYS (broken on Redis <7 for this case). + """ + sha = r.script_load("return 42") + with r.pipeline() as pipe: + pipe.evalsha(sha, 0) + assert pipe.execute() == [42] + def test_redis_cluster_pipeline(self, r): """ Test that we can use a pipeline with the RedisCluster class From 649a36bce5b87c5d11391d1f26b98ed1e11260ca Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:34:44 -0500 Subject: [PATCH 04/10] fix: address EVALSHA ClusterPipeline review feedback --- docs/lua_scripting.rst | 13 ++++-- redis/asyncio/cluster.py | 34 +++++++++++++- redis/cluster.py | 73 +++++++++++++++++++++++++----- tests/test_asyncio/test_cluster.py | 68 ++++++++++++++++++++++++++++ tests/test_cluster.py | 50 ++++++++++++++++++++ 5 files changed, 221 insertions(+), 17 deletions(-) diff --git a/docs/lua_scripting.rst b/docs/lua_scripting.rst index 7642e73602..ccebee8e34 100644 --- a/docs/lua_scripting.rst +++ b/docs/lua_scripting.rst @@ -114,8 +114,11 @@ The following commands are not supported: ``EVALSHA`` can be used inside a ``ClusterPipeline``. Keys must still map to the same hash slot (or ``numkeys`` may be ``0``, in which case the command is -routed to a random primary). The script must already be loaded on the target -node, for example via ``SCRIPT LOAD`` on the cluster client, which loads the -script on all primaries. Other scripting helpers such as ``EVAL``, -``load_scripts``, and ``script_load_for_pipeline`` remain unsupported on -cluster pipelines. +routed to a random primary). In a transactional pipeline +(``pipeline(transaction=True)``), zero-key ``EVALSHA`` reuses the +transaction's existing slot when one is already chosen so multiple zero-key +scripts (or a mix with keyed commands) stay single-slot. The script must +already be loaded on the target node, for example via ``SCRIPT LOAD`` on the +cluster client, which loads the script on all primaries. Other scripting +helpers such as ``EVAL``, ``load_scripts``, and ``script_load_for_pipeline`` +remain unsupported on cluster pipelines. diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 9a80a988df..8c71815738 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -75,6 +75,7 @@ LoadBalancingStrategy, block_pipeline_command, get_node_name, + is_zero_key_eval_command, parse_cluster_shards, parse_cluster_shards_unified, parse_cluster_shards_with_str_keys, @@ -2917,6 +2918,8 @@ def __init__(self, pipe: ClusterPipeline) -> None: self._explicit_transaction = False self._watching = False self._pipeline_slots: Set[int] = set() + # True once a keyed (non-slot-agnostic) command has fixed the slot + self._transaction_has_keyed_slot = False self._transaction_node: Optional[ClusterNode] = None self._transaction_connection: Optional[Connection] = None self._executing = False @@ -2925,6 +2928,34 @@ def __init__(self, pipe: ClusterPipeline) -> None: RedisCluster.ERRORS_ALLOW_RETRY + self.SLOT_REDIRECT_ERRORS ) + async def _resolve_transaction_slot(self, *args) -> Optional[int]: + """ + Pick a slot for a transactional pipeline command. + + Zero-key EVAL/EVALSHA can run on any primary. Reuse an existing + transaction slot when present so multiple zero-key scripts (or a + mix with keyed commands) stay single-slot. + """ + if args[0] in self.NO_SLOTS_COMMANDS: + return None + + if is_zero_key_eval_command(*args): + if self._pipeline_slots: + return next(iter(self._pipeline_slots)) + return await self._pipe.cluster_client._determine_slot(*args) + + slot_number = await self._pipe.cluster_client._determine_slot(*args) + if ( + slot_number is not None + and self._pipeline_slots + and slot_number not in self._pipeline_slots + and not self._transaction_has_keyed_slot + ): + # Prior slots came only from zero-key EVAL/EVALSHA; retarget. + self._pipeline_slots.clear() + self._transaction_has_keyed_slot = True + return slot_number + def _get_client_and_connection_for_transaction( self, ) -> Tuple[ClusterNode, Connection]: @@ -2982,7 +3013,7 @@ async def _execute_command( slot_number: Optional[int] = None if args[0] not in self.NO_SLOTS_COMMANDS: - slot_number = await self._pipe.cluster_client._determine_slot(*args) + slot_number = await self._resolve_transaction_slot(*args) if ( self._watching or args[0] in self.IMMEDIATE_EXECUTE_COMMANDS @@ -3327,6 +3358,7 @@ async def reset(self): self._watching = False self._explicit_transaction = False self._pipeline_slots = set() + self._transaction_has_keyed_slot = False self._executing = False def multi(self): diff --git a/redis/cluster.py b/redis/cluster.py index dc67a44498..1781904c50 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -3623,6 +3623,20 @@ def inner(*args, **kwargs): return inner +def is_zero_key_eval_command(*args) -> bool: + """ + True for EVAL/EVALSHA with numkeys=0 (may run on any primary). + """ + if len(args) < 3: + return False + if str(args[0]).upper() not in ("EVAL", "EVALSHA"): + return False + try: + return int(args[2]) == 0 + except (TypeError, ValueError): + return False + + # Blocked pipeline commands PIPELINE_BLOCKED_COMMANDS = ( "BGREWRITEAOF", @@ -4112,20 +4126,26 @@ def _send_cluster_commands( command_flag = self.command_flags.get(command) if not command_flag: # Fallback to default policy. - # Use determine_slot (not _get_command_keys) so EVAL / - # EVALSHA with numkeys=0 work on Redis <7, matching - # the async ClusterPipeline path. - if not self._pipe.get_default_node(): - slot = None - else: - slot = self._pipe.determine_slot(*c.args) - if slot is None: - command_policies = CommandPolicies() - else: + # EVAL/EVALSHA must not use _get_command_keys(): Redis + # <7 breaks on COMMAND GETKEYS when numkeys is 0. + # Other unflagged commands keep the keyless fallback. + if command in ("EVAL", "EVALSHA"): command_policies = CommandPolicies( request_policy=RequestPolicy.DEFAULT_KEYED, response_policy=ResponsePolicy.DEFAULT_KEYED, ) + else: + if not self._pipe.get_default_node(): + keys = None + else: + keys = self._pipe._get_command_keys(*c.args) + if not keys or len(keys) == 0: + command_policies = CommandPolicies() + else: + command_policies = CommandPolicies( + request_policy=RequestPolicy.DEFAULT_KEYED, + response_policy=ResponsePolicy.DEFAULT_KEYED, + ) else: if command_flag in self._pipe._command_flags_mapping: command_policies = CommandPolicies( @@ -4419,6 +4439,8 @@ def __init__(self, pipe: ClusterPipeline): self._explicit_transaction = False self._watching = False self._pipeline_slots: Set[int] = set() + # True once a keyed (non-slot-agnostic) command has fixed the slot + self._transaction_has_keyed_slot = False self._transaction_connection: Optional[Connection] = None self._executing = False self._retry = copy(self._pipe.retry) @@ -4426,6 +4448,34 @@ def __init__(self, pipe: ClusterPipeline): RedisCluster.ERRORS_ALLOW_RETRY + self.SLOT_REDIRECT_ERRORS ) + def _resolve_transaction_slot(self, *args) -> Optional[int]: + """ + Pick a slot for a transactional pipeline command. + + Zero-key EVAL/EVALSHA can run on any primary. Reuse an existing + transaction slot when present so multiple zero-key scripts (or a + mix with keyed commands) stay single-slot. + """ + if args[0] in ClusterPipeline.NO_SLOTS_COMMANDS: + return None + + if is_zero_key_eval_command(*args): + if self._pipeline_slots: + return next(iter(self._pipeline_slots)) + return self._pipe.determine_slot(*args) + + slot_number = self._pipe.determine_slot(*args) + if ( + slot_number is not None + and self._pipeline_slots + and slot_number not in self._pipeline_slots + and not self._transaction_has_keyed_slot + ): + # Prior slots came only from zero-key EVAL/EVALSHA; retarget. + self._pipeline_slots.clear() + self._transaction_has_keyed_slot = True + return slot_number + def _get_client_and_connection_for_transaction(self) -> Tuple[Redis, Connection]: """ Find a connection for a pipeline transaction. @@ -4462,7 +4512,7 @@ def _get_client_and_connection_for_transaction(self) -> Tuple[Redis, Connection] def execute_command(self, *args, **kwargs): slot_number: Optional[int] = None if args[0] not in ClusterPipeline.NO_SLOTS_COMMANDS: - slot_number = self._pipe.determine_slot(*args) + slot_number = self._resolve_transaction_slot(*args) if ( self._watching or args[0] in self.IMMEDIATE_EXECUTE_COMMANDS @@ -4787,6 +4837,7 @@ def reset(self): self._watching = False self._explicit_transaction = False self._pipeline_slots = set() + self._transaction_has_keyed_slot = False self._executing = False def send_cluster_commands( diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 2b2bb6516d..0afedca2a5 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -3863,6 +3863,74 @@ async def test_blocked_methods(self, r: RedisCluster) -> None: "when running redis in cluster mode..." ) + async def test_evalsha_not_blocked_on_cluster_pipeline(self) -> None: + """EVALSHA must be usable on async ClusterPipeline (see #2914).""" + assert "EVALSHA" not in PIPELINE_BLOCKED_COMMANDS + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + pipe = r.pipeline() + sha = "a" * 40 + returned = pipe.evalsha(sha, 1, "foo", "bar") + assert returned is pipe + queue = pipe._execution_strategy._command_queue + assert len(queue) == 1 + assert queue[0].args == ("EVALSHA", sha, 1, "foo", "bar") + finally: + await r.aclose() + + async def test_evalsha_zero_keys_pipeline_execute(self) -> None: + """Async ClusterPipeline must execute EVALSHA with numkeys=0.""" + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + mock_all_nodes_resp(r, 42) + sha = "a" * 40 + async with r.pipeline() as pipe: + pipe.evalsha(sha, 0) + assert await pipe.execute() == [42] + finally: + await r.aclose() + + async def test_evalsha_zero_keys_reuses_slot_in_transaction(self) -> None: + """Zero-key EVALSHA in a transactional async pipeline reuses one slot.""" + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + sha = "a" * 40 + async with r.pipeline(transaction=True) as pipe: + pipe.evalsha(sha, 0) + pipe.evalsha(sha, 0) + slots = pipe._execution_strategy._pipeline_slots + assert len(slots) == 1 + + keyed_slot = key_slot(b"foo") + + async def _fake_determine_slot(*_args, **_kwargs): + return keyed_slot + + with mock.patch.object( + pipe.cluster_client, + "_determine_slot", + side_effect=_fake_determine_slot, + ): + pipe.set("foo", "bar") + assert pipe._execution_strategy._pipeline_slots == {keyed_slot} + assert pipe._execution_strategy._transaction_has_keyed_slot is True + finally: + await r.aclose() + + async def test_evalsha_zero_keys_follows_keyed_slot_in_transaction(self) -> None: + """Zero-key EVALSHA after a keyed slot is fixed reuses that slot.""" + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + sha = "a" * 40 + async with r.pipeline(transaction=True) as pipe: + strategy = pipe._execution_strategy + strategy._pipeline_slots = {key_slot(b"foo")} + strategy._transaction_has_keyed_slot = True + pipe.evalsha(sha, 0) + assert strategy._pipeline_slots == {key_slot(b"foo")} + finally: + await r.aclose() + async def test_empty_stack(self, r: RedisCluster) -> None: """If a pipeline is executed with no commands it should return a empty list.""" p = r.pipeline() diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 41ef06ee0c..cd5e91adc5 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -3886,6 +3886,45 @@ def test_evalsha_zero_keys_pipeline_execute_skips_get_command_keys(): r.close() +def test_evalsha_zero_keys_reuses_slot_in_transaction(): + """ + Zero-key EVALSHA in a transactional pipeline must reuse one slot so + execute() does not raise CrossSlotTransactionError. + """ + r = get_mocked_redis_client(host=default_host, port=default_port) + try: + sha = "a" * 40 + with r.pipeline(transaction=True) as pipe: + pipe.evalsha(sha, 0) + pipe.evalsha(sha, 0) + slots = pipe._execution_strategy._pipeline_slots + assert len(slots) == 1 + + # Keyed follow-up must retarget without needing a live COMMAND map. + keyed_slot = key_slot(b"foo") + with patch.object(pipe, "determine_slot", return_value=keyed_slot): + pipe.set("foo", "bar") + assert pipe._execution_strategy._pipeline_slots == {keyed_slot} + assert pipe._execution_strategy._transaction_has_keyed_slot is True + finally: + r.close() + + +def test_evalsha_zero_keys_follows_keyed_slot_in_transaction(): + """Zero-key EVALSHA after a keyed slot is fixed reuses that slot.""" + r = get_mocked_redis_client(host=default_host, port=default_port) + try: + sha = "a" * 40 + with r.pipeline(transaction=True) as pipe: + strategy = pipe._execution_strategy + strategy._pipeline_slots = {key_slot(b"foo")} + strategy._transaction_has_keyed_slot = True + pipe.evalsha(sha, 0) + assert strategy._pipeline_slots == {key_slot(b"foo")} + finally: + r.close() + + @pytest.mark.onlycluster class TestClusterPipeline: """ @@ -3944,6 +3983,17 @@ def test_evalsha_zero_keys_in_pipeline(self, r): pipe.evalsha(sha, 0) assert pipe.execute() == [42] + def test_evalsha_zero_keys_in_transaction_pipeline(self, r): + """ + Multiple zero-key EVALSHA calls in a transactional pipeline share + one slot and execute successfully. + """ + sha = r.script_load("return 42") + with r.pipeline(transaction=True) as pipe: + pipe.evalsha(sha, 0) + pipe.evalsha(sha, 0) + assert pipe.execute() == [42, 42] + def test_redis_cluster_pipeline(self, r): """ Test that we can use a pipeline with the RedisCluster class From a0124055892c8c4871781726acc064ee892dbdd8 Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:46:12 -0500 Subject: [PATCH 05/10] docs: spell out EVALSHA cluster pipeline NOSCRIPT caveats --- docs/lua_scripting.rst | 52 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/docs/lua_scripting.rst b/docs/lua_scripting.rst index ccebee8e34..52a1aeaece 100644 --- a/docs/lua_scripting.rst +++ b/docs/lua_scripting.rst @@ -112,13 +112,45 @@ The following commands are not supported: - ``EVAL_RO`` - ``EVALSHA_RO`` -``EVALSHA`` can be used inside a ``ClusterPipeline``. Keys must still map to -the same hash slot (or ``numkeys`` may be ``0``, in which case the command is -routed to a random primary). In a transactional pipeline -(``pipeline(transaction=True)``), zero-key ``EVALSHA`` reuses the -transaction's existing slot when one is already chosen so multiple zero-key -scripts (or a mix with keyed commands) stay single-slot. The script must -already be loaded on the target node, for example via ``SCRIPT LOAD`` on the -cluster client, which loads the script on all primaries. Other scripting -helpers such as ``EVAL``, ``load_scripts``, and ``script_load_for_pipeline`` -remain unsupported on cluster pipelines. +``EVALSHA`` can be used inside a ``ClusterPipeline``. Keys must map to the +same hash slot (or ``numkeys`` may be ``0``, in which case the command is +routed to a random primary, exactly as in the non-pipelined case above). In a +transactional pipeline (``pipeline(transaction=True)``), zero-key ``EVALSHA`` +reuses the transaction's existing slot when one is already chosen, so multiple +zero-key scripts (or a mix with keyed commands) stay single-slot. + +``EVAL``, ``load_scripts``, and ``script_load_for_pipeline`` remain +**not supported** on cluster pipelines. + +Important caveats when using ``EVALSHA`` in a cluster pipeline: + +- The Lua script cache in Redis is **per-node and is not replicated**. + ``SCRIPT LOAD`` loads the script onto the *current* primaries only, at the + moment it is called. +- Any topology change -- a failover that promotes a replica, a rolling + upgrade that replaces nodes, resharding, or adding a new shard -- can route + an ``EVALSHA`` to a node whose cache does not contain the script, which + fails with ``NOSCRIPT`` (``redis.exceptions.NoScriptError``). +- Unlike the non-pipelined ``Script`` object, **a cluster pipeline performs no + automatic reload or retry** on ``NOSCRIPT``. Recovery is the caller's + responsibility: catch ``NoScriptError``, re-run ``SCRIPT LOAD``, and + re-execute the pipeline. Because zero-key ``EVALSHA`` is routed to a random + primary, the script must be present on **all** primaries. +- In a **transactional** pipeline, a ``NOSCRIPT`` from ``EVALSHA`` is raised at + ``EXEC`` time and does **not** roll back the other commands (this follows + Redis ``MULTI``/``EXEC`` semantics), so partial application is possible. + +.. code:: python + + >>> from redis.exceptions import NoScriptError + >>> sha = rc.script_load(lua) # loads on all current primaries + >>> def run(): + ... with rc.pipeline() as pipe: + ... pipe.evalsha(sha, 1, "{user}:1") + ... return pipe.execute() + >>> try: + ... result = run() + ... except NoScriptError: + ... # a node was missing the script (e.g. after failover/upgrade) + ... sha = rc.script_load(lua) # reload on current primaries + ... result = run() From b23b5eace17fddb3c07df79f2114252a39c8dcd8 Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:01:54 -0500 Subject: [PATCH 06/10] fix: only lock keyed slot when a transaction slot is known --- redis/asyncio/cluster.py | 3 ++- redis/cluster.py | 3 ++- tests/test_asyncio/test_cluster.py | 36 ++++++++++++++++++++++++++++++ tests/test_cluster.py | 22 ++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 8c71815738..262e35331c 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -2953,7 +2953,8 @@ async def _resolve_transaction_slot(self, *args) -> Optional[int]: ): # Prior slots came only from zero-key EVAL/EVALSHA; retarget. self._pipeline_slots.clear() - self._transaction_has_keyed_slot = True + if slot_number is not None: + self._transaction_has_keyed_slot = True return slot_number def _get_client_and_connection_for_transaction( diff --git a/redis/cluster.py b/redis/cluster.py index 1781904c50..8cbf3618a5 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -4473,7 +4473,8 @@ def _resolve_transaction_slot(self, *args) -> Optional[int]: ): # Prior slots came only from zero-key EVAL/EVALSHA; retarget. self._pipeline_slots.clear() - self._transaction_has_keyed_slot = True + if slot_number is not None: + self._transaction_has_keyed_slot = True return slot_number def _get_client_and_connection_for_transaction(self) -> Tuple[Redis, Connection]: diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 0afedca2a5..468f70f48c 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -3931,6 +3931,42 @@ async def test_evalsha_zero_keys_follows_keyed_slot_in_transaction(self) -> None finally: await r.aclose() + async def test_slotless_command_does_not_lock_keyed_slot_flag(self) -> None: + """Slotless commands must not block later keyed retargeting.""" + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + sha = "a" * 40 + async with r.pipeline(transaction=True) as pipe: + pipe.evalsha(sha, 0) + assert pipe._execution_strategy._transaction_has_keyed_slot is False + + async def _no_slot(*_args, **_kwargs): + return None + + with mock.patch.object( + pipe.cluster_client, + "_determine_slot", + side_effect=_no_slot, + ): + pipe.execute_command("CLIENT TRACKING", "ON") + assert pipe._execution_strategy._transaction_has_keyed_slot is False + + keyed_slot = key_slot(b"foo") + + async def _fake_determine_slot(*_args, **_kwargs): + return keyed_slot + + with mock.patch.object( + pipe.cluster_client, + "_determine_slot", + side_effect=_fake_determine_slot, + ): + pipe.set("foo", "bar") + assert pipe._execution_strategy._pipeline_slots == {keyed_slot} + assert pipe._execution_strategy._transaction_has_keyed_slot is True + finally: + await r.aclose() + async def test_empty_stack(self, r: RedisCluster) -> None: """If a pipeline is executed with no commands it should return a empty list.""" p = r.pipeline() diff --git a/tests/test_cluster.py b/tests/test_cluster.py index cd5e91adc5..192b8125c3 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -3925,6 +3925,28 @@ def test_evalsha_zero_keys_follows_keyed_slot_in_transaction(): r.close() +def test_slotless_command_does_not_lock_keyed_slot_flag(): + """Slotless commands must not block later keyed retargeting.""" + r = get_mocked_redis_client(host=default_host, port=default_port) + try: + sha = "a" * 40 + with r.pipeline(transaction=True) as pipe: + pipe.evalsha(sha, 0) + assert pipe._execution_strategy._transaction_has_keyed_slot is False + + with patch.object(pipe, "determine_slot", return_value=None): + pipe.execute_command("CLIENT TRACKING", "ON") + assert pipe._execution_strategy._transaction_has_keyed_slot is False + + keyed_slot = key_slot(b"foo") + with patch.object(pipe, "determine_slot", return_value=keyed_slot): + pipe.set("foo", "bar") + assert pipe._execution_strategy._pipeline_slots == {keyed_slot} + assert pipe._execution_strategy._transaction_has_keyed_slot is True + finally: + r.close() + + @pytest.mark.onlycluster class TestClusterPipeline: """ From 2b285a53f476c74673d571b3d7b3aafafa0cdf9e Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:20:10 -0500 Subject: [PATCH 07/10] fix: keep zero-key FCALL slot-agnostic and preserve async Script pipeline queuing --- redis/asyncio/cluster.py | 8 ++--- redis/cluster.py | 12 ++++---- redis/commands/core.py | 5 ++++ tests/test_asyncio/test_cluster.py | 48 ++++++++++++++++++++++++++++++ tests/test_cluster.py | 19 ++++++++++++ 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 262e35331c..9195d75113 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -2932,9 +2932,9 @@ async def _resolve_transaction_slot(self, *args) -> Optional[int]: """ Pick a slot for a transactional pipeline command. - Zero-key EVAL/EVALSHA can run on any primary. Reuse an existing - transaction slot when present so multiple zero-key scripts (or a - mix with keyed commands) stay single-slot. + Zero-key EVAL/EVALSHA/FCALL/FCALL_RO can run on any primary. Reuse + an existing transaction slot when present so multiple zero-key + scripts/functions (or a mix with keyed commands) stay single-slot. """ if args[0] in self.NO_SLOTS_COMMANDS: return None @@ -2951,7 +2951,7 @@ async def _resolve_transaction_slot(self, *args) -> Optional[int]: and slot_number not in self._pipeline_slots and not self._transaction_has_keyed_slot ): - # Prior slots came only from zero-key EVAL/EVALSHA; retarget. + # Prior slots came only from zero-key scripts/functions; retarget. self._pipeline_slots.clear() if slot_number is not None: self._transaction_has_keyed_slot = True diff --git a/redis/cluster.py b/redis/cluster.py index 8cbf3618a5..23f1cf9eff 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -3625,11 +3625,11 @@ def inner(*args, **kwargs): def is_zero_key_eval_command(*args) -> bool: """ - True for EVAL/EVALSHA with numkeys=0 (may run on any primary). + True for EVAL/EVALSHA/FCALL/FCALL_RO with numkeys=0 (any primary). """ if len(args) < 3: return False - if str(args[0]).upper() not in ("EVAL", "EVALSHA"): + if str(args[0]).upper() not in ("EVAL", "EVALSHA", "FCALL", "FCALL_RO"): return False try: return int(args[2]) == 0 @@ -4452,9 +4452,9 @@ def _resolve_transaction_slot(self, *args) -> Optional[int]: """ Pick a slot for a transactional pipeline command. - Zero-key EVAL/EVALSHA can run on any primary. Reuse an existing - transaction slot when present so multiple zero-key scripts (or a - mix with keyed commands) stay single-slot. + Zero-key EVAL/EVALSHA/FCALL/FCALL_RO can run on any primary. Reuse + an existing transaction slot when present so multiple zero-key + scripts/functions (or a mix with keyed commands) stay single-slot. """ if args[0] in ClusterPipeline.NO_SLOTS_COMMANDS: return None @@ -4471,7 +4471,7 @@ def _resolve_transaction_slot(self, *args) -> Optional[int]: and slot_number not in self._pipeline_slots and not self._transaction_has_keyed_slot ): - # Prior slots came only from zero-key EVAL/EVALSHA; retarget. + # Prior slots came only from zero-key scripts/functions; retarget. self._pipeline_slots.clear() if slot_number is not None: self._transaction_has_keyed_slot = True diff --git a/redis/commands/core.py b/redis/commands/core.py index d22bd3098d..e8ff9304e2 100644 --- a/redis/commands/core.py +++ b/redis/commands/core.py @@ -10862,10 +10862,15 @@ async def __call__( args = tuple(keys) + tuple(args) # make sure the Redis server knows about the script from redis.asyncio.client import Pipeline + from redis.asyncio.cluster import ClusterPipeline if isinstance(client, Pipeline): # Make sure the pipeline can register the script before executing. client.scripts.add(self) + if isinstance(client, ClusterPipeline): + # ClusterPipeline.evalsha queues synchronously. Awaiting the + # pipeline would call initialize() and drop the queued command. + return client.evalsha(self.sha, len(keys), *args) try: return await client.evalsha(self.sha, len(keys), *args) except NoScriptError: diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 468f70f48c..6f1b0ebba6 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -3967,6 +3967,54 @@ async def _fake_determine_slot(*_args, **_kwargs): finally: await r.aclose() + async def test_zero_key_fcall_allows_keyed_retarget(self) -> None: + """Zero-key FCALL must not lock the slot so a later keyed command can retarget.""" + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + async with r.pipeline(transaction=True) as pipe: + + async def _fcall_slot(*_args, **_kwargs): + return 111 + + with mock.patch.object( + pipe.cluster_client, + "_determine_slot", + side_effect=_fcall_slot, + ): + pipe.execute_command("FCALL", "myfunc", 0) + assert pipe._execution_strategy._pipeline_slots == {111} + assert pipe._execution_strategy._transaction_has_keyed_slot is False + + keyed_slot = key_slot(b"foo") + + async def _fake_determine_slot(*_args, **_kwargs): + return keyed_slot + + with mock.patch.object( + pipe.cluster_client, + "_determine_slot", + side_effect=_fake_determine_slot, + ): + pipe.set("foo", "bar") + assert pipe._execution_strategy._pipeline_slots == {keyed_slot} + assert pipe._execution_strategy._transaction_has_keyed_slot is True + finally: + await r.aclose() + + async def test_async_script_queues_evalsha_on_cluster_pipeline(self) -> None: + """AsyncScript must queue EVALSHA on ClusterPipeline without dropping it.""" + r = await get_mocked_redis_client(host=default_host, port=default_port) + try: + script = r.register_script("return 1") + async with r.pipeline() as pipe: + await script(client=pipe) + queue = pipe._execution_strategy._command_queue + assert len(queue) == 1 + assert queue[0].args[0] == "EVALSHA" + assert queue[0].args[1] == script.sha + finally: + await r.aclose() + async def test_empty_stack(self, r: RedisCluster) -> None: """If a pipeline is executed with no commands it should return a empty list.""" p = r.pipeline() diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 192b8125c3..af43746480 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -3947,6 +3947,25 @@ def test_slotless_command_does_not_lock_keyed_slot_flag(): r.close() +def test_zero_key_fcall_allows_keyed_retarget(): + """Zero-key FCALL must not lock the slot so a later keyed command can retarget.""" + r = get_mocked_redis_client(host=default_host, port=default_port) + try: + with r.pipeline(transaction=True) as pipe: + with patch.object(pipe, "determine_slot", return_value=111): + pipe.execute_command("FCALL", "myfunc", 0) + assert pipe._execution_strategy._pipeline_slots == {111} + assert pipe._execution_strategy._transaction_has_keyed_slot is False + + keyed_slot = key_slot(b"foo") + with patch.object(pipe, "determine_slot", return_value=keyed_slot): + pipe.set("foo", "bar") + assert pipe._execution_strategy._pipeline_slots == {keyed_slot} + assert pipe._execution_strategy._transaction_has_keyed_slot is True + finally: + r.close() + + @pytest.mark.onlycluster class TestClusterPipeline: """ From 4b1196b7b368ccaad241bef293b307b68df94c1f Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Tue, 4 Aug 2026 10:03:53 -0500 Subject: [PATCH 08/10] Address review feedback for cluster EVALSHA pipeline support Narrow zero-key slot handling to EVAL/EVALSHA, add cross-slot regression coverage, and mirror async live-cluster EVALSHA tests. --- docs/lua_scripting.rst | 5 +- redis/asyncio/cluster.py | 8 +-- redis/cluster.py | 12 ++--- tests/test_asyncio/test_cluster.py | 79 ++++++++++++++++++++++-------- tests/test_cluster.py | 44 ++++++++++++----- 5 files changed, 104 insertions(+), 44 deletions(-) diff --git a/docs/lua_scripting.rst b/docs/lua_scripting.rst index 52a1aeaece..4f0bbdaa40 100644 --- a/docs/lua_scripting.rst +++ b/docs/lua_scripting.rst @@ -119,8 +119,9 @@ transactional pipeline (``pipeline(transaction=True)``), zero-key ``EVALSHA`` reuses the transaction's existing slot when one is already chosen, so multiple zero-key scripts (or a mix with keyed commands) stay single-slot. -``EVAL``, ``load_scripts``, and ``script_load_for_pipeline`` remain -**not supported** on cluster pipelines. +``load_scripts`` and ``script_load_for_pipeline`` remain **not supported** +on cluster pipelines. On the sync client, ``ClusterPipeline.eval()`` is also +blocked; the async cluster pipeline has no ``eval`` override. Important caveats when using ``EVALSHA`` in a cluster pipeline: diff --git a/redis/asyncio/cluster.py b/redis/asyncio/cluster.py index 9195d75113..028142d215 100644 --- a/redis/asyncio/cluster.py +++ b/redis/asyncio/cluster.py @@ -2932,9 +2932,9 @@ async def _resolve_transaction_slot(self, *args) -> Optional[int]: """ Pick a slot for a transactional pipeline command. - Zero-key EVAL/EVALSHA/FCALL/FCALL_RO can run on any primary. Reuse - an existing transaction slot when present so multiple zero-key - scripts/functions (or a mix with keyed commands) stay single-slot. + Zero-key EVAL/EVALSHA can run on any primary. Reuse an existing + transaction slot when present so multiple zero-key scripts (or a + mix with keyed commands) stay single-slot. """ if args[0] in self.NO_SLOTS_COMMANDS: return None @@ -2951,7 +2951,7 @@ async def _resolve_transaction_slot(self, *args) -> Optional[int]: and slot_number not in self._pipeline_slots and not self._transaction_has_keyed_slot ): - # Prior slots came only from zero-key scripts/functions; retarget. + # Prior slots came only from zero-key scripts; retarget. self._pipeline_slots.clear() if slot_number is not None: self._transaction_has_keyed_slot = True diff --git a/redis/cluster.py b/redis/cluster.py index 23f1cf9eff..f12057a154 100644 --- a/redis/cluster.py +++ b/redis/cluster.py @@ -3625,11 +3625,11 @@ def inner(*args, **kwargs): def is_zero_key_eval_command(*args) -> bool: """ - True for EVAL/EVALSHA/FCALL/FCALL_RO with numkeys=0 (any primary). + True for EVAL/EVALSHA with numkeys=0 (any primary). """ if len(args) < 3: return False - if str(args[0]).upper() not in ("EVAL", "EVALSHA", "FCALL", "FCALL_RO"): + if str(args[0]).upper() not in ("EVAL", "EVALSHA"): return False try: return int(args[2]) == 0 @@ -4452,9 +4452,9 @@ def _resolve_transaction_slot(self, *args) -> Optional[int]: """ Pick a slot for a transactional pipeline command. - Zero-key EVAL/EVALSHA/FCALL/FCALL_RO can run on any primary. Reuse - an existing transaction slot when present so multiple zero-key - scripts/functions (or a mix with keyed commands) stay single-slot. + Zero-key EVAL/EVALSHA can run on any primary. Reuse an existing + transaction slot when present so multiple zero-key scripts (or a + mix with keyed commands) stay single-slot. """ if args[0] in ClusterPipeline.NO_SLOTS_COMMANDS: return None @@ -4471,7 +4471,7 @@ def _resolve_transaction_slot(self, *args) -> Optional[int]: and slot_number not in self._pipeline_slots and not self._transaction_has_keyed_slot ): - # Prior slots came only from zero-key scripts/functions; retarget. + # Prior slots came only from zero-key scripts; retarget. self._pipeline_slots.clear() if slot_number is not None: self._transaction_has_keyed_slot = True diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 6f1b0ebba6..8e711e2034 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -43,6 +43,7 @@ AskError, ClusterDownError, ConnectionError, + CrossSlotTransactionError, DataError, MaxConnectionsError, MovedError, @@ -3967,37 +3968,40 @@ async def _fake_determine_slot(*_args, **_kwargs): finally: await r.aclose() - async def test_zero_key_fcall_allows_keyed_retarget(self) -> None: - """Zero-key FCALL must not lock the slot so a later keyed command can retarget.""" + async def test_evalsha_zero_keys_then_cross_slot_raises_at_execute(self) -> None: + """ + Zero-key EVALSHA retarget must not swallow a later true cross-slot + conflict: two keyed commands on different slots still raise at execute(). + """ r = await get_mocked_redis_client(host=default_host, port=default_port) try: + sha = "a" * 40 + slot_a = key_slot(b"{foo}a") + slot_b = key_slot(b"{bar}b") + assert slot_a != slot_b async with r.pipeline(transaction=True) as pipe: - - async def _fcall_slot(*_args, **_kwargs): - return 111 - - with mock.patch.object( - pipe.cluster_client, - "_determine_slot", - side_effect=_fcall_slot, - ): - pipe.execute_command("FCALL", "myfunc", 0) - assert pipe._execution_strategy._pipeline_slots == {111} - assert pipe._execution_strategy._transaction_has_keyed_slot is False - - keyed_slot = key_slot(b"foo") + slots = iter([111, slot_a, slot_b]) async def _fake_determine_slot(*_args, **_kwargs): - return keyed_slot + return next(slots) with mock.patch.object( pipe.cluster_client, "_determine_slot", side_effect=_fake_determine_slot, ): - pipe.set("foo", "bar") - assert pipe._execution_strategy._pipeline_slots == {keyed_slot} - assert pipe._execution_strategy._transaction_has_keyed_slot is True + pipe.evalsha(sha, 0) + pipe.set("{foo}a", "1") + pipe.set("{bar}b", "2") + assert pipe._execution_strategy._pipeline_slots == {slot_a, slot_b} + with pytest.raises( + CrossSlotTransactionError, + match=( + "All keys involved in a cluster transaction " + "must map to the same slot" + ), + ): + await pipe.execute() finally: await r.aclose() @@ -4015,6 +4019,41 @@ async def test_async_script_queues_evalsha_on_cluster_pipeline(self) -> None: finally: await r.aclose() + async def test_evalsha_in_pipeline(self, r: RedisCluster) -> None: + """ + EVALSHA is allowed in ClusterPipeline when keys map to one slot + and the script is already loaded on cluster primaries (#2914). + """ + multiply = "return redis.call('GET', KEYS[1]) * ARGV[1]" + sha = await r.script_load(multiply) + await r.set("{user}a", 2) + async with r.pipeline() as pipe: + pipe.evalsha(sha, 1, "{user}a", 3) + assert await pipe.execute() == [6] + + async def test_evalsha_zero_keys_in_pipeline(self, r: RedisCluster) -> None: + """ + EVALSHA with numkeys=0 must execute in ClusterPipeline without + calling COMMAND GETKEYS (broken on Redis <7 for this case). + """ + sha = await r.script_load("return 42") + async with r.pipeline() as pipe: + pipe.evalsha(sha, 0) + assert await pipe.execute() == [42] + + async def test_evalsha_zero_keys_in_transaction_pipeline( + self, r: RedisCluster + ) -> None: + """ + Multiple zero-key EVALSHA calls in a transactional pipeline share + one slot and execute successfully. + """ + sha = await r.script_load("return 42") + async with r.pipeline(transaction=True) as pipe: + pipe.evalsha(sha, 0) + pipe.evalsha(sha, 0) + assert await pipe.execute() == [42, 42] + async def test_empty_stack(self, r: RedisCluster) -> None: """If a pipeline is executed with no commands it should return a empty list.""" p = r.pipeline() diff --git a/tests/test_cluster.py b/tests/test_cluster.py index af43746480..610f95c0cc 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -42,6 +42,7 @@ AskError, ClusterDownError, ConnectionError, + CrossSlotTransactionError, DataError, MovedError, NoPermissionError, @@ -3836,11 +3837,13 @@ def test_disconnect_calls_disconnect_on_existing_connections(self, r): mock_node_conn.disconnect.assert_called_once() +@pytest.mark.onlycluster def test_evalsha_not_in_pipeline_blocked_commands(): """EVALSHA must be usable in ClusterPipeline (see #2914).""" assert "EVALSHA" not in PIPELINE_BLOCKED_COMMANDS +@pytest.mark.onlycluster def test_evalsha_can_be_queued_on_cluster_pipeline(): """ Queuing EVALSHA on a ClusterPipeline must not raise the historical @@ -3863,6 +3866,7 @@ def test_evalsha_can_be_queued_on_cluster_pipeline(): r.close() +@pytest.mark.onlycluster def test_evalsha_zero_keys_pipeline_execute_skips_get_command_keys(): """ Sync ClusterPipeline must route EVALSHA via determine_slot, not @@ -3886,6 +3890,7 @@ def test_evalsha_zero_keys_pipeline_execute_skips_get_command_keys(): r.close() +@pytest.mark.onlycluster def test_evalsha_zero_keys_reuses_slot_in_transaction(): """ Zero-key EVALSHA in a transactional pipeline must reuse one slot so @@ -3910,6 +3915,7 @@ def test_evalsha_zero_keys_reuses_slot_in_transaction(): r.close() +@pytest.mark.onlycluster def test_evalsha_zero_keys_follows_keyed_slot_in_transaction(): """Zero-key EVALSHA after a keyed slot is fixed reuses that slot.""" r = get_mocked_redis_client(host=default_host, port=default_port) @@ -3925,6 +3931,7 @@ def test_evalsha_zero_keys_follows_keyed_slot_in_transaction(): r.close() +@pytest.mark.onlycluster def test_slotless_command_does_not_lock_keyed_slot_flag(): """Slotless commands must not block later keyed retargeting.""" r = get_mocked_redis_client(host=default_host, port=default_port) @@ -3947,21 +3954,34 @@ def test_slotless_command_does_not_lock_keyed_slot_flag(): r.close() -def test_zero_key_fcall_allows_keyed_retarget(): - """Zero-key FCALL must not lock the slot so a later keyed command can retarget.""" +@pytest.mark.onlycluster +def test_evalsha_zero_keys_then_cross_slot_raises_at_execute(): + """ + Zero-key EVALSHA retarget must not swallow a later true cross-slot + conflict: two keyed commands on different slots still raise at execute(). + """ r = get_mocked_redis_client(host=default_host, port=default_port) try: + sha = "a" * 40 + slot_a = key_slot(b"{foo}a") + slot_b = key_slot(b"{bar}b") + assert slot_a != slot_b with r.pipeline(transaction=True) as pipe: - with patch.object(pipe, "determine_slot", return_value=111): - pipe.execute_command("FCALL", "myfunc", 0) - assert pipe._execution_strategy._pipeline_slots == {111} - assert pipe._execution_strategy._transaction_has_keyed_slot is False - - keyed_slot = key_slot(b"foo") - with patch.object(pipe, "determine_slot", return_value=keyed_slot): - pipe.set("foo", "bar") - assert pipe._execution_strategy._pipeline_slots == {keyed_slot} - assert pipe._execution_strategy._transaction_has_keyed_slot is True + with patch.object( + pipe, "determine_slot", side_effect=[111, slot_a, slot_b] + ): + pipe.evalsha(sha, 0) + pipe.set("{foo}a", "1") + pipe.set("{bar}b", "2") + assert pipe._execution_strategy._pipeline_slots == {slot_a, slot_b} + with pytest.raises( + CrossSlotTransactionError, + match=( + "All keys involved in a cluster transaction " + "must map to the same slot" + ), + ): + pipe.execute() finally: r.close() From da63b94b6fff03b4bc71c9cb60554a83cf438a93 Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:49:26 -0500 Subject: [PATCH 09/10] docs: warn that NOSCRIPT pipeline retry can duplicate side effects --- docs/lua_scripting.rst | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/lua_scripting.rst b/docs/lua_scripting.rst index 4f0bbdaa40..4b4de9e153 100644 --- a/docs/lua_scripting.rst +++ b/docs/lua_scripting.rst @@ -134,8 +134,12 @@ Important caveats when using ``EVALSHA`` in a cluster pipeline: fails with ``NOSCRIPT`` (``redis.exceptions.NoScriptError``). - Unlike the non-pipelined ``Script`` object, **a cluster pipeline performs no automatic reload or retry** on ``NOSCRIPT``. Recovery is the caller's - responsibility: catch ``NoScriptError``, re-run ``SCRIPT LOAD``, and - re-execute the pipeline. Because zero-key ``EVALSHA`` is routed to a random + responsibility: catch ``NoScriptError``, re-run ``SCRIPT LOAD``, then retry + only when replay is safe (for example an idempotent or single-command + pipeline). Blindly re-executing a multi-command pipeline can duplicate + side effects: Redis still runs the rest of a non-transactional batch when + one ``EVALSHA`` returns ``NOSCRIPT``, and the client raises only after + reading every response. Because zero-key ``EVALSHA`` is routed to a random primary, the script must be present on **all** primaries. - In a **transactional** pipeline, a ``NOSCRIPT`` from ``EVALSHA`` is raised at ``EXEC`` time and does **not** roll back the other commands (this follows @@ -149,9 +153,9 @@ Important caveats when using ``EVALSHA`` in a cluster pipeline: ... with rc.pipeline() as pipe: ... pipe.evalsha(sha, 1, "{user}:1") ... return pipe.execute() - >>> try: - ... result = run() - ... except NoScriptError: - ... # a node was missing the script (e.g. after failover/upgrade) - ... sha = rc.script_load(lua) # reload on current primaries - ... result = run() +>>> try: +... result = run() +... except NoScriptError: +... # single-command pipeline: safe to reload and retry +... sha = rc.script_load(lua) # reload on current primaries +... result = run() From 1cf8313f74b9da61d3b2d6d35b1e38627cb7bb34 Mon Sep 17 00:00:00 2001 From: Ahmed Mohamed <68402624+ahmed5145@users.noreply.github.com> Date: Wed, 5 Aug 2026 13:15:05 -0500 Subject: [PATCH 10/10] docs: fix indentation of NOSCRIPT recovery example The last six lines of the recovery example started at column 0, so Sphinx ended the code:: python block early and rendered the example as two disconnected chunks. Also add resharding and idempotent to the spellcheck wordlist; they are the only words in the new prose that aspell does not know. --- .github/wordlist.txt | 2 ++ docs/lua_scripting.rst | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/wordlist.txt b/.github/wordlist.txt index c66c9310e6..b72c49fa1b 100644 --- a/.github/wordlist.txt +++ b/.github/wordlist.txt @@ -132,6 +132,7 @@ gmail himport hiredis http +idempotent idx iff ini @@ -182,6 +183,7 @@ redismodules reinitialization replicaof repo +resharding runtime sedrik serializers diff --git a/docs/lua_scripting.rst b/docs/lua_scripting.rst index 4b4de9e153..9f6d96352f 100644 --- a/docs/lua_scripting.rst +++ b/docs/lua_scripting.rst @@ -153,9 +153,9 @@ Important caveats when using ``EVALSHA`` in a cluster pipeline: ... with rc.pipeline() as pipe: ... pipe.evalsha(sha, 1, "{user}:1") ... return pipe.execute() ->>> try: -... result = run() -... except NoScriptError: -... # single-command pipeline: safe to reload and retry -... sha = rc.script_load(lua) # reload on current primaries -... result = run() + >>> try: + ... result = run() + ... except NoScriptError: + ... # single-command pipeline: safe to reload and retry + ... sha = rc.script_load(lua) # reload on current primaries + ... result = run()