Publish vector index files atomically (but not durably) (speedkick) by edwinyyyu · Pull Request #1588 · MemMachine/MemMachine

edwinyyyu · 2026-09-08T18:40:03Z

Copy of #1460 onto speedkick. Cherry-picked cleanly; no conflicts and no changes were needed. Targeted suites common/vector_store and semantic_memory pass (656 passed, 2 skipped).

Problem

SQLiteVectorStore persists each collection's index by calling the search engine's save(), which writes directly to the final path. A crash partway through leaves a truncated file on disk.

The torn file is only half of it. _save_collection_index trims the applied _PendingOperationRows as soon as save() returns, and that log is the only other copy of those vectors — the records table has no vector column. So the trim is safe at exactly one instant: when the index at the published path holds them. Getting that instant wrong is silent, because a missing vector is indistinguishable from a vector that simply doesn't match.

Fix

A shared index_persistence helper writes the index to a sibling temp file, flushes it, and swaps it into place with os.replace; both engines adopt it and neither gains anything else. load clears a temp left by an interrupted save, so a crash leaks at most one stale file per index.

After this, a save either publishes a complete index or leaves the previous one exactly where it was. Nothing else changes: no schema change, no migration, no rename of index_path, and no new file at the base path.

What a save does not promise

It does not promise that the publication is durable. That is a decision rather than an oversight, so the reasoning is below.

A rename changes a directory entry, not file data, and the two have very different guarantees:

	make it durable	available
file contents	`fsync` / `F_FULLFSYNC` / `FlushFileBuffers`	everywhere, documented
directory entry	`fsync` on a directory file descriptor	POSIX only, and not on every filesystem

POSIX closes the gap with a parent-directory fsync, but only best-effort — network and FUSE filesystems are where that bites.
Windows has no equivalent at all. os.fsync is _commit, which is FlushFileBuffers, which is for file data; you cannot open a directory to fsync it. MOVEFILE_WRITE_THROUGH does not help either: its documented guarantee is scoped to "a move performed as a copy and delete operation", the cross-volume path, and says nothing about flushing NTFS metadata for a same-volume rename. The decisive evidence is SQLite's — its VFS threads a directory-sync flag through every commit-relevant directory operation, unixDelete honors it, and winDelete declares the same parameter /* Not used on win32 */. SQLite cares about this more than almost any software and makes no attempt on Windows.

So a power failure can roll the rename back after save returned, while the trim behind it stays committed.

Why that gap stays open

Closing it means never using a directory operation as the commit point: two index slots plus a generation record written into a file that already exists, in the spirit of SQLite's PERSIST journal mode. An earlier revision of this PR implemented exactly that, and it worked. It was still the wrong trade, for four reasons.

The window is narrow, and only one kind of failure lands in it. A clean shutdown, an unhandled exception, an OOM kill, a SIGKILL — none of these lose a rename, because the kernel still owns the page cache and writes it back. Those failures are already covered: the pending log holds everything the engine has not been checkpointed with, and startup replays it. What is left is the machine itself dying between the rename and writeback — a power cut, a host failure, a panic — plus the filesystems where a directory fsync is best-effort or unavailable anyway.

What it costs is recall, not correctness. The records table is the authority on what exists, and every query hit resolves through it, so a reverted publication cannot produce a wrong or stale result — only fewer results. Concretely: the records survive, get still returns them, query stops finding them, and the exposure is bounded by the operations applied since the previous checkpoint, i.e. at most save_threshold. Re-upserting an affected record repairs it. That is the direction this store already tolerates, and it is repairable by the same ingest path that created the record.

Detecting it is not cheap enough to be worth it. Comparing the index's size against the record count is the obvious check and it does not work: row_ids are AUTOINCREMENT and never reused, so deleting a record and upserting the same uuid again moves it to a new id. A rollback spanning that pair leaves the index holding the old id and missing the new one — one extra, one missing, identical count — and a re-embedding workload produces that shape constantly. A check that does work needs an id-set digest maintained by every engine, or a full reconcile scan at every boot, which is most expensive precisely where the index is large. Neither earns its keep against a bounded recall gap.

And the guarantee is not free to hold. It cannot be delegated to a rename, so it has to become a layout: two slots and a generation record per index, which every engine has to implement and every future engine has to be audited against. Engines that persist by writing one file — which is what both engines here do, and what most ANN libraries expose — stop being usable as they ship. Existing indexes stop loading until they are cleared and re-ingested, which is a migration this revision no longer asks anyone to perform. Fewer guarantees, less machinery to keep correct, and a wider set of engines that can plug in unmodified.

The direction that would cost — a published index that will not parse — stays closed: the atomic swap prevents it, the pre-swap fsync narrows the window where a rename outlives the data behind it, and a saved-but-unloadable index remains a loud IndexLoadError rather than a silently empty rebuild.

Nothing here detects a lost publication for the caller. A deployment that needs every record searchable after a power failure must be able to re-ingest.

Where the contract is stated

The guarantee is only useful if callers can find it, so it is written where each of them looks:

VectorSearchEngine.save — returning means a later load reads this index or the one it replaced, never a mixture; it does not mean the publication survives a power failure.
index_persistence — the mechanism, and why a stronger guarantee is not portable.
sqlite_vector_store — what survives a process crash, what a power failure costs, and that re-ingest is the repair.
_save_collection_index — why the save-then-trim order is the whole protocol.

One consumer needed more than a docstring. VectorStoreSemanticStorage.update_feature reads a feature's stored embedding back when the caller updates the feature without supplying one — the only place in the server that depends on the index still holding a vector. It reported a lost vector as Vector record not found, naming the feature, which points at the wrong thing and hides the repair; it now separates a record that is genuinely absent from one whose embedding is gone, and the second says to pass a fresh embedding.

Tests

test_index_persistence.py: the swap publishes completely or not at all, a failed write leaves the previous index intact and removes the temp, and a stale temp is cleared on load.
Per-engine (hnswlib + usearch): repeated saves leave no stray files, and a reload after two checkpoints sees the newer index.
Store-level: test_a_reverted_publication_costs_search_not_records reconstructs a lost publication deterministically — restore the previous index bytes after the trim has committed — and pins the direction it fails in: the record still resolves by uuid, and only search loses it. Missing and corrupt indexes each still raise IndexLoadError once index_saved is set.

vector_store suites pass (284 passed, 140 integration deselected) and semantic_memory suites pass (345 passed, 2 skipped, 999 integration deselected); ruff and ty clean.

Changed from the earlier revision

This PR previously shipped the two-slot generation-record protocol and its breaking on-disk change. That guarantee has been dropped in favor of the atomic swap plus an explicit contract, so the breaking-change notice no longer applies: indexes written by the current code keep loading, and there is nothing to migrate.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

SQLiteVectorStore persists each collection's index by calling the search engine's save(), which wrote directly to the final path. A crash mid-write left a truncated/corrupt file. Because index_saved=True makes the on-disk index a durable contract (missing/corrupt is a hard IndexLoadError, not a silent empty rebuild), an interrupted save could render a collection unrecoverable. Write the index to a sibling temp file and swap it into place with os.replace (atomic on POSIX and Windows on the same filesystem), so a reader sees either the old or new index, never a partial write; a failed save leaves the previous index intact. Leftover temp files are cleared on load so a crash does not leak them across restarts. Implemented in the engines (shared index_persistence helper) rather than in SQLiteVectorStore/SQLiteVectorStoreCollection, since the index save location and number of files written differ across engine implementations. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

