fix: allow EVALSHA in ClusterPipeline (#2914)

[ahmed5145]

Description of change

Fixes #2914

EVALSHA was listed in PIPELINE_BLOCKED_COMMANDS, so ClusterPipeline.evalsha() always raised even though RedisCluster.determine_slot() already routes EVAL/EVALSHA by keys (including the 0-key random-slot case).

This removes EVALSHA from the blocked list so cluster pipelines can queue EVALSHA like other keyed commands. Callers must still:

• load the script on the target node(s) first (e.g. cluster SCRIPT LOAD, which loads on all primaries)
• keep all script keys on the same hash slot

Also updates docs/lua_scripting.rst and adds regression coverage:

• mock tests that EVALSHA is no longer blocked and can be queued on ClusterPipeline
• a cluster integration test (test_evalsha_in_pipeline) for CI with a live cluster

Pull Request check-list

• Do tests and lints pass with this change?
• Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
• Is the new or changed code fully tested?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Is there an example added to the examples folder (if applicable)?

---

Note

Medium Risk
Changes cluster pipeline routing and MULTI/EXEC slot logic for scripts; misuse can still cause cross-slot errors or partial pipeline effects on NOSCRIPT, but behavior is covered by extensive new tests.

Overview
Cluster pipelines can queue EVALSHA again (#2914) by removing it from PIPELINE_BLOCKED_COMMANDS, with routing still driven by determine_slot / keyed policies instead of COMMAND GETKEYS (which breaks on Redis <7 when numkeys is 0).

Transactional pipelines gain _resolve_transaction_slot (sync and async): zero-key EVAL/EVALSHA reuse an existing transaction slot or pick one at random; keyed commands can retarget when prior slots came only from zero-key scripts; true cross-slot keyed mixes still raise CrossSlotTransactionError at execute.

Async Script on ClusterPipeline calls evalsha synchronously so awaiting the pipeline does not run initialize() and drop the queued command.

Docs (lua_scripting.rst) describe cluster-pipeline EVALSHA usage, limits (load_scripts / sync eval still blocked), and caller-owned NOSCRIPT recovery. Tests cover blocking removal, zero-key execution, transaction slot behavior, and integration runs.

^{Reviewed by Cursor Bugbot for commit 1cf8313. Bugbot is set up for automated code reviews on this repo. Configure here.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3bbd5d830f

ℹ️ 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".

[petyaslavova]

Request: make the operational contract explicit in the docs

The routing fix is correct, but the current note understates the operational risk. EVALSHA in a ClusterPipeline has a materially weaker safety story than non-pipelined EVALSHA: the single-node self-healing already documented in this file ("In the event of a NOSCRIPT error, it will load the script and retry executing it") is not available in a cluster pipeline — the Script/register_script fallback and EVAL are both disabled there — and the Lua script cache is per-node and not replicated, so any failover, rolling upgrade, resharding, or newly added shard can route the command to a node without the script and raise NOSCRIPT with no recovery. The referenced ioredis fix (PR #1201) mitigates this with an automatic pre-SCRIPT LOAD; this PR does not, so the caller must own the reload.

Please replace the final line of docs/lua_scripting.rst (Using scripting within pipelines in cluster mode is **not supported**.) with the following, which matches the file's conventions (plain prose + `` literals, no cross-ref roles, .. code:: python, bold for emphasis):

``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()

[ahmed5145]

I updated docs/lua_scripting.rst with the explicit operational contract for EVALSHA in ClusterPipeline (per-node cache, no pipeline reload/retry, transactional NOSCRIPT semantics, and a SCRIPT LOAD retry example).

Please lmk if you have any other feedback!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b23b5eace1

ℹ️ 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".

[petyaslavova]

Thanks for turning this around so quickly!

Few more things before we can merge:

1. Please add a regression test (sync and async) that a zero-key EVALSHA followed by two keyed commands on different slots still raises CrossSlotTransactionError at execute(). The new _pipeline_slots.clear() retarget is the only thing standing between that case and a silent wrong-node execution, so the invariant needs to be pinned down.
2. The async side has no live-cluster coverage. Please mirror the three onlycluster tests you added to TestClusterPipeline in tests/test_cluster.py (keyed, zero-key, and zero-key transactional) under tests/test_asyncio/. Sync/async test parity is a repository requirement.
3. Including FCALL/FCALL_RO in is_zero_key_eval_command changes existing behavior: FCALL was never in PIPELINE_BLOCKED_COMMANDS, so a zero-key FCALL combined with a keyed command in a transactional pipeline used to raise CrossSlotTransactionError and now succeeds. That is probably the behavior we want, but it is outside #2914's scope and is currently undocumented. Please either narrow the helper to EVAL/EVALSHA or call the FCALL change out in the PR description and in docs/lua_scripting.rst.

Few non-blocking notes: the sentence stating that EVAL remains unsupported on cluster pipelines holds for the sync client (ClusterPipeline.eval() raises) but not for the async one, which has no eval override; and the new sync mock tests are the only module-level test functions in tests/test_cluster.py and carry no topology marker, so they also run in the standalone suite, whereas the async ones are inside an onlycluster class.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b1196b7b3

ℹ️ 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".

[petyaslavova]

Thanks for turning this around — all three blocking items from the last round are covered: the cross-slot regression test on both stacks, the async live-cluster mirrors, and narrowing the helper back to EVAL/EVALSHA.

One blocker left before merge: in docs/lua_scripting.rst the >>> try: / ... except NoScriptError: lines of the recovery example start at column 0, so they fall outside the .. code:: python block and Sphinx renders the example as two disconnected chunks. Please indent those six lines by three spaces to match the lines above them. Docs CI does not build with -W, so it will not catch this for us.

Two optional follow-ups, not merge blockers: the async transaction strategy does not release a connection owned by the previous node when the new retarget changes the transaction slot (the sync path does, redis/cluster.py _get_client_and_connection_for_transaction), and a sync counterpart to test_async_script_queues_evalsha_on_cluster_pipeline would complete the parity pair. Happy to take either as a separate issue.

[ahmed5145]

Thanks! I fixed the indentation. The six >>> try: / ... except NoScriptError: lines now sit at three spaces, matching the rest of the block, so the whole example renders as one code:: python block.

I also fixed the check-spelling CI failure while I was in there. docs/lua_scripting.rst is the only spellcheck-scanned file this PR touches, and resharding and idempotent were the only new prose words aspell didn't already know, so I added them to .github/wordlist.txt rather than rewording the text you'd already reviewed.

On the Validate building and installing the package (tar.gz) failure: I don't think it's related to this PR. The whl job in the same matrix runs the identical suites via install_and_test.sh and passed in 21 minutes, while tar.gz ran 82 minutes before failing; same code, same tests, so it looks like the docker/test environment hung rather than a real regression. Could you re-run it if it doesn't clear on the new commit?

On the two optional follow-ups, the async transaction connection release and the sync counterpart to test_async_script_queues_evalsha_on_cluster_pipeline, I agree they're worth doing, and I'd be happy to pick them up, but I'd prefer to keep them out of this PR. Could you open them as a separate issue (or two)? Happy to take them from there.

[cursor]

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

^{Reviewed by Cursor Bugbot for commit 1cf8313. Configure here.}