Repository navigation
fix(index): read cluster FT.INFO on RESP3 and initialise fresh async cluster clients - #795
Draft
vishal-bala wants to merge 6 commits into
Draft
vishal-bala wants to merge 6 commits into
vishal-bala wants to merge 6 commits into
Conversation
…cluster clients The cluster branch of SearchIndex._info() and AsyncSearchIndex._info() sends a raw FT.INFO to one node, so redis-py's search parser never sees the reply. On RESP3, the redis-py 8 default, that reply is a map, and make_dict() raised KeyError: 1. Accept either shape. The async cluster client discovers its nodes on its first command, so on a fresh client get_random_node() raised IndexError and get_default_node() returned None. That broke create(), and delete() then failed with "'NoneType' object has no attribute 'connection_kwargs'". Initialise the client in _get_client(), and in _info() for callers that bypass it.
vishal-bala
changed the base branch from
main
to
test/cluster-pipelining-client
October 30, 2026 10:57
_info() sent FT.INFO to get_random_node(), which picks from every node in the cluster, replicas included. FT.CREATE goes to the default node, a primary, and Redis replicates asynchronously, so a replica can answer "no such index" straight after create(). The nightly cluster job hit this on redis-py 8.1.0, with FT.INFO landing on the replica at port 7005. Ask the default node instead, the node FT.CREATE, FT.DROPINDEX and FT.SEARCH already use. Async listall() had the same random-node read and now does the same; sync listall() already reached the default node through redis-py's keyless routing.
The previous commit said sync listall() already reached the default node
through redis-py's keyless routing. That holds only on redis-py 6.x. On 7.x
and 8.x, FT._LIST has no routing entry, so a keyless call on a cluster
raises RedisClusterException ("No way to dispatch this command to Redis
Cluster. Missing key"). Target the default node explicitly, as the async
twin and _info() do.
vishal-bala
added a commit
that referenced
this pull request
Oct 30, 2026
Co-Authored-By: Claude <noreply@anthropic.com>
…nfo-resp3-async-init Co-Authored-By: Claude <noreply@anthropic.com>
This branch has not been deployed
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
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.
Motivation
When the
requires_clustersuite was run against a real six-node cluster (Redis 8.4, redis-py 8.1.0), three tests failed because of two bugs in the cluster branch of_info(). CI never runs these tests, so neither bug was caught:test_redis_cluster_support.py::test_search_index_cluster_infofails withKeyError: 1.test_async_search_index_cluster_infoandtest_async_search_index_clientfail withIndexError: Cannot choose from an empty sequence, and the cleanup then fails with'NoneType' object has no attribute 'connection_kwargs'.Anyone who passes a freshly built
AsyncRedisClustertoAsyncSearchIndexhits the second bug oncreate(). The first bug hits any sync cluster user on redis-py 8, which uses RESP3 by default.Changes
RESP3 replies to the cluster
FT.INFOOn a cluster,
SearchIndex._info()andAsyncSearchIndex._info()send a rawFT.INFOto a single node, so redis-py's search parser never sees the reply. That reply is a flat list on RESP2 and a map on RESP3, andmake_dict()only accepted the list. Both versions of_info()now accept either shape. The nested sections stay maps on RESP3, and every internal consumer already handles them:convert_index_info_to_schema, the MCP schema loader, migration and the CLI.Fresh async cluster clients
redis-py's async cluster client finds out about its nodes on its first command. Until then,
get_random_node()raisesIndexErrorandget_default_node()returnsNone.AsyncSearchIndex._get_client()now callsinitialize()on cluster clients; when the client is already initialised, that call does nothing. This also fixesdelete(),listall()andsearch()when one of them is the first call._info()initialises the client as well, becausefrom_existing()and the MCP server call it directly without going through_get_client().Index metadata is read from the default node
_info()used to sendFT.INFOtoget_random_node(), which can return a replica.FT.CREATE,FT.DROPINDEXandFT.SEARCHall go to the default node, a primary. Redis replicates asynchronously, so a replica can answer "no such index" straight aftercreate(). That is how the nightly cluster job failed on redis-py 8.1.0. Sync and async_info()now ask the default node. Bothlistall()variants do the same: the async one used a random node too, and on redis-py 7.x and 8.x the sync one raisedRedisClusterExceptionon a cluster, because redis-py has no routing for a keylessFT._LIST.Tests
tests/unit/test_cluster_index_info.pyruns in default CI. Its stand-in clients are built withcreate_autospecand raiseIndexErroruntilinitialize()has been called. The tests feed both RESP2 and RESP3 replies. Two existing mocks needed an awaitableinitialize.Notes
_get_client()runs outside each method'stry, so if an injectedAsyncRedisClustercan't reach its startup node,exists()and the other async methods raise redis-py's ownConnectionError, notRedisSearchError. This follows the existing convention: only the Redis command is wrapped, never the step that gets a client. A client built from a URL already fails the same way, because it sendsCLIENT SETINFOwhen it connects.Reviewer Brief
Start here:
SearchIndex._info()andAsyncSearchIndex._info()inredisvl/index/index.py. Every cluster read of index metadata now goes through one shape-agnosticFT.INFOsent to the default node.exists(),info(),create()andfrom_existing()all depend on it.Where it could fail
AsyncSearchIndex._get_client(): it now callsinitialize()on every injectedAsyncRedisCluster. Check that nothing depends on a cluster client staying uninitialised until its first command._info()on a cluster: on RESP3, which redis-py 8 uses by default,info()returnsattributesandindex_definitionas maps. A standalone client in redis-py 8's default legacy mode returns them as flat lists. Every internal consumer accepts both shapes, but an external caller ofinfo()might rely on the list shape.Not verified: the Linux nightly job after these commits, since it hasn't been re-dispatched. The replica lag behind the CI failure didn't reproduce locally (0 misses in 900
FT.INFOcalls to replicas straight afterFT.CREATE), because all six local nodes share one container. I also haven't checked Redis versions other than 8.4, or Python versions other than 3.13 and 3.14.Beyond CI:
test_redis_cluster_support.py,test_cluster_pipelining.pyandtest_async_cluster_connection.pygave 24 passed on each of redis-py 8.1.0, 7.4.1 and 6.4.0. That ran on310af379, before the synclistall()change. The router test errored each time becausesentence-transformersisn't installed.listall(): on a live cluster it raisedRedisClusterExceptionon redis-py 8.1.0 and 7.4.1 before62164c00, and returns normally after it on all three versions.protocol=2, withprotocol=3and with the default. It coversinfo(), afrom_existing()round trip, and fresh async clients wherelistall(),create()ordelete()is the first command. It passed on all three redis-py versions, on8e075faf.index.pyfrom before310af379, 7 of the new or changed unit tests fail. The synclistall()unit test wasn't run against the old code.Reviewed
8e075fafand310af379, fixes inb2626140and62164c00· Claude Code (claude-opus-5-5) · lenses: correctness, security, design, tests, docs, Redis behaviour (310af379: correctness, Redis behaviour)