The swap protects a reader from a torn index, but the vector store also trims its pending-operation log once `save` returns -- and that log is the only other copy of those vectors, since the records table stores no vector column. So the swap reaching disk is load-bearing rather than a bonus: - fsync the parent directory after the replace, since POSIX `rename(2)` leaves the new directory entry in the page cache. Best-effort and ignored on failure, matching SQLite's `unixSync`; a no-op on Windows, which has no equivalent operation. - stop swallowing a failed fsync of the temp file. SQLite draws the same line -- a file fsync failure raises SQLITE_IOERR_FSYNC while a directory fsync failure is ignored -- and `EIO` means the writeback already failed and the dirty pages were dropped, which is exactly when the save must not be reported as committed. The existing cleanup then leaves the previous index in place with the log untrimmed, so the next save retries. - use F_FULLFSYNC on macOS, where plain `fsync` leaves the data in the drive's volatile write cache, falling back when a filesystem refuses it. State the resulting obligation on `VectorSearchEngine.save` itself, since that is what the store now relies on: replace atomically, then make the replacement as durable as the platform allows. An engine whose backend already implements the whole protocol can delegate to it and skip these helpers; the rest use `atomic_index_write`. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

The pending log holds the only durable copy of a vector between checkpoints -- the records table has no vector column -- so trimming it is safe only at an instant when the index provably holds those vectors. The temp-write + rename protocol this PR shipped could not provide that instant. A rename changes a directory entry, and Windows exposes no way to flush one: os.fsync is _commit, which is FlushFileBuffers, which is for file data, and you cannot open a directory to fsync it. The decisive evidence is SQLite's own -- it threads a directory-sync flag through every commit-relevant directory operation, honors it in unixDelete, and declares it /* Not used on win32 */ in winDelete. So os.replace could return, _save_collection_index could commit its trim durably behind it, and a power cut could still roll the rename back: records forward, index back, no copy of the difference left. MOVEFILE_WRITE_THROUGH is not a fix; its documented guarantee covers copy-and-delete (cross-volume) moves, not same-volume renames. Take SQLite's answer, which was not to harden the directory operation but to stop using one as a commit point (PERSIST commits by zeroing a header, TRUNCATE by truncating, WAL by appending frames). A base path now expands into two index slots plus a generation record each, created once and thereafter only overwritten. A checkpoint writes the index over the inactive slot and flushes it, then writes that slot's generation record and flushes that. The record is the commit, and it is a write into a file that already exists. It holds the generation and its bitwise complement, so a torn write reads as absent rather than as some other generation -- all or nothing without needing single-sector atomicity from the hardware. load takes the highest believable generation, and deliberately does not fall back to the older slot when the published index will not parse: the log was trimmed against the newer one, so the older is stale by exactly the ops that can no longer be replayed. Both backends already write straight to the path they are given, which is what this protocol wants -- verified that repeated saves preserve the inode and leave no stray files -- so no engine gains a temp file, a buffer, or a rename. Durability is entirely the engine's, including which artifact is live. The store keeps no slot pointer, manifest, or generation, so no schema change and no migration: what remains is one rule, never trim past what save says is durable, and _save_collection_index already had that order. index_path becomes index_base_path since it no longer names a file, and discarding a collection asks the engine layer which files that covers. BREAKING CHANGE: an index written by the previous protocol is not published under the new one, so a collection with index_saved=True raises IndexLoadError until its index directory is cleared and the records re-ingested. Anomaly tests walk every crash point in the publish sequence by constructing the on-disk state each would leave, plus one that pins the ordering itself (a failed index write must publish nothing) since state-based tests cannot observe it. Verified against three deliberate breaks -- dropping the complement check, writing the record first, and reusing one slot instead of alternating -- each caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

The two-slot generation-record protocol bought a guarantee we have decided not to make: that a save survives a power failure. Every engine would have to implement and maintain that protocol, and the failure it buys out is bounded -- search recall for the records applied since the last checkpoint, repaired by re-ingesting them. The direction that actually costs, a published index that will not parse, is closed by the atomic swap on its own. So this returns to the temp-file-plus-rename publication and spends the difference on stating the contract instead of strengthening it: `save` publishes atomically, never durably; the store trims the pending log behind a publication a power failure can revert; a record whose vector is lost that way still resolves by uuid, is absent from search until it is upserted again, and nothing here detects the gap for the caller. Reverts the durability and engine-owned-publication commits, keeps the atomic swap, and adds a store-level test that reconstructs a reverted publication deterministically -- restore the previous index bytes after the trim -- to pin the direction it fails in. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

`update_feature` reads the stored embedding back when a caller updates a feature without supplying one, and that is the only place in the server that depends on the index still holding a vector. With publication now atomic rather than durable, a power failure can leave a feature whose row is intact and whose vector is not -- a state this path reported as "Vector record not found", which points the caller at the wrong thing and hides the repair. Split the two cases. A record that is genuinely absent keeps the old message; a record whose embedding the index no longer holds says so and names the fix, which is to pass a fresh embedding. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

`_flush_to_disk` wrapped its fsync in `contextlib.suppress(OSError)` and called itself best-effort. A failed fsync is exactly the evidence that the bytes are not safe to publish -- on Linux an EIO from fsync means writeback failed, reported once and then cleared -- so swallowing it and renaming anyway published a file we had positive evidence was bad. The safeguard cost something and, in the one case it existed for, guaranteed nothing. Nothing tested it either. Let it propagate. `atomic_index_write` already unlinks the temp and re-raises, so a failed flush now leaves the previously published index standing, which is the correct outcome. A test pins that. The fsync is not best-effort, and the docstring should not have said so: it rules out a class rather than narrowing a window. Because the flush completes before the rename is issued, and a durable write does not un-happen, the new name can never appear over incomplete bytes. What the missing directory fsync costs is the other direction -- the rename may not survive, so the publish reverts -- and that is the benign one this store already accepts. The module docstring was 76 lines against 49 of everything else, most of it argument rather than documentation: a walk through SQLite's `unixDelete` / `winDelete` sync-flag handling, and a rejected two-slot commit protocol. That is the PR's case for the design, not something to re-read every time someone opens a 20-line module, and the PR body carries it. What a reader here needs is the guarantee, the non-guarantee, and the cost. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

Two things the module was silent on. Why a rename at all. The stronger answer is to put the commit point inside the file, where an fsync reaches it portably -- SQLite never renames, and commits by truncating or zeroing its rollback journal, or in WAL mode by appending frames whose checksums make a torn tail self-identifying. Both need the writer to own the file format. A search engine owns its own and exposes `save(path)`, so above that call a rename is the only atomicity primitive left, and an engine whose format already commits that way needs none of this. Worth saying, because "why not do the better thing" is the first question the module invites. What the reopened descriptor cannot see. Flushing is fine on a fresh fd -- dirty pages belong to the file, not to the descriptor that dirtied them -- but error reporting is not: Linux hands a writeback error to descriptors open when it was recorded, so one recorded between the engine's close and this open is never reported and the save proceeds on bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. It cannot be closed from here: the engine writes through its own descriptor and closes it before returning, and closing the window needs an engine that writes through a handle the caller supplies. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

