Skip to content

fix(sqlite3): run BEGIN, COMMIT and ROLLBACK through exec so a failed COMMIT releases its lock - #362

Open
just-some-random-pal wants to merge 1 commit into
tursodatabase:mainfrom
just-some-random-pal:fix/commit-busy-keeps-lock
Open

just-some-random-pal wants to merge 1 commit into
tursodatabase:mainfrom
just-some-random-pal:fix/commit-busy-keeps-lock

Conversation

@just-some-random-pal

Copy link
Copy Markdown

Fixes #361.

On a local file database, a COMMIT that fails with SQLITE_BUSY leaves the database write-locked until the process exits. Every later write from any connection fails with SQLITE_BUSY, even after the failed transaction is rolled back and the reader that caused the busy error is gone. Through @prisma/adapter-libsql this shows up as every later query timing out.

The cause is the one the issue pins down in the libsql binding: transaction control went through db.prepare(sql).run(), and when a prepared COMMIT throws, the statement is left un-reset. An un-reset statement keeps the connection's lock past a successful ROLLBACK and past close(), until garbage collection finalizes it. db.exec() finalizes its statement before returning, whether it succeeded or failed.

This adds executeControl, which runs a statement through exec() and maps the error like executeStmt, and uses it for every BEGIN, COMMIT and ROLLBACK: in Sqlite3Transaction.commit(), rollback() and close(), in Sqlite3Client.transaction(), batch() and migrate(), and in the pool's rollback when a connection is released mid-transaction. 0.18.0's pool already rolls back and drops a connection whose rollback fails, so with the lock no longer stuck, that recovery now works. migrate() also returns its connection to the pool in every case; before, a throwing ROLLBACK in its finally skipped the release.

The libsql 0.6 incompatibility mentioned in the issue is separate and not touched here.

Tests: three cases next to the existing busy-timeout test, one each for a failed COMMIT in transaction().commit(), batch() and migrate(). Each holds a SHARED lock from a read transaction on a second client, lets the COMMIT fail with SQLITE_BUSY, releases the reader, and then writes from the same client and from a new one. All three fail on main with SQLITE_BUSY: database is locked and pass with this change. The local test suite passes (200 passed, 11 network tests skipped without a server), and tsc --noEmit and prettier --check are clean.

… COMMIT releases its lock

On a local file database, a COMMIT that failed with SQLITE_BUSY left the
database write-locked until the process exited: every later write from
any connection failed with SQLITE_BUSY, even after the failed
transaction was rolled back and the reader that caused the busy error
was gone. Through @prisma/adapter-libsql this surfaces as every query
timing out.

Transaction control went through db.prepare(sql).run(). When a prepared
COMMIT throws, libsql leaves the statement un-reset, and that statement
keeps the connection's lock past a successful ROLLBACK and past close(),
until garbage collection finalizes it. exec() finalizes its statement
before returning, failed or not.

Route every BEGIN, COMMIT and ROLLBACK, including the pool's rollback on
release, through exec(). migrate() now always returns its connection to
the pool, even when the rollback after a failed COMMIT itself fails.

Fixes tursodatabase#361

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local file: a COMMIT that fails with SQLITE_BUSY keeps the database write-locked until the process exits

1 participant