fix: Rebuild the SQLite vector and keyword search index on every query - #6923
Open
MohammadHijjawi97 wants to merge 1 commit into
Open
MohammadHijjawi97 wants to merge 1 commit into
MohammadHijjawi97 wants to merge 1 commit into
Conversation
The SQLite online store answers retrieve_online_documents and retrieve_online_documents_v2 by copying the feature view's rows into a vec0 (vector) or FTS5 (keyword) table and querying that table. The table lived in the online store database and was never cleared, so every search added another copy of every row: - Vector search only worked once. Later searches failed with "UNIQUE constraint failed on vec_table primary key" (v2) or "table vec_table already exists" (v1), and once a write committed the leftover rows, even the first search of a new process failed. - Keyword search returned fewer documents on every call, as copies of the best matches filled top_k, and kept matching text that had since been overwritten. The search also left its transaction open, so writes from other processes failed with "database is locked". Build the index in the connection's temp schema, drop and refill it for every search, and commit it right away. Signed-off-by: Mohammad Hijjawi <mohammad.hijjawi1997@gmail.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6923 +/- ##
=======================================
Coverage 48.58% 48.58%
=======================================
Files 427 427
Lines 53787 53792 +5
Branches 7834 7834
=======================================
+ Hits 26132 26136 +4
- Misses 25788 25789 +1
Partials 1867 1867
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The SQLite online store answers
retrieve_online_documentsandretrieve_online_documents_v2(and so the feature server's/search) by copying the feature view's rows into avec0(vector) or FTS5 (keyword) table and querying that. The table lives in the online store database and is never cleared, so every search adds another copy of every row:vector_enabled: true) works only once. The nextretrieve_online_documents_v2call fails withsqlite3.OperationalError: UNIQUE constraint failed on vec_table primary key, andretrieve_online_documentswithtable vec_table already exists. Once a write commits the leftover rows, even the first search of a new process fails.text_search_enabled: true) returns fewer documents on every call, because copies of the best matches filltop_k, and it keeps matching text that has since been overwritten.feast materializenext tofeast serve) fail withdatabase is lockeduntil the searching process writes something itself.Repro on master, same data and query each time:
This PR builds the index in the connection's
tempschema, drops and refills it for every search, and commits it right away. Every call now returns the sametop_kfrom the current rows, a rewritten document no longer matches its old text, and nothing is left in the online store file or locked. The vector index build is shared by both APIs.Which issue(s) this PR fixes:
No existing issue.
Checks
git commit -s)Testing Strategy
Misc
Added two tests to
sdk/python/tests/unit/online_store/test_online_retrieval.py; both fail on master and pass with this change:test_sqlite_get_online_documents_v2_search_is_repeatablerepeats a keyword search and rewrites a document.test_sqlite_get_online_documents_is_repeatablerunsretrieve_online_documents_v2andretrieve_online_documentstwice each (Python 3.10 only, like the existing sqlite-vec test).All sqlite tests in that file pass on Python 3.10 with SQLite 3.53 (vec0 KNN queries need SQLite >= 3.41 for
LIMIT, as on ubuntu-latest), as doestest_sqlite_get_online_documentswhen run directly. I did not run the Milvus tests (milvus-lite has no Windows build).ruffandmypyonsqlite.pyare clean.