Repository navigation
Take SQLite's write lock at BEGIN, not at the first write (speedkick) - #1609
Merged
edwinyyyu merged 1 commit intoSep 11, 2026
Conversation
edwinyyyu
force-pushed
the
fix/sqlite-vector-store-begin-immediate-speedkick
branch
from
September 10, 2026 21:45
8fb0fa8 to
7d5e6f8
Compare
edwinyyyu
force-pushed
the
fix/sqlite-vector-store-begin-immediate-speedkick
branch
2 times, most recently
from
September 10, 2026 22:25
9f57c4b to
4541681
Compare
This was referenced Sep 10, 2026
edwinyyyu
force-pushed
the
fix/sqlite-vector-store-begin-immediate-speedkick
branch
7 times, most recently
from
September 11, 2026 00:21
a3becdc to
29b78ab
Compare
`delete` resolves row ids and then writes, in one transaction. Under
SQLite's default deferred BEGIN the write lock is taken at the first
write, so another writer can take it between that read and the write,
and the transaction that read first is the one that loses: its own write
cannot upgrade, and SQLite reports that immediately rather than waiting
out busy_timeout, because waiting could only deadlock. Reproduced in both
journal modes:
journal_mode BEGIN other writer our write
delete deferred shut out fails
delete IMMEDIATE shut out ok
wal deferred commits fails
wal IMMEDIATE shut out ok
The store now emits BEGIN explicitly and lets a transaction ask for
BEGIN IMMEDIATE, which every write path does. The mode is chosen per
transaction, not per engine: a hook that asked for IMMEDIATE
unconditionally would make every read take the write lock, and two
readers would then serialize against each other.
Two tests fail without it. One holds a competing lock across a
read-then-write transaction and asserts that transaction completes;
asserting instead that the other writer is excluded passes either way,
because a deferred BEGIN shuts it out too, later and by a different lock.
The other races four creates of one name: under a deferred BEGIN the
losers read no stored config, go on to CREATE TABLE, and fail there with
"table already exists" instead of VectorStoreCollectionAlreadyExistsError.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn
edwinyyyu
force-pushed
the
fix/sqlite-vector-store-begin-immediate-speedkick
branch
from
September 11, 2026 00:54
29b78ab to
5852fc3
Compare
This was referenced Sep 16, 2026
Merged
Closed
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 17, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
This was referenced Sep 17, 2026
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 17, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 17, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 17, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 18, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 18, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 21, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 21, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 25, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
This was referenced Oct 1, 2026
Draft
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Oct 10, 2026
…MemMachine#1609) Take SQLite's write lock at BEGIN, not at the first write `delete` resolves row ids and then writes, in one transaction. Under SQLite's default deferred BEGIN the write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting out busy_timeout, because waiting could only deadlock. Reproduced in both journal modes: journal_mode BEGIN other writer our write delete deferred shut out fails delete IMMEDIATE shut out ok wal deferred commits fails wal IMMEDIATE shut out ok The store now emits BEGIN explicitly and lets a transaction ask for BEGIN IMMEDIATE, which every write path does. The mode is chosen per transaction, not per engine: a hook that asked for IMMEDIATE unconditionally would make every read take the write lock, and two readers would then serialize against each other. Two tests fail without it. One holds a competing lock across a read-then-write transaction and asserts that transaction completes; asserting instead that the other writer is excluded passes either way, because a deferred BEGIN shuts it out too, later and by a different lock. The other races four creates of one name: under a deferred BEGIN the losers read no stored config, go on to CREATE TABLE, and fail there with "table already exists" instead of VectorStoreCollectionAlreadyExistsError. Claude-Session: https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn Co-authored-by: Claude Fable 5.1 <[email protected]>
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
1 participant
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.
Purpose of the change
deleteresolves row ids and then writes, in one transaction. Under SQLite's default deferredBEGINthe write lock is taken at the first write, so another writer can take it between that read and the write, and the transaction that read first is the one that loses: its own write cannot upgrade, and SQLite reports that immediately rather than waiting outbusy_timeout, because waiting could only deadlock.Measured in both journal modes, so this is not a WAL-only concern:
journal_modeBEGINdeletedeleteIMMEDIATEwalwalIMMEDIATEThe store now emits
BEGINexplicitly and lets a transaction ask forBEGIN IMMEDIATE, which every write path does. The mode is per transaction, not per engine: a hook that asked forIMMEDIATEunconditionally would make every read take the write lock, and two readers would then serialize against each other. This makes read-then-write atomic by construction rather than by every write transaction happening to write first.Tests
Two fail without it.
test_a_write_transaction_can_still_write_after_it_has_readholds a competing lock across a read-then-write transaction and asserts that transaction completes. Two earlier versions passed either way: asserting that the other writer is excluded passes because a deferredBEGINshuts it out too, later and by a different lock; letting the other writer's context manager exit before our write passes because rollback releases the lock first.test_racing_creates_admit_exactly_oneraces four creates of one name: under a deferredBEGINthe losers read no stored config, go on toCREATE TABLE, and fail there with SQLite's "table already exists" instead ofVectorStoreCollectionAlreadyExistsError.Verification
test_sqlite_vector_store.py: 90 passed. Against the replay-refusal PR's store, with_write_transactionstubbed as a plainsession.begin()so the file imports, the 2 tests above fail and the other 88 pass.Stacked on #1608, so the diff here includes #1612, #1607 and #1608 until they merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ESpWYTmCR7X3bJEpoA8SAn