Add flush(), so a RAG index survives the process (#492) - #507
Merged
Conversation
Neither RAG store persisted on demand. qdrant never called EdgeShard.flush(), so points added through addDocument stayed in the shard's in-RAM segment: an index built in one session was gone in the next and the corpus was embedded again on every launch. The reporter measured it on two Android devices and named the missing piece — the segment's id-tracker files never reach disk, which is why the next open fails with "Failed to load ID tracker mappings". close() persisted already, since unloading writes too, but that does not help the case this is about: an Android app the system kills in the background never gets to close anything, and there was no way to persist without giving up the store. flush() joins the VectorStoreRepository contract rather than only the qdrant class. A caller holding the interface — which is what FlutterGemma.initialize hands back — would otherwise need a backend type-check to ask for durability. On web it is NOT a no-op, and assuming it was is what the first draft of this change got wrong. sqlite3 autocommits, so the bytes reach the VFS; whether the VFS wrote them anywhere durable is a separate question, and for the one Flutter web actually gets — IndexedDB, since OPFS needs a dedicated worker — the answer is no. Its xSync is a documented no-op and its own docs describe writes as asynchronous "without any durability guarantees. You can invoke flush". The store now keeps the VFS reference so it can drain it, close() drains too, and the in-memory fallback throws instead of reporting a success it cannot deliver: returning quietly there would have been this very bug wearing a different hat. The qdrant flush translates QdrantException into VectorStoreException like every other method on that class. It is not a VectorStoreException and is not exported from the barrel, so leaking it raw would have made `on VectorStoreException` — the catch the contract asks for — miss every flush failure. Native SQLite really is a no-op: the connection is in autocommit and never opens a transaction. It says so at its own site rather than leaving callers to guess which backends need the call. Core takes a minor bump, not a patch: adding a member to a publicly implementable interface is source-breaking for an external `implements`-er, and the neighbouring filterSchema doc claimed otherwise — that claim is corrected here too. Tested against a real shard, not a mock: 20 documents written, id-tracker files asserted absent before flush and present after, mutation-checked by dropping the call. A failing flush is pinned to VectorStoreException by removing the shard directory under an open store. The reopen-after-close test is relabelled to say what it actually proves, since close() persists on its own and it passes with the flush deleted.
Round two of review, on the fix itself rather than the bug.
The web flush was still wrong, and in the opposite direction from round one.
It no longer claims to be a no-op — it drains the IndexedDB VFS — but on
sqlite3 >= 3.4.0 that drain does not wait. Upstream commit 11be8acb
("Optimize indexeddb flush on idle") replaced a marker queued behind the
running batch with a bare `_startWorkingIfNeeded`, whose body is skipped while
a batch is in flight, so flush returns in zero event-loop turns. Their own doc
still promises to await; 3.3.4 still did. It is an unintended regression, not
a change of model.
So the code stays and the words change. Capping the dependency below 3.4 was
the tempting alternative and is not viable: drift 2.35.0 requires sqlite3
^3.4.0 and sqlite_async ^3.5.0, so a cap would turn our durability gap into a
resolution failure for the most common consumer. Owning the drain ourselves
(writeAutomatically: false plus a timer) needs a 3.4 floor anyway, and its own
timer manufactures the interleaving that makes flush return early — while
widening today's loss window for every web caller that never calls flush. The
exposure being documented is bounded: the VFS streams writes continuously, so
what an early return misses is the batch in flight, not the index.
close() is the stronger drain on web and now says so, in both directions: the
contract no longer calls a flush before close redundant in a way that reads as
"close is enough", because on qdrant close swallows a failed save and flush is
the call that reports it.
Also fixed, all found by the same round:
- flush() was unreachable from FlutterGemma.rag, so the facade every app uses
could not ask for durability at all. Added through the platform interface and
all three shells.
- close() settled its fields in a `finally` after the await, so isInitialized
reported true over a closed database and a flush landing in that window
reached a VFS that was closing.
- a re-initialize overwrote the VFS field without draining the old one,
orphaning its queued pages.
- flush()'s guard returned early when a failed re-initialize had left the
previous run's VFS holding unwritten pages.
- the class dartdoc had attached itself to a private enum I inserted above it.
- _rethrow's final `throw e` escaped raw, so anything that is neither an
EdgeException nor a UniffiInternalError crossed all twelve wrappers and the
store's translation never fired.
- the OPFS comment named the wrong mechanism: durability comes from SQLite
calling xSync on commit, not from the write having reached storage.
- core's bump is marked as breaking for custom implementations, matching how
1.8.0 marked the same shape of change.
Tests: the id-tracker assertion could pass on a store that never wrote, so the
segment directory is asserted present first. A black-box test reproduces #492
in one process — a copy of the shard taken before the flush reopens empty and
silent, after it carries every document — which pins the contract rather than
this engine's file names. Mutation-checked: dropping the flush call fails
three tests, including that one. The two tests that shell out or delete a
mmapped directory are skipped on Windows.
The example's sqlite3 dev-dep moves to ^3.5.2 while the package keeps ^3.3.0:
the example resolves separately from the workspace, and was pinning 3.3.3 — so
the web suite was exercising a flush() that consumers resolving latest do not
get.
Three findings from a review of the previous commit. - Web flush() threw the in-memory error while initialize() was still running: `_sqlite3` is set before a VFS is chosen, so the guard let the call through while `_persistence` was unset. The guard is now "not initialized and no IndexedDB VFS", and a re-initialize no longer clears `_persistence` up front. - `_rethrow` wrapped the package's own QdrantException a second time, burying "not written by this package" under "unexpected error" and dropping QdrantShardLockedException's type. Ours are rethrown as they are. The foreign-shard test now checks the message; removing the fix fails it. - The example's sqlite3 dev-dep goes back to ^3.3.0. The previous commit said the example lock pinned 3.3.3; it did not — it already resolved 3.5.2, and only the root workspace lock is on 3.3.3. The bump changed nothing.
…1, rag_sqlite 1.3.2
DenisovAV
added a commit
that referenced
this pull request
Sep 12, 2026
#507 bumped flutter_gemma to 1.8.1 for VectorStoreRepository.flush(), the version this branch had claimed. Neither is published, so the skills take 1.8.2 and main's 1.8.1 can be released on its own. CLAUDE.md takes main's Current Version line, which also corrects this branch's speech 0.5.1 to the real 0.5.0.
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.
Fixes #492 (first finding).
flutter_gemma_rag_qdrantnever calledEdgeShard.flush(), so points added throughaddDocumentstayed in the shard's in-RAM segment: an index built in one session was gone in the next, and the corpus was embedded again on every launch.@think2execute measured it on two Android devices and named the missing piece precisely — the next open fails with
Failed to load ID tracker mappings. That was exactly the right place to look:flush()mutable_id_tracker.mappingsmutable_id_tracker.versionsEverything else — vector storage, payload storage,
segment.json— is on disk well before that.close()persisted already, since unloading writes too, but that only covers a store the app closes cleanly. An Android app the system kills in the background never gets there, and there was no way to persist without giving up the store.What
flush()means per backendflush()joins theVectorStoreRepositorycontract rather than only the qdrant class — a caller holding the interface, which is whatFlutterGemma.initializehands back, would otherwise need a backend type-check to ask for durability. It is also exposed asFlutterGemma.rag.flush(), without which the facade every app uses could not reach the fix at all.xSyncdoes nothing and whose writes are asynchronous by design. So the store keeps the VFS reference and drains it, andclose()drains too.Why web is documented rather than worked around
On
sqlite3>= 3.4.0 that drain is partial:flush()returns without awaiting a batch already in flight. Upstream commit11be8acb("Optimize indexeddb flush on idle") replaced a marker queued behind the running batch with a bare_startWorkingIfNeeded, whose body is skipped while_isWorkingis true. 3.3.4 still had the marker; the doc comment still promises to await. It is an unintended regression, and no upstream issue reports it yet.The two alternatives were both worse:
sqlite3 <3.4.0—drift 2.35.0requires^3.4.0andsqlite_async^3.5.0. A cap turns our durability gap into a hard resolution failure for the most common consumer.writeAutomatically: false+ a debounce timer) — needs a 3.4 floor anyway, since the option does not exist on 3.3.x; the timer manufactures exactly the interleaving that makesflush()return early; and it widens today's loss window for every web caller that never callsflush().The exposure being documented is bounded: the VFS streams writes into IndexedDB continuously, so what an early return misses is the batch in flight, not the index. Reported upstream as simolus3/sqlite3.dart#408, with a fix in simolus3/sqlite3.dart#409 that keeps their idle optimisation and restores the marker only while a batch is running. When it ships, raising the floor makes our
flush()a true fence with no code change here.Other findings from review, all fixed
close()settled its fields in afinallyafter the await, soisInitializedreported true over a closed database and aflush()in that window reached a closing VFS.flush()'s guard returned early when a failed re-initialize had left the previous run's VFS holding unwritten pages._rethrow's finalthrow eescaped raw, so anything neither anEdgeExceptionnor aUniffiInternalErrorcrossed all twelve wrapper methods and the store's translation toVectorStoreExceptionnever fired.xSyncon commit, not from the write having reached storage.Verified
flush()call fails three tests, including the black-box one.flutter_gemma_rag_qdrant115/115,flutter_gemma_rag_sqlite121/121,flutter_gemma710/710,flutter analyze packages/clean.sqlite3dev-dep moves to^3.5.2while the package keeps^3.3.0: the example resolves separately from the workspace and was pinning 3.3.3, so the web suite was exercising aflush()consumers do not get.Not covered
The web path still has no automated test — it needs a browser, and the existing web suite runs under
flutter drive. The IndexedDB behaviour here is argued from the VFS's own source plus a headless-Chrome measurement from review, not from a test in this repo.The second finding in #492 — one device failing to reopen an intact shard — is not ours: it is
qdrant/qdrant#10307, a zero-lengthPOSIX_FADV_NOREUSEreturningENOENTon F2FS kernels with the memory-donation patch. Fix isqdrant/qdrant#10365, approved and waiting on CI.