The fsync was on a descriptor opened after the engine had written and closed its own, which flushes correctly -- dirty pages belong to the file, not to the descriptor that dirtied them -- but reports nothing useful. Linux samples the writeback error sequence when a file is opened, so a descriptor opened after an error was recorded never learns of it: the fsync returns success and the save publishes bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. Open the temp before yielding it and hold it across the caller's write, so the descriptor predates the bytes and any error from writing them is reported here, where it fails the save. That assumes the caller writes in place. Both engines do -- verified: the inode is unchanged across `save_index` and `save`, and the held descriptor sees the written size -- but it is their behaviour, not their contract. An engine that built a file of its own and renamed it over the temp would leave this descriptor on an orphaned inode, and the fsync would report on a file nobody is about to publish. So it is checked before the fsync, and a mismatch fails the save rather than passing it quietly. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

The descriptor held across the body only flushes what the body wrote if the body writes the yielded path in place, and that requirement was recorded in `_flush_to_disk` -- a private function nobody writing an engine opens. It belongs on `atomic_index_write`, which is the API they use, alongside what happens when it is broken: an `OSError` and no publication, so the mistake surfaces at the first save rather than at a power cut. `_flush_to_disk` keeps the mechanism -- why the descriptor has to predate the write -- and now points at the rule instead of restating it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

`os.fsync` is not the same guarantee on all three platforms this ships to. On Linux it flushes to the device, and on Windows `FlushFileBuffers` does the same. Darwin's `fsync` explicitly does not: it returns once the data reaches the drive, which may hold it in a volatile write cache. So on macOS the ordering this module is built on -- data durable before the rename is issued -- did not hold at the device, which is exactly the case it claims to rule out. `F_FULLFSYNC` asks the drive to flush that cache. Filesystems that cannot refuse it, and there `fsync` is the most that can be asked, so a refusal falls back; any other error is a write failure and propagates, as before. The flush-failure test patched `os.fsync`, which Darwin no longer reaches. It patches the module's own `_fsync` instead, which every platform does. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

The fallback from `F_FULLFSYNC` to `fsync` was gated on an errno allowlist -- ENOTSUP, EOPNOTSUPP, EINVAL -- so that a genuine write failure would propagate rather than be quietly downgraded. Probing the actual returns on Darwin shows the list is both incomplete and partly invented: ENOTSUP=45 EOPNOTSUPP=102 distinct here, so both are needed /dev/null F_FULLFSYNC -> ENODEV(19), while fsync succeeds pipe/socket F_FULLFSYNC -> EBADF(9) EINVAL never came from F_FULLFSYNC at all; it came from fsync So ENODEV -- a real refusal, on a path anyone can reproduce -- would have raised instead of falling back, and EINVAL was in the list by analogy rather than evidence. What a network mount answers is not knowable from here, which makes the whole list a guess that fails closed on whatever it missed. This codebase does not classify driver errors by guessing, and this was that. Fall through on any failure instead. It is not a suppression: `fsync` runs on the same descriptor and raises in its turn, so a flush that cannot happen still fails the save. What the fallback gives up is the drive-cache flush -- the guarantee this had before `F_FULLFSYNC` was asked for at all. That is also what SQLite does with this same call, for the same reason. The test drives it through `/dev/null`, which refuses with ENODEV and accepts `fsync`; it fails against the allowlist and passes without it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

`F_FULLFSYNC` failing fell through to `fsync`, on the reasoning that a refusal is a statement about the filesystem rather than a write failure. That reasoning does not survive asking what `fsync` alone actually buys on Darwin. Against a process or kernel crash it is enough: the data has left the OS for the drive before the rename is issued, so the rename dies in the page cache and the old index stands. Against power loss it is not. The data sits in the drive's volatile cache, the rename's metadata joins it moments later, and nothing orders them -- and the rename is a few bytes against an index of megabytes, so a drive flushing as it pleases can easily put the new name on media while the bytes behind it are still queued. That is the torn publication this module exists to prevent, in precisely the scenario its docstring is about. So the fallback answered a request for ordering with a flush that does not provide it, and said nothing. A filesystem that cannot order data ahead of a rename is not one to publish an index onto; raise, and let the operator point `index_directory` at storage that can. This is also the simpler code. Refusal and failure now take the same path, so no errno is inspected -- there is no line to draw and no list to get wrong, which is what the previous two revisions kept getting wrong in opposite directions. The `/dev/null` test went with the fallback it pinned. The earlier defence of falling back rested on network and FUSE mounts being a realistic home for an index directory. They are not. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL

