feat(redis): Add retrieve_online_documents_v2 support with Redis vect…#6478
feat(redis): Add retrieve_online_documents_v2 support with Redis vect…#6478falloficaruss wants to merge 6 commits into
Conversation
…or sets Signed-off-by: Abhishek Shinde <norizzabhii@gmail.com>
…or sets Signed-off-by: Abhishek Shinde <norizzabhii@gmail.com>
10ea7b7 to
ab39cdb
Compare
franciscojavierarceo
left a comment
There was a problem hiding this comment.
Left a few inline comments on the Redis vector-set implementation. The main blockers are around the claimed L2 semantics and Redis < 8 capability detection.
| return self._vadd_supported | ||
| try: | ||
| info = client.execute_command("COMMAND", "INFO", "VADD") | ||
| self._vadd_supported = info is not None and len(info) > 0 |
There was a problem hiding this comment.
This can report VADD as supported on Redis versions that do not have it. COMMAND INFO <name> returns a nil entry for unknown commands, so a response shaped like [None] still has len(info) > 0; writes would then proceed into VADD and fail instead of cleanly skipping/raising the intended unsupported-server path. Can we inspect the actual command entry/value, not just the container length?
| floats = [float(v) for v in python_vector] | ||
| if not floats: | ||
| continue | ||
| if metric in {"COSINE", "L2"}: |
There was a problem hiding this comment.
This makes the advertised L2 mode behave like cosine/angular search. Normalizing indexed vectors and the query discards magnitude, so vectors like [2, 0] and [100, 0] become equivalent even though true L2 distances are very different. Since VSIM WITHSCORES returns Redis's similarity score, Redis should either reject L2 here or use a backend/query path that preserves real L2 semantics.
| deleted_count += 1 | ||
| pipe.execute() | ||
|
|
||
| # Also delete the Vector Set key if it exists |
There was a problem hiding this comment.
This cleanup is skipped when delete_table() returns early because no hash keys were found. That can leave the vector set behind after hash TTL expiry or partial cleanup. Could we delete vs_key before the if not all_keys: return path, or otherwise ensure table cleanup always removes the vector set?
What this PR does / why we need it:
Adds Redis 8 Vector Set support to the Redis online store for
retrieve_online_documents_v2, so Feast can do ANN/vector similarity search without bypassing the SDK.This PR:
VADDVSIMCOSINEandL2metrics from Feast vector metadataWhich issue(s) this PR fixes:
Fixes #6461
Checks
git commit -s)Testing Strategy