Skip to content

Commit 524edc7

Browse files
committed
perf: remove proactive module validation to reduce network calls (#370)
Remove MODULE LIST calls during connection initialization and index creation. Operations now fail naturally when Redis modules are missing, providing clear error messages at the point of failure. Changes: - Remove validate_modules() function and DEFAULT_REQUIRED_MODULES constant - Remove required_modules parameter from all connection methods - Remove proactive validation from SearchIndex and AsyncSearchIndex - Remove validation from SemanticRouter.from_existing() - Fix SemanticCache to use lazy client initialization - Add comprehensive TDD tests verifying no MODULE LIST calls - Remove skip_if_module_version_error test helper - Remove unused RedisModuleVersionError exception This change improves performance by eliminating unnecessary network roundtrips and reduces latency during connection initialization, especially beneficial for Redis Cluster deployments.
1 parent 547085a commit 524edc7

File tree

14 files changed

+498
-249
lines changed

14 files changed

+498
-249
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,3 +230,4 @@ tests/data
230230
.cursor
231231
.junie
232232
.undodir
233+
.claude/settings.local.json

redisvl/exceptions.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,6 @@ class RedisVLError(Exception):
1111
pass
1212

1313

14-
class RedisModuleVersionError(RedisVLError):
15-
"""Error raised when required Redis modules are missing or have incompatible versions."""
16-
17-
pass
18-
19-
2014
class RedisSearchError(RedisVLError):
2115
"""Error raised for Redis Search specific operations."""
2216

redisvl/extensions/cache/llm/semantic.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def __init__(
162162
)
163163

164164
existing_index = SearchIndex.from_existing(
165-
name, redis_client=self._index.client
165+
name, redis_client=self._index._redis_client
166166
)
167167
if existing_index.schema.to_dict() != self._index.schema.to_dict():
168168
raise ValueError(

redisvl/extensions/router/semantic.py

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from redis.commands.search.aggregation import AggregateRequest, AggregateResult, Reducer
88
from redis.exceptions import ResponseError
99

10-
from redisvl.exceptions import RedisModuleVersionError
1110
from redisvl.extensions.constants import ROUTE_VECTOR_FIELD_NAME
1211
from redisvl.extensions.router.schema import (
1312
DistanceAggregationMethod,
@@ -118,18 +117,14 @@ def from_existing(
118117
**kwargs,
119118
) -> "SemanticRouter":
120119
"""Return SemanticRouter instance from existing index."""
121-
try:
122-
if redis_url:
123-
redis_client = RedisConnectionFactory.get_redis_connection(
124-
redis_url=redis_url,
125-
**kwargs,
126-
)
127-
elif redis_client:
128-
RedisConnectionFactory.validate_sync_redis(redis_client)
129-
except RedisModuleVersionError as e:
130-
raise RedisModuleVersionError(
131-
f"Loading from existing index failed. {str(e)}"
120+
if redis_url:
121+
redis_client = RedisConnectionFactory.get_redis_connection(
122+
redis_url=redis_url,
123+
**kwargs,
132124
)
125+
elif redis_client:
126+
# Just validate client type and set lib name
127+
RedisConnectionFactory.validate_sync_redis(redis_client)
133128
if redis_client is None:
134129
raise ValueError(
135130
"Creating Redis client failed. Please check the redis_url and connection_kwargs."

redisvl/index/index.py

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@
6666

6767
from redisvl.exceptions import (
6868
QueryValidationError,
69-
RedisModuleVersionError,
7069
RedisSearchError,
7170
RedisVLError,
7271
SchemaValidationError,
@@ -438,23 +437,15 @@ def from_existing(
438437
439438
Raises:
440439
ValueError: If redis_url or redis_client is not provided.
441-
RedisModuleVersionError: If required Redis modules are not installed.
442440
"""
443-
try:
444-
if redis_url:
445-
redis_client = RedisConnectionFactory.get_redis_connection(
446-
redis_url=redis_url,
447-
required_modules=REQUIRED_MODULES_FOR_INTROSPECTION,
448-
**kwargs,
449-
)
450-
elif redis_client:
451-
RedisConnectionFactory.validate_sync_redis(
452-
redis_client, required_modules=REQUIRED_MODULES_FOR_INTROSPECTION
453-
)
454-
except RedisModuleVersionError as e:
455-
raise RedisModuleVersionError(
456-
f"Loading from existing index failed. {str(e)}"
441+
if redis_url:
442+
redis_client = RedisConnectionFactory.get_redis_connection(
443+
redis_url=redis_url,
444+
**kwargs,
457445
)
446+
elif redis_client:
447+
# Validate client type and set lib name
448+
RedisConnectionFactory.validate_sync_redis(redis_client)
458449

459450
if not redis_client:
460451
raise ValueError("Must provide either a redis_url or redis_client")
@@ -480,11 +471,16 @@ def _redis_client(self) -> SyncRedisClient:
480471
if self.__redis_client is None:
481472
with self._lock:
482473
if self.__redis_client is None:
474+
# Pass lib_name to connection factory
475+
kwargs = {**self._connection_kwargs}
476+
if self._lib_name:
477+
kwargs["lib_name"] = self._lib_name
483478
self.__redis_client = RedisConnectionFactory.get_redis_connection(
484479
redis_url=self._redis_url,
485-
**self._connection_kwargs,
480+
**kwargs,
486481
)
487-
if not self._validated_client:
482+
if not self._validated_client and self._lib_name:
483+
# Only set lib name for user-provided clients
488484
RedisConnectionFactory.validate_sync_redis(
489485
self.__redis_client,
490486
self._lib_name,
@@ -1192,21 +1188,14 @@ async def from_existing(
11921188
"Must provide either a redis_url or redis_client to fetch Redis index info."
11931189
)
11941190

1195-
try:
1196-
if redis_url:
1197-
redis_client = await RedisConnectionFactory._get_aredis_connection(
1198-
url=redis_url,
1199-
required_modules=REQUIRED_MODULES_FOR_INTROSPECTION,
1200-
**kwargs,
1201-
)
1202-
elif redis_client:
1203-
await RedisConnectionFactory.validate_async_redis(
1204-
redis_client, required_modules=REQUIRED_MODULES_FOR_INTROSPECTION
1205-
)
1206-
except RedisModuleVersionError as e:
1207-
raise RedisModuleVersionError(
1208-
f"Loading from existing index failed. {str(e)}"
1209-
) from e
1191+
if redis_url:
1192+
redis_client = await RedisConnectionFactory._get_aredis_connection(
1193+
url=redis_url,
1194+
**kwargs,
1195+
)
1196+
elif redis_client:
1197+
# Validate client type and set lib name
1198+
await RedisConnectionFactory.validate_async_redis(redis_client)
12101199

12111200
if redis_client is None:
12121201
raise ValueError(
@@ -1255,13 +1244,17 @@ async def _get_client(self) -> AsyncRedisClient:
12551244
async with self._lock:
12561245
# Double-check to protect against concurrent access
12571246
if self._redis_client is None:
1258-
kwargs = self._connection_kwargs
1247+
# Pass lib_name to connection factory
1248+
kwargs = {**self._connection_kwargs}
12591249
if self._redis_url:
12601250
kwargs["url"] = self._redis_url
1251+
if self._lib_name:
1252+
kwargs["lib_name"] = self._lib_name
12611253
self._redis_client = (
12621254
await RedisConnectionFactory._get_aredis_connection(**kwargs)
12631255
)
1264-
if not self._validated_client:
1256+
if not self._validated_client and self._lib_name:
1257+
# Set lib name for user-provided clients
12651258
await RedisConnectionFactory.validate_async_redis(
12661259
self._redis_client,
12671260
self._lib_name,

redisvl/redis/connection.py

Lines changed: 31 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
from redis.exceptions import ResponseError
1414

1515
from redisvl import __version__
16-
from redisvl.exceptions import RedisModuleVersionError
17-
from redisvl.redis.constants import DEFAULT_REQUIRED_MODULES, REDIS_URL_ENV_VAR
16+
from redisvl.redis.constants import REDIS_URL_ENV_VAR
1817
from redisvl.redis.utils import convert_bytes, is_cluster_url
1918
from redisvl.types import AsyncRedisClient, RedisClient, SyncRedisClient
2019
from redisvl.utils.utils import deprecated_function
@@ -155,40 +154,6 @@ def parse_attrs(attrs):
155154
}
156155

157156

158-
def validate_modules(
159-
installed_modules: Dict[str, Any],
160-
required_modules: Optional[List[Dict[str, Any]]] = None,
161-
) -> None:
162-
"""
163-
Validates if required Redis modules are installed.
164-
165-
Args:
166-
installed_modules: List of installed modules.
167-
required_modules: List of required modules.
168-
169-
Raises:
170-
RedisModuleVersionError: If required Redis modules are not installed.
171-
"""
172-
required_modules = required_modules or DEFAULT_REQUIRED_MODULES
173-
174-
for required_module in required_modules:
175-
if required_module["name"] in installed_modules:
176-
installed_version = installed_modules[required_module["name"]] # type: ignore
177-
if int(installed_version) >= int(required_module["ver"]): # type: ignore
178-
return
179-
180-
# Build the error message dynamically
181-
required_modules_str = " OR ".join(
182-
[f'{module["name"]} >= {module["ver"]}' for module in required_modules]
183-
)
184-
error_message = (
185-
f"Required Redis db module {required_modules_str} not installed. "
186-
"See Redis Stack docs at https://redis.io/docs/latest/operate/oss_and_stack/install/install-stack/."
187-
)
188-
189-
raise RedisModuleVersionError(error_message)
190-
191-
192157
class RedisConnectionFactory:
193158
"""Builds connections to a Redis database, supporting both synchronous and
194159
asynchronous clients.
@@ -232,16 +197,13 @@ def connect(
232197
@staticmethod
233198
def get_redis_connection(
234199
redis_url: Optional[str] = None,
235-
required_modules: Optional[List[Dict[str, Any]]] = None,
236200
**kwargs,
237201
) -> SyncRedisClient:
238202
"""Creates and returns a synchronous Redis client.
239203
240204
Args:
241205
url (Optional[str]): The URL of the Redis server. If not provided,
242206
the environment variable REDIS_URL is used.
243-
required_modules (Optional[List[Dict[str, Any]]]): List of required
244-
Redis modules with version requirements.
245207
**kwargs: Additional keyword arguments to be passed to the Redis
246208
client constructor.
247209
@@ -251,22 +213,26 @@ def get_redis_connection(
251213
Raises:
252214
ValueError: If url is not provided and REDIS_URL environment
253215
variable is not set.
254-
RedisModuleVersionError: If required Redis modules are not installed.
255216
"""
256217
url = redis_url or get_address_from_env()
257218
if is_cluster_url(url, **kwargs):
258219
client = RedisCluster.from_url(url, **kwargs)
259220
else:
260221
client = Redis.from_url(url, **kwargs)
261-
RedisConnectionFactory.validate_sync_redis(
262-
client, required_modules=required_modules
263-
)
222+
# Module validation removed - operations will fail naturally if modules are missing
223+
# Set client library name only
224+
_lib_name = make_lib_name(kwargs.get("lib_name"))
225+
try:
226+
client.client_setinfo("LIB-NAME", _lib_name)
227+
except ResponseError:
228+
# Fall back to a simple log echo
229+
if hasattr(client, "echo"):
230+
client.echo(_lib_name)
264231
return client
265232

266233
@staticmethod
267234
async def _get_aredis_connection(
268235
url: Optional[str] = None,
269-
required_modules: Optional[List[Dict[str, Any]]] = None,
270236
**kwargs,
271237
) -> AsyncRedisClient:
272238
"""Creates and returns an asynchronous Redis client.
@@ -277,8 +243,6 @@ async def _get_aredis_connection(
277243
Args:
278244
url (Optional[str]): The URL of the Redis server. If not provided,
279245
the environment variable REDIS_URL is used.
280-
required_modules (Optional[List[Dict[str, Any]]]): List of required
281-
Redis modules with version requirements.
282246
**kwargs: Additional keyword arguments to be passed to the async
283247
Redis client constructor.
284248
@@ -288,7 +252,6 @@ async def _get_aredis_connection(
288252
Raises:
289253
ValueError: If url is not provided and REDIS_URL environment
290254
variable is not set.
291-
RedisModuleVersionError: If required Redis modules are not installed.
292255
"""
293256
url = url or get_address_from_env()
294257

@@ -297,9 +260,15 @@ async def _get_aredis_connection(
297260
else:
298261
client = AsyncRedis.from_url(url, **kwargs)
299262

300-
await RedisConnectionFactory.validate_async_redis(
301-
client, required_modules=required_modules
302-
)
263+
# Module validation removed - operations will fail naturally if modules are missing
264+
# Set client library name only
265+
_lib_name = make_lib_name(kwargs.get("lib_name"))
266+
try:
267+
await client.client_setinfo("LIB-NAME", _lib_name)
268+
except ResponseError:
269+
# Fall back to a simple log echo
270+
if hasattr(client, "echo"):
271+
await client.echo(_lib_name)
303272
return client
304273

305274
@staticmethod
@@ -386,9 +355,12 @@ async def get_modules_async(client: AsyncRedisClient) -> Dict[str, Any]:
386355
def validate_sync_redis(
387356
redis_client: SyncRedisClient,
388357
lib_name: Optional[str] = None,
389-
required_modules: Optional[List[Dict[str, Any]]] = None,
390358
) -> None:
391-
"""Validates the sync Redis client."""
359+
"""Validates the sync Redis client.
360+
361+
Note: Module validation has been removed. This method now only validates
362+
the client type and sets the library name.
363+
"""
392364
if not issubclass(type(redis_client), (Redis, RedisCluster)):
393365
raise TypeError(
394366
"Invalid Redis client instance. Must be Redis or RedisCluster."
@@ -404,19 +376,18 @@ def validate_sync_redis(
404376
if hasattr(redis_client, "echo"):
405377
redis_client.echo(_lib_name)
406378

407-
# Get list of modules
408-
installed_modules = RedisConnectionFactory.get_modules(redis_client)
409-
410-
# Validate available modules
411-
validate_modules(installed_modules, required_modules)
379+
# Module validation removed - operations will fail naturally if modules are missing
412380

413381
@staticmethod
414382
async def validate_async_redis(
415383
redis_client: AsyncRedisClient,
416384
lib_name: Optional[str] = None,
417-
required_modules: Optional[List[Dict[str, Any]]] = None,
418385
) -> None:
419-
"""Validates the async Redis client."""
386+
"""Validates the async Redis client.
387+
388+
Note: Module validation has been removed. This method now only validates
389+
the client type and sets the library name.
390+
"""
420391
if not issubclass(type(redis_client), (AsyncRedis, AsyncRedisCluster)):
421392
raise TypeError(
422393
"Invalid async Redis client instance. Must be async Redis or async RedisCluster."
@@ -427,12 +398,7 @@ async def validate_async_redis(
427398
await redis_client.client_setinfo("LIB-NAME", _lib_name)
428399
except ResponseError:
429400
# Fall back to a simple log echo
430-
await redis_client.echo(_lib_name)
431401
if hasattr(redis_client, "echo"):
432402
await redis_client.echo(_lib_name)
433403

434-
# Get list of modules
435-
installed_modules = await RedisConnectionFactory.get_modules_async(redis_client)
436-
437-
# Validate available modules
438-
validate_modules(installed_modules, required_modules)
404+
# Module validation removed - operations will fail naturally if modules are missing

0 commit comments

Comments
 (0)