…emMachine#1588) * Atomically swap vector search engine index files on save SQLiteVectorStore persists each collection's index by calling the search engine's save(), which wrote directly to the final path. A crash mid-write left a truncated/corrupt file. Because index_saved=True makes the on-disk index a durable contract (missing/corrupt is a hard IndexLoadError, not a silent empty rebuild), an interrupted save could render a collection unrecoverable. Write the index to a sibling temp file and swap it into place with os.replace (atomic on POSIX and Windows on the same filesystem), so a reader sees either the old or new index, never a partial write; a failed save leaves the previous index intact. Leftover temp files are cleared on load so a crash does not leak them across restarts. Implemented in the engines (shared index_persistence helper) rather than in SQLiteVectorStore/SQLiteVectorStoreCollection, since the index save location and number of files written differ across engine implementations. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Make the index swap durable, not only atomic The swap protects a reader from a torn index, but the vector store also trims its pending-operation log once `save` returns -- and that log is the only other copy of those vectors, since the records table stores no vector column. So the swap reaching disk is load-bearing rather than a bonus: - fsync the parent directory after the replace, since POSIX `rename(2)` leaves the new directory entry in the page cache. Best-effort and ignored on failure, matching SQLite's `unixSync`; a no-op on Windows, which has no equivalent operation. - stop swallowing a failed fsync of the temp file. SQLite draws the same line -- a file fsync failure raises SQLITE_IOERR_FSYNC while a directory fsync failure is ignored -- and `EIO` means the writeback already failed and the dirty pages were dropped, which is exactly when the save must not be reported as committed. The existing cleanup then leaves the previous index in place with the log untrimmed, so the next save retries. - use F_FULLFSYNC on macOS, where plain `fsync` leaves the data in the drive's volatile write cache, falling back when a filesystem refuses it. State the resulting obligation on `VectorSearchEngine.save` itself, since that is what the store now relies on: replace atomically, then make the replacement as durable as the platform allows. An engine whose backend already implements the whole protocol can delegate to it and skip these helpers; the rest use `atomic_index_write`. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Let the engine own index durability, not the vector store The pending log holds the only durable copy of a vector between checkpoints -- the records table has no vector column -- so trimming it is safe only at an instant when the index provably holds those vectors. The temp-write + rename protocol this PR shipped could not provide that instant. A rename changes a directory entry, and Windows exposes no way to flush one: os.fsync is _commit, which is FlushFileBuffers, which is for file data, and you cannot open a directory to fsync it. The decisive evidence is SQLite's own -- it threads a directory-sync flag through every commit-relevant directory operation, honors it in unixDelete, and declares it /* Not used on win32 */ in winDelete. So os.replace could return, _save_collection_index could commit its trim durably behind it, and a power cut could still roll the rename back: records forward, index back, no copy of the difference left. MOVEFILE_WRITE_THROUGH is not a fix; its documented guarantee covers copy-and-delete (cross-volume) moves, not same-volume renames. Take SQLite's answer, which was not to harden the directory operation but to stop using one as a commit point (PERSIST commits by zeroing a header, TRUNCATE by truncating, WAL by appending frames). A base path now expands into two index slots plus a generation record each, created once and thereafter only overwritten. A checkpoint writes the index over the inactive slot and flushes it, then writes that slot's generation record and flushes that. The record is the commit, and it is a write into a file that already exists. It holds the generation and its bitwise complement, so a torn write reads as absent rather than as some other generation -- all or nothing without needing single-sector atomicity from the hardware. load takes the highest believable generation, and deliberately does not fall back to the older slot when the published index will not parse: the log was trimmed against the newer one, so the older is stale by exactly the ops that can no longer be replayed. Both backends already write straight to the path they are given, which is what this protocol wants -- verified that repeated saves preserve the inode and leave no stray files -- so no engine gains a temp file, a buffer, or a rename. Durability is entirely the engine's, including which artifact is live. The store keeps no slot pointer, manifest, or generation, so no schema change and no migration: what remains is one rule, never trim past what save says is durable, and _save_collection_index already had that order. index_path becomes index_base_path since it no longer names a file, and discarding a collection asks the engine layer which files that covers. BREAKING CHANGE: an index written by the previous protocol is not published under the new one, so a collection with index_saved=True raises IndexLoadError until its index directory is cleared and the records re-ingested. Anomaly tests walk every crash point in the publish sequence by constructing the on-disk state each would leave, plus one that pins the ordering itself (a failed index write must publish nothing) since state-based tests cannot observe it. Verified against three deliberate breaks -- dropping the complement check, writing the record first, and reusing one slot instead of alternating -- each caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Publish the index atomically, and stop promising durability The two-slot generation-record protocol bought a guarantee we have decided not to make: that a save survives a power failure. Every engine would have to implement and maintain that protocol, and the failure it buys out is bounded -- search recall for the records applied since the last checkpoint, repaired by re-ingesting them. The direction that actually costs, a published index that will not parse, is closed by the atomic swap on its own. So this returns to the temp-file-plus-rename publication and spends the difference on stating the contract instead of strengthening it: `save` publishes atomically, never durably; the store trims the pending log behind a publication a power failure can revert; a record whose vector is lost that way still resolves by uuid, is absent from search until it is upserted again, and nothing here detects the gap for the caller. Reverts the durability and engine-owned-publication commits, keeps the atomic swap, and adds a store-level test that reconstructs a reverted publication deterministically -- restore the previous index bytes after the trim -- to pin the direction it fails in. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Report a lost embedding as lost, not as a missing feature `update_feature` reads the stored embedding back when a caller updates a feature without supplying one, and that is the only place in the server that depends on the index still holding a vector. With publication now atomic rather than durable, a power failure can leave a feature whose row is intact and whose vector is not -- a state this path reported as "Vector record not found", which points the caller at the wrong thing and hides the repair. Split the two cases. A record that is genuinely absent keeps the old message; a record whose embedding the index no longer holds says so and names the fix, which is to pass a fresh embedding. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Let a failed fsync fail the save, and cut the essay around it `_flush_to_disk` wrapped its fsync in `contextlib.suppress(OSError)` and called itself best-effort. A failed fsync is exactly the evidence that the bytes are not safe to publish -- on Linux an EIO from fsync means writeback failed, reported once and then cleared -- so swallowing it and renaming anyway published a file we had positive evidence was bad. The safeguard cost something and, in the one case it existed for, guaranteed nothing. Nothing tested it either. Let it propagate. `atomic_index_write` already unlinks the temp and re-raises, so a failed flush now leaves the previously published index standing, which is the correct outcome. A test pins that. The fsync is not best-effort, and the docstring should not have said so: it rules out a class rather than narrowing a window. Because the flush completes before the rename is issued, and a durable write does not un-happen, the new name can never appear over incomplete bytes. What the missing directory fsync costs is the other direction -- the rename may not survive, so the publish reverts -- and that is the benign one this store already accepts. The module docstring was 76 lines against 49 of everything else, most of it argument rather than documentation: a walk through SQLite's `unixDelete` / `winDelete` sync-flag handling, and a rejected two-slot commit protocol. That is the PR's case for the design, not something to re-read every time someone opens a 20-line module, and the PR body carries it. What a reader here needs is the guarantee, the non-guarantee, and the cost. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Say why the swap is a rename, and what the reopened fd cannot see Two things the module was silent on. Why a rename at all. The stronger answer is to put the commit point inside the file, where an fsync reaches it portably -- SQLite never renames, and commits by truncating or zeroing its rollback journal, or in WAL mode by appending frames whose checksums make a torn tail self-identifying. Both need the writer to own the file format. A search engine owns its own and exposes `save(path)`, so above that call a rename is the only atomicity primitive left, and an engine whose format already commits that way needs none of this. Worth saying, because "why not do the better thing" is the first question the module invites. What the reopened descriptor cannot see. Flushing is fine on a fresh fd -- dirty pages belong to the file, not to the descriptor that dirtied them -- but error reporting is not: Linux hands a writeback error to descriptors open when it was recorded, so one recorded between the engine's close and this open is never reported and the save proceeds on bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. It cannot be closed from here: the engine writes through its own descriptor and closes it before returning, and closing the window needs an engine that writes through a handle the caller supplies. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Fsync the index on a descriptor that predates the write The fsync was on a descriptor opened after the engine had written and closed its own, which flushes correctly -- dirty pages belong to the file, not to the descriptor that dirtied them -- but reports nothing useful. Linux samples the writeback error sequence when a file is opened, so a descriptor opened after an error was recorded never learns of it: the fsync returns success and the save publishes bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. Open the temp before yielding it and hold it across the caller's write, so the descriptor predates the bytes and any error from writing them is reported here, where it fails the save. That assumes the caller writes in place. Both engines do -- verified: the inode is unchanged across `save_index` and `save`, and the held descriptor sees the written size -- but it is their behaviour, not their contract. An engine that built a file of its own and renamed it over the temp would leave this descriptor on an orphaned inode, and the fsync would report on a file nobody is about to publish. So it is checked before the fsync, and a mismatch fails the save rather than passing it quietly. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * State the in-place rule where the caller reads it The descriptor held across the body only flushes what the body wrote if the body writes the yielded path in place, and that requirement was recorded in `_flush_to_disk` -- a private function nobody writing an engine opens. It belongs on `atomic_index_write`, which is the API they use, alongside what happens when it is broken: an `OSError` and no publication, so the mistake surfaces at the first save rather than at a power cut. `_flush_to_disk` keeps the mechanism -- why the descriptor has to predate the write -- and now points at the rule instead of restating it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Ask the drive to flush on macOS, where fsync does not `os.fsync` is not the same guarantee on all three platforms this ships to. On Linux it flushes to the device, and on Windows `FlushFileBuffers` does the same. Darwin's `fsync` explicitly does not: it returns once the data reaches the drive, which may hold it in a volatile write cache. So on macOS the ordering this module is built on -- data durable before the rename is issued -- did not hold at the device, which is exactly the case it claims to rule out. `F_FULLFSYNC` asks the drive to flush that cache. Filesystems that cannot refuse it, and there `fsync` is the most that can be asked, so a refusal falls back; any other error is a write failure and propagates, as before. The flush-failure test patched `os.fsync`, which Darwin no longer reaches. It patches the module's own `_fsync` instead, which every platform does. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Stop guessing which errnos mean "this drive cannot do that" The fallback from `F_FULLFSYNC` to `fsync` was gated on an errno allowlist -- ENOTSUP, EOPNOTSUPP, EINVAL -- so that a genuine write failure would propagate rather than be quietly downgraded. Probing the actual returns on Darwin shows the list is both incomplete and partly invented: ENOTSUP=45 EOPNOTSUPP=102 distinct here, so both are needed /dev/null F_FULLFSYNC -> ENODEV(19), while fsync succeeds pipe/socket F_FULLFSYNC -> EBADF(9) EINVAL never came from F_FULLFSYNC at all; it came from fsync So ENODEV -- a real refusal, on a path anyone can reproduce -- would have raised instead of falling back, and EINVAL was in the list by analogy rather than evidence. What a network mount answers is not knowable from here, which makes the whole list a guess that fails closed on whatever it missed. This codebase does not classify driver errors by guessing, and this was that. Fall through on any failure instead. It is not a suppression: `fsync` runs on the same descriptor and raises in its turn, so a flush that cannot happen still fails the save. What the fallback gives up is the drive-cache flush -- the guarantee this had before `F_FULLFSYNC` was asked for at all. That is also what SQLite does with this same call, for the same reason. The test drives it through `/dev/null`, which refuses with ENODEV and accepts `fsync`; it fails against the allowlist and passes without it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Fail the save when the drive cannot be told to flush `F_FULLFSYNC` failing fell through to `fsync`, on the reasoning that a refusal is a statement about the filesystem rather than a write failure. That reasoning does not survive asking what `fsync` alone actually buys on Darwin. Against a process or kernel crash it is enough: the data has left the OS for the drive before the rename is issued, so the rename dies in the page cache and the old index stands. Against power loss it is not. The data sits in the drive's volatile cache, the rename's metadata joins it moments later, and nothing orders them -- and the rename is a few bytes against an index of megabytes, so a drive flushing as it pleases can easily put the new name on media while the bytes behind it are still queued. That is the torn publication this module exists to prevent, in precisely the scenario its docstring is about. So the fallback answered a request for ordering with a flush that does not provide it, and said nothing. A filesystem that cannot order data ahead of a rename is not one to publish an index onto; raise, and let the operator point `index_directory` at storage that can. This is also the simpler code. Refusal and failure now take the same path, so no errno is inspected -- there is no line to draw and no list to get wrong, which is what the previous two revisions kept getting wrong in opposite directions. The `/dev/null` test went with the fallback it pinned. The earlier defence of falling back rested on network and FUSE mounts being a realistic home for an index directory. They are not. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

