search

zombiezen / go-sqlite

Low-level Go interface to SQLite 3
https://pkg.go.dev/zombiezen.com/go/sqlite
ISC License
741 stars 18 forks source link

Use immediate rather than deferred transaction in sqlitemigration ensureAppID #98

Closed bradenrayhorn closed 4 months ago

bradenrayhorn commented 5 months ago

This fixes an issue that can occur when using multiple sqlitemigration pools attempt to connect to the same database file concurrently. When opening a sqlitemigration pool while another pool is writing to the database, the pool may fail to startup with a SQLITE_BUSY error (shown as database is locked).

In my use case, I had some end-to-end tests running that started an API server and modified the database using a CLI. Both the CLI and the API server started a sqlitemigration pool and occasionally the CLI calls would fail due to this issue.

From my testing, the culprit is the ensureAppID function, which opens a deferred transaction that will later be upgraded to a write transaction. At the point of upgrading to a write transaction, if the database is locked, SQLite will fail immediately with SQLITE_BUSY [1] [2] and the busy timeout will have no effect, causing the pool to fail to start.

If ensureAppID is switched use an immediate transaction, that tells SQLite this is a write transaction. SQLite will attempt to lock the database immediately. If a lock cannot be acquired, the busy timeout is used allowing the transaction to wait until it can get a lock, thus allowing the pool to start successfully.

I've also included a test case. Please let me know on feedback on the test, it is a bit unwieldy. I'm not sure if it fits with the standards of this project.

zombiezen commented 4 months ago

This is great, thank you for the investigation and the fix!