Skip to content

Commit 4e546cc

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 b32ed95 commit 4e546cc

File tree

13 files changed

+497
-249
lines changed

13 files changed

+497
-249
lines changed

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
@@ -67,7 +67,6 @@
6767

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

460451
if not redis_client:
461452
raise ValueError("Must provide either a redis_url or redis_client")
@@ -481,11 +472,16 @@ def _redis_client(self) -> SyncRedisClient:
481472
if self.__redis_client is None:
482473
with self._lock:
483474
if self.__redis_client is None:
475+
# Pass lib_name to connection factory
476+
kwargs = {**self._connection_kwargs}
477+
if self._lib_name:
478+
kwargs["lib_name"] = self._lib_name
484479
self.__redis_client = RedisConnectionFactory.get_redis_connection(
485480
redis_url=self._redis_url,
486-
**self._connection_kwargs,
481+
**kwargs,
487482
)
488-
if not self._validated_client:
483+
if not self._validated_client and self._lib_name:
484+
# Only set lib name for user-provided clients
489485
RedisConnectionFactory.validate_sync_redis(
490486
self.__redis_client,
491487
self._lib_name,
@@ -1193,21 +1189,14 @@ async def from_existing(
11931189
"Must provide either a redis_url or redis_client to fetch Redis index info."
11941190
)
11951191

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

12121201
if redis_client is None:
12131202
raise ValueError(
@@ -1256,13 +1245,17 @@ async def _get_client(self) -> AsyncRedisClient:
12561245
async with self._lock:
12571246
# Double-check to protect against concurrent access
12581247
if self._redis_client is None:
1259-
kwargs = self._connection_kwargs
1248+
# Pass lib_name to connection factory
1249+
kwargs = {**self._connection_kwargs}
12601250
if self._redis_url:
12611251
kwargs["url"] = self._redis_url
1252+
if self._lib_name:
1253+
kwargs["lib_name"] = self._lib_name
12621254
self._redis_client = (
12631255
await RedisConnectionFactory._get_aredis_connection(**kwargs)
12641256
)
1265-
if not self._validated_client:
1257+
if not self._validated_client and self._lib_name:
1258+
# Set lib name for user-provided clients
12661259
await RedisConnectionFactory.validate_async_redis(
12671260
self._redis_client,
12681261
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

tests/conftest.py

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import pytest
77
from testcontainers.compose import DockerCompose
88

9-
from redisvl.exceptions import RedisModuleVersionError
109
from redisvl.index.index import AsyncSearchIndex, SearchIndex
1110
from redisvl.redis.connection import RedisConnectionFactory, compare_versions
1211
from redisvl.redis.utils import array_to_buffer
@@ -610,33 +609,3 @@ async def skip_if_redis_version_below_async(
610609
if not compare_versions(redis_version, min_version):
611610
skip_msg = message or f"Redis version {redis_version} < {min_version} required"
612611
pytest.skip(skip_msg)
613-
614-
615-
def skip_if_module_version_error(func, *args, **kwargs):
616-
"""
617-
Execute function and skip test if RedisModuleVersionError is raised.
618-
619-
Args:
620-
func: Function to execute
621-
*args: Arguments for the function
622-
**kwargs: Keyword arguments for the function
623-
"""
624-
try:
625-
return func(*args, **kwargs)
626-
except RedisModuleVersionError:
627-
pytest.skip("Required Redis modules not available or version too low")
628-
629-
630-
async def skip_if_module_version_error_async(func, *args, **kwargs):
631-
"""
632-
Execute async function and skip test if RedisModuleVersionError is raised.
633-
634-
Args:
635-
func: Async function to execute
636-
*args: Arguments for the function
637-
**kwargs: Keyword arguments for the function
638-
"""
639-
try:
640-
return await func(*args, **kwargs)
641-
except RedisModuleVersionError:
642-
pytest.skip("Required Redis modules not available or version too low")

0 commit comments

Comments
 (0)