…emMachine#1588) * Atomically swap vector search engine index files on save SQLiteVectorStore persists each collection's index by calling the search engine's save(), which wrote directly to the final path. A crash mid-write left a truncated/corrupt file. Because index_saved=True makes the on-disk index a durable contract (missing/corrupt is a hard IndexLoadError, not a silent empty rebuild), an interrupted save could render a collection unrecoverable. Write the index to a sibling temp file and swap it into place with os.replace (atomic on POSIX and Windows on the same filesystem), so a reader sees either the old or new index, never a partial write; a failed save leaves the previous index intact. Leftover temp files are cleared on load so a crash does not leak them across restarts. Implemented in the engines (shared index_persistence helper) rather than in SQLiteVectorStore/SQLiteVectorStoreCollection, since the index save location and number of files written differ across engine implementations. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Make the index swap durable, not only atomic The swap protects a reader from a torn index, but the vector store also trims its pending-operation log once `save` returns -- and that log is the only other copy of those vectors, since the records table stores no vector column. So the swap reaching disk is load-bearing rather than a bonus: - fsync the parent directory after the replace, since POSIX `rename(2)` leaves the new directory entry in the page cache. Best-effort and ignored on failure, matching SQLite's `unixSync`; a no-op on Windows, which has no equivalent operation. - stop swallowing a failed fsync of the temp file. SQLite draws the same line -- a file fsync failure raises SQLITE_IOERR_FSYNC while a directory fsync failure is ignored -- and `EIO` means the writeback already failed and the dirty pages were dropped, which is exactly when the save must not be reported as committed. The existing cleanup then leaves the previous index in place with the log untrimmed, so the next save retries. - use F_FULLFSYNC on macOS, where plain `fsync` leaves the data in the drive's volatile write cache, falling back when a filesystem refuses it. State the resulting obligation on `VectorSearchEngine.save` itself, since that is what the store now relies on: replace atomically, then make the replacement as durable as the platform allows. An engine whose backend already implements the whole protocol can delegate to it and skip these helpers; the rest use `atomic_index_write`. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Let the engine own index durability, not the vector store The pending log holds the only durable copy of a vector between checkpoints -- the records table has no vector column -- so trimming it is safe only at an instant when the index provably holds those vectors. The temp-write + rename protocol this PR shipped could not provide that instant. A rename changes a directory entry, and Windows exposes no way to flush one: os.fsync is _commit, which is FlushFileBuffers, which is for file data, and you cannot open a directory to fsync it. The decisive evidence is SQLite's own -- it threads a directory-sync flag through every commit-relevant directory operation, honors it in unixDelete, and declares it /* Not used on win32 */ in winDelete. So os.replace could return, _save_collection_index could commit its trim durably behind it, and a power cut could still roll the rename back: records forward, index back, no copy of the difference left. MOVEFILE_WRITE_THROUGH is not a fix; its documented guarantee covers copy-and-delete (cross-volume) moves, not same-volume renames. Take SQLite's answer, which was not to harden the directory operation but to stop using one as a commit point (PERSIST commits by zeroing a header, TRUNCATE by truncating, WAL by appending frames). A base path now expands into two index slots plus a generation record each, created once and thereafter only overwritten. A checkpoint writes the index over the inactive slot and flushes it, then writes that slot's generation record and flushes that. The record is the commit, and it is a write into a file that already exists. It holds the generation and its bitwise complement, so a torn write reads as absent rather than as some other generation -- all or nothing without needing single-sector atomicity from the hardware. load takes the highest believable generation, and deliberately does not fall back to the older slot when the published index will not parse: the log was trimmed against the newer one, so the older is stale by exactly the ops that can no longer be replayed. Both backends already write straight to the path they are given, which is what this protocol wants -- verified that repeated saves preserve the inode and leave no stray files -- so no engine gains a temp file, a buffer, or a rename. Durability is entirely the engine's, including which artifact is live. The store keeps no slot pointer, manifest, or generation, so no schema change and no migration: what remains is one rule, never trim past what save says is durable, and _save_collection_index already had that order. index_path becomes index_base_path since it no longer names a file, and discarding a collection asks the engine layer which files that covers. BREAKING CHANGE: an index written by the previous protocol is not published under the new one, so a collection with index_saved=True raises IndexLoadError until its index directory is cleared and the records re-ingested. Anomaly tests walk every crash point in the publish sequence by constructing the on-disk state each would leave, plus one that pins the ordering itself (a failed index write must publish nothing) since state-based tests cannot observe it. Verified against three deliberate breaks -- dropping the complement check, writing the record first, and reusing one slot instead of alternating -- each caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Publish the index atomically, and stop promising durability The two-slot generation-record protocol bought a guarantee we have decided not to make: that a save survives a power failure. Every engine would have to implement and maintain that protocol, and the failure it buys out is bounded -- search recall for the records applied since the last checkpoint, repaired by re-ingesting them. The direction that actually costs, a published index that will not parse, is closed by the atomic swap on its own. So this returns to the temp-file-plus-rename publication and spends the difference on stating the contract instead of strengthening it: `save` publishes atomically, never durably; the store trims the pending log behind a publication a power failure can revert; a record whose vector is lost that way still resolves by uuid, is absent from search until it is upserted again, and nothing here detects the gap for the caller. Reverts the durability and engine-owned-publication commits, keeps the atomic swap, and adds a store-level test that reconstructs a reverted publication deterministically -- restore the previous index bytes after the trim -- to pin the direction it fails in. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Report a lost embedding as lost, not as a missing feature `update_feature` reads the stored embedding back when a caller updates a feature without supplying one, and that is the only place in the server that depends on the index still holding a vector. With publication now atomic rather than durable, a power failure can leave a feature whose row is intact and whose vector is not -- a state this path reported as "Vector record not found", which points the caller at the wrong thing and hides the repair. Split the two cases. A record that is genuinely absent keeps the old message; a record whose embedding the index no longer holds says so and names the fix, which is to pass a fresh embedding. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Let a failed fsync fail the save, and cut the essay around it `_flush_to_disk` wrapped its fsync in `contextlib.suppress(OSError)` and called itself best-effort. A failed fsync is exactly the evidence that the bytes are not safe to publish -- on Linux an EIO from fsync means writeback failed, reported once and then cleared -- so swallowing it and renaming anyway published a file we had positive evidence was bad. The safeguard cost something and, in the one case it existed for, guaranteed nothing. Nothing tested it either. Let it propagate. `atomic_index_write` already unlinks the temp and re-raises, so a failed flush now leaves the previously published index standing, which is the correct outcome. A test pins that. The fsync is not best-effort, and the docstring should not have said so: it rules out a class rather than narrowing a window. Because the flush completes before the rename is issued, and a durable write does not un-happen, the new name can never appear over incomplete bytes. What the missing directory fsync costs is the other direction -- the rename may not survive, so the publish reverts -- and that is the benign one this store already accepts. The module docstring was 76 lines against 49 of everything else, most of it argument rather than documentation: a walk through SQLite's `unixDelete` / `winDelete` sync-flag handling, and a rejected two-slot commit protocol. That is the PR's case for the design, not something to re-read every time someone opens a 20-line module, and the PR body carries it. What a reader here needs is the guarantee, the non-guarantee, and the cost. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Say why the swap is a rename, and what the reopened fd cannot see Two things the module was silent on. Why a rename at all. The stronger answer is to put the commit point inside the file, where an fsync reaches it portably -- SQLite never renames, and commits by truncating or zeroing its rollback journal, or in WAL mode by appending frames whose checksums make a torn tail self-identifying. Both need the writer to own the file format. A search engine owns its own and exposes `save(path)`, so above that call a rename is the only atomicity primitive left, and an engine whose format already commits that way needs none of this. Worth saying, because "why not do the better thing" is the first question the module invites. What the reopened descriptor cannot see. Flushing is fine on a fresh fd -- dirty pages belong to the file, not to the descriptor that dirtied them -- but error reporting is not: Linux hands a writeback error to descriptors open when it was recorded, so one recorded between the engine's close and this open is never reported and the save proceeds on bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. It cannot be closed from here: the engine writes through its own descriptor and closes it before returning, and closing the window needs an engine that writes through a handle the caller supplies. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Fsync the index on a descriptor that predates the write The fsync was on a descriptor opened after the engine had written and closed its own, which flushes correctly -- dirty pages belong to the file, not to the descriptor that dirtied them -- but reports nothing useful. Linux samples the writeback error sequence when a file is opened, so a descriptor opened after an error was recorded never learns of it: the fsync returns success and the save publishes bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. Open the temp before yielding it and hold it across the caller's write, so the descriptor predates the bytes and any error from writing them is reported here, where it fails the save. That assumes the caller writes in place. Both engines do -- verified: the inode is unchanged across `save_index` and `save`, and the held descriptor sees the written size -- but it is their behaviour, not their contract. An engine that built a file of its own and renamed it over the temp would leave this descriptor on an orphaned inode, and the fsync would report on a file nobody is about to publish. So it is checked before the fsync, and a mismatch fails the save rather than passing it quietly. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * State the in-place rule where the caller reads it The descriptor held across the body only flushes what the body wrote if the body writes the yielded path in place, and that requirement was recorded in `_flush_to_disk` -- a private function nobody writing an engine opens. It belongs on `atomic_index_write`, which is the API they use, alongside what happens when it is broken: an `OSError` and no publication, so the mistake surfaces at the first save rather than at a power cut. `_flush_to_disk` keeps the mechanism -- why the descriptor has to predate the write -- and now points at the rule instead of restating it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Ask the drive to flush on macOS, where fsync does not `os.fsync` is not the same guarantee on all three platforms this ships to. On Linux it flushes to the device, and on Windows `FlushFileBuffers` does the same. Darwin's `fsync` explicitly does not: it returns once the data reaches the drive, which may hold it in a volatile write cache. So on macOS the ordering this module is built on -- data durable before the rename is issued -- did not hold at the device, which is exactly the case it claims to rule out. `F_FULLFSYNC` asks the drive to flush that cache. Filesystems that cannot refuse it, and there `fsync` is the most that can be asked, so a refusal falls back; any other error is a write failure and propagates, as before. The flush-failure test patched `os.fsync`, which Darwin no longer reaches. It patches the module's own `_fsync` instead, which every platform does. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Stop guessing which errnos mean "this drive cannot do that" The fallback from `F_FULLFSYNC` to `fsync` was gated on an errno allowlist -- ENOTSUP, EOPNOTSUPP, EINVAL -- so that a genuine write failure would propagate rather than be quietly downgraded. Probing the actual returns on Darwin shows the list is both incomplete and partly invented: ENOTSUP=45 EOPNOTSUPP=102 distinct here, so both are needed /dev/null F_FULLFSYNC -> ENODEV(19), while fsync succeeds pipe/socket F_FULLFSYNC -> EBADF(9) EINVAL never came from F_FULLFSYNC at all; it came from fsync So ENODEV -- a real refusal, on a path anyone can reproduce -- would have raised instead of falling back, and EINVAL was in the list by analogy rather than evidence. What a network mount answers is not knowable from here, which makes the whole list a guess that fails closed on whatever it missed. This codebase does not classify driver errors by guessing, and this was that. Fall through on any failure instead. It is not a suppression: `fsync` runs on the same descriptor and raises in its turn, so a flush that cannot happen still fails the save. What the fallback gives up is the drive-cache flush -- the guarantee this had before `F_FULLFSYNC` was asked for at all. That is also what SQLite does with this same call, for the same reason. The test drives it through `/dev/null`, which refuses with ENODEV and accepts `fsync`; it fails against the allowlist and passes without it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Fail the save when the drive cannot be told to flush `F_FULLFSYNC` failing fell through to `fsync`, on the reasoning that a refusal is a statement about the filesystem rather than a write failure. That reasoning does not survive asking what `fsync` alone actually buys on Darwin. Against a process or kernel crash it is enough: the data has left the OS for the drive before the rename is issued, so the rename dies in the page cache and the old index stands. Against power loss it is not. The data sits in the drive's volatile cache, the rename's metadata joins it moments later, and nothing orders them -- and the rename is a few bytes against an index of megabytes, so a drive flushing as it pleases can easily put the new name on media while the bytes behind it are still queued. That is the torn publication this module exists to prevent, in precisely the scenario its docstring is about. So the fallback answered a request for ordering with a flush that does not provide it, and said nothing. A filesystem that cannot order data ahead of a rename is not one to publish an index onto; raise, and let the operator point `index_directory` at storage that can. This is also the simpler code. Refusal and failure now take the same path, so no errno is inspected -- there is no line to draw and no list to get wrong, which is what the previous two revisions kept getting wrong in opposite directions. The `/dev/null` test went with the fallback it pinned. The earlier defence of falling back rested on network and FUSE mounts being a realistic home for an index directory. They are not. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit c4b6c4d)

