fix: cache clear() and migration key scans hang or crash on Redis Cluster

[vishal-bala]

Two independent bugs with one root cause: four modules hand-rolled a SCAN cursor loop, and a cluster client's SCAN reply is not a cursor.

RedisCluster.scan broadcasts SCAN to every primary and replies with a {node_name: cursor} mapping of node-local cursors. A single value cannot be broadcast back and the mapping cannot be handed to scan(cursor=...). Every hand-rolled loop in the codebase got this wrong, in one of two ways.

Why

BaseCache.clear/aclear spins forever and leaves the cache populated. cursor_int == 0 was never true against the mapping, the Mapping branch only broke once every node reported 0, and the else that advanced the cursor was unreachable — so the cursor stayed 0 and SCAN 0 was re-issued indefinitely. Affects SemanticCache and EmbeddingsCache; neither overrides BaseCache.

The reason this shipped is worth stating, because it is what made the bug invisible to tests. Re-issuing SCAN 0 accidentally makes progress when every key in the DB matches the cache prefix: each round deletes the first page, the keyspace shrinks, and the loop drains. A cache that is the entire keyspace therefore clears fine even with the bug present. The genuine hang needs keys that do not match the prefix — the normal case, since RedisVL shares a keyspace between index documents, caches and application data. Then a SCAN 0 page can match nothing, nothing is deleted, and the loop makes zero progress.

Measured against a real 3-primary Redis 8.4 cluster with 50k unrelated keys and 200 cache keys: 20,000 SCAN calls without terminating, 197 of 200 cache keys orphaned.

Six migration SCAN loops had the same bug, and worse. They fed the reply cursor straight back into client.scan(cursor=...), so the second iteration passes a dict and redis-py raises DataError: Invalid input of type: 'dict'. Verified against a real cluster. Unlike the cache hang this is a hard crash on the second SCAN, not a silent under-count, and it fires for any cluster keyspace regardless of what the keys look like — arguably the wider blast radius of the two, even though it was only found while chasing the cache hang. Reachability, most to least exposed:

site	when it runs
validation._count_index_keys	every validate run
async_validation._count_index_keys	every validate run
async_planner._async_sample_keys	count=max(limit, 10) rarely fills the limit on the first page
planner._sample_keys	count=max(limit, 1000) usually returns early, but not when the keyspace is smaller than the limit
executor._enumerate_with_scan	fallback paths only
async_executor._enumerate_with_scan	fallback paths only

What changed

All seven sites are plain "enumerate keys matching a pattern", so each collapses to redis-py's scan_iter, which already drives each primary on its own cursor via target_nodes. Cluster cursor semantics move upstream where they belong. That also drops the sync/async duplication in clear and both # type: ignore comments in base.py (a third goes from planner.py); none are added back, and mypy ./redisvl is clean.

redisvl/extensions/cache/base.py	clear/aclear become 9 lines each on scan_iter. Deletes batch at CLEAR_BATCH_SIZE = 500 rather than one DEL per SCAN page.
redisvl/migration/{planner,executor,validation}.py + async twins	Six loops → scan_iter. The sample sites gain a bonus: a generator lets the sample limit stop mid-page instead of draining the page first.
redisvl/utils/utils.py	scan_by_pattern widened from Redis to SyncRedisClient. It was already cluster-correct via scan_iter; only the annotation said otherwise.

The clear/aclear docstrings now document what callers actually get: SCAN is not a point-in-time snapshot, so this is a best-effort sweep and not an atomic flush — concurrent writers may or may not be swept, duplicate pages are harmless because DEL on a gone key is a no-op, and a mid-sweep failure is safe to retry because the operation is idempotent. Worth reviewing as prose, not just as comments: docs/api/cache.rst autodocs both caches with :inherited-members:, so BaseCache.clear's docstring is the published API reference text.

Commit split

Five commits, each independently reviewable, in an order where every fix is separable from its tests:

