On a RESP3 connection the migration executor's readiness check concludes that every index is fully built with zero indexing failures, whatever the server actually reports. It therefore skips the SCAN fallback that exists precisely to avoid missing documents. This is a silent wrong answer rather than a crash, which is what makes it worth fixing ahead of louder failures: the check passes, the migration proceeds, and documents the server could not index are never enumerated.
Reproduction
Measured on Redis 8.4.6 with redis-py 8.1.0, RedisVL at edcc78e. The index covers five hashes: three index cleanly, and two carry a malformed vector blob, so the server reports hash_indexing_failures of 2.
=== RESP2 ===
actual hash_indexing_failures on server: 2
info.get("hash_indexing_failures", 0) as the executor reads it: 2
keys enumerated (5): ['f:bad0', 'f:bad1', 'f:ok0', 'f:ok1', 'f:ok2']
=== RESP3 ===
actual hash_indexing_failures on server: 2
info.get("hash_indexing_failures", 0) as the executor reads it: 0
keys enumerated: none, raises KeyError: slice(1, None, None)
Under RESP2 the guard fires, logs Index 'fail_idx' has 2 indexing failures. Using SCAN for complete enumeration., and enumerates all five keys including the two that failed to index. Under RESP3 the guard reads zero failures, skips SCAN, and hands control to the aggregate path.
Cause
FT.INFO comes back with bytes keys under RESP3. Measured on both redis-py 6.3.0 and 8.1.0:
[b'index_name', b'index_options', b'index_definition', b'attributes', b'num_docs', ...]
redisvl/migration/executor.py:254 reads that dict with string keys:
failures = int(info.get("hash_indexing_failures", 0) or 0)
percent_indexed = float(info.get("percent_indexed", 1.0) or 1.0)
Both lookups miss, so both defaults apply. The defaults were chosen to mean "nothing is wrong", so the guard concludes the index is complete and failure-free and takes the fast path. redisvl/migration/async_executor.py:107 and :108 are the same code, and async_executor.py repeats the shape around line 222, where _enumerate_with_scan reads prefixes out of the same raw info reply.
The proximate cause is that these sites call client.ft(index_name).info() directly rather than going through RedisVL's own wrapper. SearchIndex._info in redisvl/index/index.py passes the reply through convert_bytes, so SearchIndex.info() returns string keys and the right value on the same RESP3 client:
>>> index.info()["hash_indexing_failures"]
2
Measured on the same build and the same index that produced the zero above. The planner already uses that wrapper, since redisvl/migration/planner.py:216 calls index.info(), so its readiness checks at lines 157 and 174 read correctly. Only the executor's raw calls are wrong, in both the sync and async variants.
Suggested fix
Route the executor's readiness checks through convert_bytes, or through SearchIndex.info(), so that the executor and the planner read FT.INFO the same way. Reading one field under two conventions in two files of the same package is the underlying problem, and normalising at a single point fixes every call site at once. The defaults are worth revisiting too: a lookup that misses should not be indistinguishable from a server reporting good news.
Related
The path this bug hands control to is itself broken on RESP3: see #714, where the aggregate cursor read raises KeyError, which is the exception shown in the reproduction above. The two cannot both surface under RESP2, because a correct readiness check diverts to SCAN before the aggregate code runs. Under RESP3 this bug removes the guard and #714 then raises.
On a RESP3 connection the migration executor's readiness check concludes that every index is fully built with zero indexing failures, whatever the server actually reports. It therefore skips the SCAN fallback that exists precisely to avoid missing documents. This is a silent wrong answer rather than a crash, which is what makes it worth fixing ahead of louder failures: the check passes, the migration proceeds, and documents the server could not index are never enumerated.
Reproduction
Measured on Redis 8.4.6 with redis-py 8.1.0, RedisVL at
edcc78e. The index covers five hashes: three index cleanly, and two carry a malformed vector blob, so the server reportshash_indexing_failuresof 2.Under RESP2 the guard fires, logs
Index 'fail_idx' has 2 indexing failures. Using SCAN for complete enumeration., and enumerates all five keys including the two that failed to index. Under RESP3 the guard reads zero failures, skips SCAN, and hands control to the aggregate path.Cause
FT.INFOcomes back with bytes keys under RESP3. Measured on both redis-py 6.3.0 and 8.1.0:redisvl/migration/executor.py:254reads that dict with string keys:Both lookups miss, so both defaults apply. The defaults were chosen to mean "nothing is wrong", so the guard concludes the index is complete and failure-free and takes the fast path.
redisvl/migration/async_executor.py:107and:108are the same code, andasync_executor.pyrepeats the shape around line 222, where_enumerate_with_scanreads prefixes out of the same rawinforeply.The proximate cause is that these sites call
client.ft(index_name).info()directly rather than going through RedisVL's own wrapper.SearchIndex._infoinredisvl/index/index.pypasses the reply throughconvert_bytes, soSearchIndex.info()returns string keys and the right value on the same RESP3 client:Measured on the same build and the same index that produced the zero above. The planner already uses that wrapper, since
redisvl/migration/planner.py:216callsindex.info(), so its readiness checks at lines 157 and 174 read correctly. Only the executor's raw calls are wrong, in both the sync and async variants.Suggested fix
Route the executor's readiness checks through
convert_bytes, or throughSearchIndex.info(), so that the executor and the planner readFT.INFOthe same way. Reading one field under two conventions in two files of the same package is the underlying problem, and normalising at a single point fixes every call site at once. The defaults are worth revisiting too: a lookup that misses should not be indistinguishable from a server reporting good news.Related
The path this bug hands control to is itself broken on RESP3: see #714, where the aggregate cursor read raises
KeyError, which is the exception shown in the reproduction above. The two cannot both surface under RESP2, because a correct readiness check diverts to SCAN before the aggregate code runs. Under RESP3 this bug removes the guard and #714 then raises.