…emMachine#1588) * Atomically swap vector search engine index files on save SQLiteVectorStore persists each collection's index by calling the search engine's save(), which wrote directly to the final path. A crash mid-write left a truncated/corrupt file. Because index_saved=True makes the on-disk index a durable contract (missing/corrupt is a hard IndexLoadError, not a silent empty rebuild), an interrupted save could render a collection unrecoverable. Write the index to a sibling temp file and swap it into place with os.replace (atomic on POSIX and Windows on the same filesystem), so a reader sees either the old or new index, never a partial write; a failed save leaves the previous index intact. Leftover temp files are cleared on load so a crash does not leak them across restarts. Implemented in the engines (shared index_persistence helper) rather than in SQLiteVectorStore/SQLiteVectorStoreCollection, since the index save location and number of files written differ across engine implementations. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Make the index swap durable, not only atomic The swap protects a reader from a torn index, but the vector store also trims its pending-operation log once `save` returns -- and that log is the only other copy of those vectors, since the records table stores no vector column. So the swap reaching disk is load-bearing rather than a bonus: - fsync the parent directory after the replace, since POSIX `rename(2)` leaves the new directory entry in the page cache. Best-effort and ignored on failure, matching SQLite's `unixSync`; a no-op on Windows, which has no equivalent operation. - stop swallowing a failed fsync of the temp file. SQLite draws the same line -- a file fsync failure raises SQLITE_IOERR_FSYNC while a directory fsync failure is ignored -- and `EIO` means the writeback already failed and the dirty pages were dropped, which is exactly when the save must not be reported as committed. The existing cleanup then leaves the previous index in place with the log untrimmed, so the next save retries. - use F_FULLFSYNC on macOS, where plain `fsync` leaves the data in the drive's volatile write cache, falling back when a filesystem refuses it. State the resulting obligation on `VectorSearchEngine.save` itself, since that is what the store now relies on: replace atomically, then make the replacement as durable as the platform allows. An engine whose backend already implements the whole protocol can delegate to it and skip these helpers; the rest use `atomic_index_write`. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Let the engine own index durability, not the vector store The pending log holds the only durable copy of a vector between checkpoints -- the records table has no vector column -- so trimming it is safe only at an instant when the index provably holds those vectors. The temp-write + rename protocol this PR shipped could not provide that instant. A rename changes a directory entry, and Windows exposes no way to flush one: os.fsync is _commit, which is FlushFileBuffers, which is for file data, and you cannot open a directory to fsync it. The decisive evidence is SQLite's own -- it threads a directory-sync flag through every commit-relevant directory operation, honors it in unixDelete, and declares it /* Not used on win32 */ in winDelete. So os.replace could return, _save_collection_index could commit its trim durably behind it, and a power cut could still roll the rename back: records forward, index back, no copy of the difference left. MOVEFILE_WRITE_THROUGH is not a fix; its documented guarantee covers copy-and-delete (cross-volume) moves, not same-volume renames. Take SQLite's answer, which was not to harden the directory operation but to stop using one as a commit point (PERSIST commits by zeroing a header, TRUNCATE by truncating, WAL by appending frames). A base path now expands into two index slots plus a generation record each, created once and thereafter only overwritten. A checkpoint writes the index over the inactive slot and flushes it, then writes that slot's generation record and flushes that. The record is the commit, and it is a write into a file that already exists. It holds the generation and its bitwise complement, so a torn write reads as absent rather than as some other generation -- all or nothing without needing single-sector atomicity from the hardware. load takes the highest believable generation, and deliberately does not fall back to the older slot when the published index will not parse: the log was trimmed against the newer one, so the older is stale by exactly the ops that can no longer be replayed. Both backends already write straight to the path they are given, which is what this protocol wants -- verified that repeated saves preserve the inode and leave no stray files -- so no engine gains a temp file, a buffer, or a rename. Durability is entirely the engine's, including which artifact is live. The store keeps no slot pointer, manifest, or generation, so no schema change and no migration: what remains is one rule, never trim past what save says is durable, and _save_collection_index already had that order. index_path becomes index_base_path since it no longer names a file, and discarding a collection asks the engine layer which files that covers. BREAKING CHANGE: an index written by the previous protocol is not published under the new one, so a collection with index_saved=True raises IndexLoadError until its index directory is cleared and the records re-ingested. Anomaly tests walk every crash point in the publish sequence by constructing the on-disk state each would leave, plus one that pins the ordering itself (a failed index write must publish nothing) since state-based tests cannot observe it. Verified against three deliberate breaks -- dropping the complement check, writing the record first, and reusing one slot instead of alternating -- each caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Publish the index atomically, and stop promising durability The two-slot generation-record protocol bought a guarantee we have decided not to make: that a save survives a power failure. Every engine would have to implement and maintain that protocol, and the failure it buys out is bounded -- search recall for the records applied since the last checkpoint, repaired by re-ingesting them. The direction that actually costs, a published index that will not parse, is closed by the atomic swap on its own. So this returns to the temp-file-plus-rename publication and spends the difference on stating the contract instead of strengthening it: `save` publishes atomically, never durably; the store trims the pending log behind a publication a power failure can revert; a record whose vector is lost that way still resolves by uuid, is absent from search until it is upserted again, and nothing here detects the gap for the caller. Reverts the durability and engine-owned-publication commits, keeps the atomic swap, and adds a store-level test that reconstructs a reverted publication deterministically -- restore the previous index bytes after the trim -- to pin the direction it fails in. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Report a lost embedding as lost, not as a missing feature `update_feature` reads the stored embedding back when a caller updates a feature without supplying one, and that is the only place in the server that depends on the index still holding a vector. With publication now atomic rather than durable, a power failure can leave a feature whose row is intact and whose vector is not -- a state this path reported as "Vector record not found", which points the caller at the wrong thing and hides the repair. Split the two cases. A record that is genuinely absent keeps the old message; a record whose embedding the index no longer holds says so and names the fix, which is to pass a fresh embedding. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * Let a failed fsync fail the save, and cut the essay around it `_flush_to_disk` wrapped its fsync in `contextlib.suppress(OSError)` and called itself best-effort. A failed fsync is exactly the evidence that the bytes are not safe to publish -- on Linux an EIO from fsync means writeback failed, reported once and then cleared -- so swallowing it and renaming anyway published a file we had positive evidence was bad. The safeguard cost something and, in the one case it existed for, guaranteed nothing. Nothing tested it either. Let it propagate. `atomic_index_write` already unlinks the temp and re-raises, so a failed flush now leaves the previously published index standing, which is the correct outcome. A test pins that. The fsync is not best-effort, and the docstring should not have said so: it rules out a class rather than narrowing a window. Because the flush completes before the rename is issued, and a durable write does not un-happen, the new name can never appear over incomplete bytes. What the missing directory fsync costs is the other direction -- the rename may not survive, so the publish reverts -- and that is the benign one this store already accepts. The module docstring was 76 lines against 49 of everything else, most of it argument rather than documentation: a walk through SQLite's `unixDelete` / `winDelete` sync-flag handling, and a rejected two-slot commit protocol. That is the PR's case for the design, not something to re-read every time someone opens a 20-line module, and the PR body carries it. What a reader here needs is the guarantee, the non-guarantee, and the cost. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Say why the swap is a rename, and what the reopened fd cannot see Two things the module was silent on. Why a rename at all. The stronger answer is to put the commit point inside the file, where an fsync reaches it portably -- SQLite never renames, and commits by truncating or zeroing its rollback journal, or in WAL mode by appending frames whose checksums make a torn tail self-identifying. Both need the writer to own the file format. A search engine owns its own and exposes `save(path)`, so above that call a rename is the only atomicity primitive left, and an engine whose format already commits that way needs none of this. Worth saying, because "why not do the better thing" is the first question the module invites. What the reopened descriptor cannot see. Flushing is fine on a fresh fd -- dirty pages belong to the file, not to the descriptor that dirtied them -- but error reporting is not: Linux hands a writeback error to descriptors open when it was recorded, so one recorded between the engine's close and this open is never reported and the save proceeds on bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. It cannot be closed from here: the engine writes through its own descriptor and closes it before returning, and closing the window needs an engine that writes through a handle the caller supplies. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Fsync the index on a descriptor that predates the write The fsync was on a descriptor opened after the engine had written and closed its own, which flushes correctly -- dirty pages belong to the file, not to the descriptor that dirtied them -- but reports nothing useful. Linux samples the writeback error sequence when a file is opened, so a descriptor opened after an error was recorded never learns of it: the fsync returns success and the save publishes bytes already known bad. Same shape as the 2018 PostgreSQL fsync report. Open the temp before yielding it and hold it across the caller's write, so the descriptor predates the bytes and any error from writing them is reported here, where it fails the save. That assumes the caller writes in place. Both engines do -- verified: the inode is unchanged across `save_index` and `save`, and the held descriptor sees the written size -- but it is their behaviour, not their contract. An engine that built a file of its own and renamed it over the temp would leave this descriptor on an orphaned inode, and the fsync would report on a file nobody is about to publish. So it is checked before the fsync, and a mismatch fails the save rather than passing it quietly. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * State the in-place rule where the caller reads it The descriptor held across the body only flushes what the body wrote if the body writes the yielded path in place, and that requirement was recorded in `_flush_to_disk` -- a private function nobody writing an engine opens. It belongs on `atomic_index_write`, which is the API they use, alongside what happens when it is broken: an `OSError` and no publication, so the mistake surfaces at the first save rather than at a power cut. `_flush_to_disk` keeps the mechanism -- why the descriptor has to predate the write -- and now points at the rule instead of restating it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Ask the drive to flush on macOS, where fsync does not `os.fsync` is not the same guarantee on all three platforms this ships to. On Linux it flushes to the device, and on Windows `FlushFileBuffers` does the same. Darwin's `fsync` explicitly does not: it returns once the data reaches the drive, which may hold it in a volatile write cache. So on macOS the ordering this module is built on -- data durable before the rename is issued -- did not hold at the device, which is exactly the case it claims to rule out. `F_FULLFSYNC` asks the drive to flush that cache. Filesystems that cannot refuse it, and there `fsync` is the most that can be asked, so a refusal falls back; any other error is a write failure and propagates, as before. The flush-failure test patched `os.fsync`, which Darwin no longer reaches. It patches the module's own `_fsync` instead, which every platform does. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Stop guessing which errnos mean "this drive cannot do that" The fallback from `F_FULLFSYNC` to `fsync` was gated on an errno allowlist -- ENOTSUP, EOPNOTSUPP, EINVAL -- so that a genuine write failure would propagate rather than be quietly downgraded. Probing the actual returns on Darwin shows the list is both incomplete and partly invented: ENOTSUP=45 EOPNOTSUPP=102 distinct here, so both are needed /dev/null F_FULLFSYNC -> ENODEV(19), while fsync succeeds pipe/socket F_FULLFSYNC -> EBADF(9) EINVAL never came from F_FULLFSYNC at all; it came from fsync So ENODEV -- a real refusal, on a path anyone can reproduce -- would have raised instead of falling back, and EINVAL was in the list by analogy rather than evidence. What a network mount answers is not knowable from here, which makes the whole list a guess that fails closed on whatever it missed. This codebase does not classify driver errors by guessing, and this was that. Fall through on any failure instead. It is not a suppression: `fsync` runs on the same descriptor and raises in its turn, so a flush that cannot happen still fails the save. What the fallback gives up is the drive-cache flush -- the guarantee this had before `F_FULLFSYNC` was asked for at all. That is also what SQLite does with this same call, for the same reason. The test drives it through `/dev/null`, which refuses with ENODEV and accepts `fsync`; it fails against the allowlist and passes without it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL * Fail the save when the drive cannot be told to flush `F_FULLFSYNC` failing fell through to `fsync`, on the reasoning that a refusal is a statement about the filesystem rather than a write failure. That reasoning does not survive asking what `fsync` alone actually buys on Darwin. Against a process or kernel crash it is enough: the data has left the OS for the drive before the rename is issued, so the rename dies in the page cache and the old index stands. Against power loss it is not. The data sits in the drive's volatile cache, the rename's metadata joins it moments later, and nothing orders them -- and the rename is a few bytes against an index of megabytes, so a drive flushing as it pleases can easily put the new name on media while the bytes behind it are still queued. That is the torn publication this module exists to prevent, in precisely the scenario its docstring is about. So the fallback answered a request for ordering with a flush that does not provide it, and said nothing. A filesystem that cannot order data ahead of a rename is not one to publish an index onto; raise, and let the operator point `index_directory` at storage that can. This is also the simpler code. Refusal and failure now take the same path, so no errno is inspected -- there is no line to draw and no list to get wrong, which is what the previous two revisions kept getting wrong in opposite directions. The `/dev/null` test went with the fallback it pinned. The earlier defence of falling back rested on network and FUSE mounts being a realistic home for an index directory. They are not. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NKmF9xNph9QH3ozNw3ZnJL --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]> (cherry picked from commit 21105d6)