1. fix(cache): the cache hang — base.py plus its unit tests.
2. fix(migration): the six migration loops — a different subsystem with a different failure mode, and reviewable without an opinion on the cache.
3. test(cluster): repairs the pre-existing cluster cache tests, described below.
4. fix(migration): an aclosing follow-up on _async_sample_keys — see below.
5. test: the mutation-driven trim of the cache suite and the new migration regression file.

Commit 4 exists because scan_iter introduced a generator where a while loop had none: _async_sample_keys returns from inside async for once the sample limit is reached, and an async generator abandoned that way is not closed until loop shutdown. That surfaced as RuntimeWarning: coroutine method 'aclose' ... was never awaited in the migration tests. Wrapped in contextlib.aclosing. The sync planner has the same early return but a plain generator is closed deterministically by refcounting on CPython and emits no warning, so it is left alone.

Tests

Two pre-existing cluster tests had never executed. test_embeddings_cache_cluster_sync/_async passed text= to EmbeddingsCache.set/aset, whose parameter is content. They raised TypeError on their first statement — which is part of why the cluster clear hang shipped. Both also called clear() with no assertion afterward, so even once repaired they would not have caught it.

The two new cluster regression tests (sync + async) therefore seed both unrelated keys and a cache larger than one SCAN page, because of the accidental-progress effect above — 100 keys, what the existing test used, is exactly the size that passes either way. They bound the number of SCAN calls clear() may issue, since a regression hangs rather than fails and pytest-timeout is not installed, and they assert paging actually happened so the test cannot pass vacuously. Key counting goes through scan_iter, never KEYS or DBSIZE: those route to a single node on a cluster and silently report roughly one shard's worth (measured: 201 of 600).

Unit coverage, at both ends. tests/unit/test_migration_cluster_scan.py (4 tests) is new because reverting all six migration modules left 65/65 existing tests green — that fix had no regression coverage at all. tests/unit/test_cache_clear_cluster_cursor.py went the other way: mutation testing showed that of 12 cache tests only 3 killed anything no other test killed and two killed nothing, so it is trimmed to 5 with a strictly larger kill set. Now that the cursor walk lives in redis-py, most of what the dropped tests asserted was upstream's behavior. test_deletes_are_batched is kept deliberately — it is the only thing standing between us and client.delete(*list(client.scan_iter(...))), which would OOM on a large cache — and now patches CLEAR_BATCH_SIZE instead of depending on its value.

Both fake clients bind redis-py's real scan_iter over their own scan, so the tests drive the actual library loop rather than a reimplementation of it. The migration fake replies with per-node cursors and raises DataError on a dict cursor, so the old loop fails there exactly as it fails against a real cluster.

1411 passed, 1 skipped on the unit suite; mypy, black and isort clean.

Not in scope

There is no CI signal for any of this. --run-cluster-tests is passed by no workflow in .github/workflows/ and make test never sets it, so all 20 requires_cluster tests — including the 2 added here — are skipped in CI. The unit tests above are what actually guards these fixes on every run; the cluster tests only ran locally. A CI job that passes the flag is the obvious follow-up, but it is a cost/runtime decision (6 containers per run) and is left to a separate PR.

clear() uses DEL where drop_keys uses UNLINK. SearchIndex.drop_keys was deliberately moved to UNLINK in #616 (issue #600) to avoid stalling the server on bulk deletes. BaseCache.clear on a large cache has the same blocking profile and was not changed here, to keep this PR to the cursor bug. Worth its own issue.

EmbeddingsCache.amset silently writes nothing on an async cluster client — it awaits the pipeline object returned by the queueing call, which drains the queue before execute(). That is a separate bug being fixed separately. The async cluster test here deliberately seeds with aset instead and says so in a comment, since this test is about aclear.

Adjacent but untouched: #601 (drop_keys does not validate cluster hash-tag co-location) is the same "written against standalone, wrong on cluster" family, if you want a theme for a follow-up sweep.