Skip to content

perf: replace DELETE with UNLINK in EmbeddingsCache for better memory management #613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LizerAIDev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf: replace DELETE with UNLINK in EmbeddingsCache for better memory management #613

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions redisvl/extensions/cache/embeddings/embeddings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,8 @@ def drop(self, content: bytes | str, model_name: str) -> None:
def drop_by_key(self, key: str) -> None:
"""Remove an embedding from the cache by its Redis key.

Uses UNLINK instead of DELETE for better performance by freeing memory asynchronously.

Comment on lines +515 to +516

@vishal-bala vishal-bala Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop these two lines. Which command runs isn't part of the method's contract, and docs/api/cache.rst:68 autoclasses this class, so the note ships to docs.redisvl.com. It also only describes one of the four methods you changed.

Args:
key (str): The full Redis key for the embedding.

Expand All @@ -520,7 +522,7 @@ def drop_by_key(self, key: str) -> None:
cache.drop_by_key("embedcache:1234567890abcdef")
"""
client = self._get_redis_client()
client.delete(key)
client.unlink(key)

def mdrop_by_keys(self, keys: list[str]) -> None:
"""Remove multiple embeddings from the cache by their Redis keys.
Expand All @@ -542,7 +544,7 @@ def mdrop_by_keys(self, keys: list[str]) -> None:

with client.pipeline(transaction=False) as pipeline:
for key in keys:
pipeline.delete(key)
pipeline.unlink(key)

@vishal-bala vishal-bala Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This queues one single-key UNLINK per key inside a pipeline, while amdrop_by_keys (line 888) sends the whole list in one call. If you want the throughput win the description claims, mirror the async path here: drop the pipeline and call client.unlink(*keys) in chunks of batch_size, the way SearchIndex.drop_keys does. That's one round trip per chunk instead of one per key, and redis-py splits the keys by slot on cluster, so it stays safe there.

pipeline.execute()

def mdrop(self, contents: Iterable[bytes | str], model_name: str) -> None:
Expand Down Expand Up @@ -883,7 +885,7 @@ async def amdrop_by_keys(self, keys: list[str]) -> None:
return

client = await self._get_async_redis_client()
await client.delete(*keys)
await client.unlink(*keys)

async def amdrop(self, contents: Iterable[bytes | str], model_name: str) -> None:
"""Async remove multiple embeddings from the cache by their contents and model name.
Expand Down Expand Up @@ -982,4 +984,4 @@ async def adrop_by_key(self, key: str) -> None:
await cache.adrop_by_key("embedcache:1234567890abcdef")
"""
client = await self._get_async_redis_client()
await client.delete(key)
await client.unlink(key)