This was referenced Sep 8, 2026

Add VectorSearchEngine backed by turbovec, and take vector read-back out of the vector store contract (speedkick) #1591

Closed

Make cosine the only similarity, and add get_cosine_similarity (speedkick) #1593

Closed

edwinyyyu and others added 9 commits September 9, 2026 10:17

edwinyyyu force-pushed the fix/atomic-index-swap-speedkick branch from 1e98020 to 4502f42 Compare September 9, 2026 17:17

edwinyyyu and others added 3 commits September 9, 2026 10:21

edwinyyyu merged commit 21105d6 into MemMachine:speedkick Sep 9, 2026
30 of 39 checks passed

edwinyyyu mentioned this pull request Sep 9, 2026

Add session, source and expansion to EventMemory (speedkick) #1597

Closed

edwinyyyu mentioned this pull request Sep 17, 2026

Hand the Qdrant vector store a metrics factory, so its tracker emits (port of #1532) #1682

Merged

This was referenced Sep 18, 2026

[declared schema 1/2] Make a vector store filter only on the properties it declares #1628

Draft

[declared schema 2/2] Close the filter union, and make negation the complement on every backend #1616

Draft

edwinyyyu mentioned this pull request Sep 21, 2026

[user properties 1/2] Keep undeclared properties out of the vector store #1702

Open

Navigation Menu

Repository navigation

Publish vector index files atomically (but not durably) (speedkick) - #1588

Publish vector index files atomically (but not durably) (speedkick)#1588
edwinyyyu merged 12 commits into
MemMachine:speedkickfrom
edwinyyyu:fix/atomic-index-swap-speedkick

edwinyyyu commented Sep 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

edwinyyyu commented Sep 8, 2026

Problem

Fix

What a save does not promise

Why that gap stays open

Where the contract is stated

Tests

Changed from the earlier revision

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant