Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle SQLITE_BUSY gracefully #47

Open
FiloSottile opened this issue Dec 17, 2018 · 10 comments
Open

Handle SQLITE_BUSY gracefully #47

FiloSottile opened this issue Dec 17, 2018 · 10 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@FiloSottile
Copy link
Contributor

Now that the shared cache is disabled and WAL-level locking is used directly, I am seeing a lot of errors like sqlite.Exec: Stmt.Step: SQLITE_BUSY: database is locked surface to the application.

They should be transparently handled by sqlite3_busy_handler just like SQLITE_LOCKED was handled with sqlite3_unlock_notify.

@crawshaw
Copy link
Owner

Interesting. Any idea how long you're holding transactions for?

I've considered doing this, but connections are already using sqlite3_busy_timeout with a 10 second timeout, and in everything I'm doing that's much longer than I ever actually hold a TX for if everything is working properly.

(Avoiding long TXs is actually pretty tricky. It requires performing no network blocking actions while holding a transaction.)

@crawshaw
Copy link
Owner

The first easy thing we should do is pass context.Deadline from sqlite.Pool through to the returned connection, so that if user code wants to use a connection for more than 10 seconds, it does not get SQLITE_BUSY calls. (I still think infinite deadlines should be ignored because it's too easy to deadlock.)

I've been avoiding a direct context dependency on sqlite because that drags in reflect and the rest of the kitchen sink, which makes life harder on potential tinygo use. So I'm considering:

  1. Rename sqliteutil to sqlitex.
  2. Move sqlite.Pool to sqlitex.
  3. Replace the done <-chan struct{} parameter to Pool.Get with a context.Context.

@crawshaw
Copy link
Owner

A second motivation for moving the sqlitex.Pool object to using a full context is that then it may be possible to use runtime/trace inside the sqlite package. (Though it will have to be done via some kind of adapter that removes the context dependency.)

@crawshaw
Copy link
Owner

I'm playing with these API changes on https://github.com/crawshaw/sqlite/tree/sqlitex

@FiloSottile
Copy link
Contributor Author

(I am a little surprised I would actually have 10s write transactions in there, although it was a 600GB database so I suppose it’s possible. I didn’t have the time to look into it yet unfortunately.)

@zombiezen
Copy link
Contributor

zombiezen commented Feb 8, 2019

I'm hitting this on a really small database where I'm fairly certain that I'm not keeping transactions open and with no contention (single process). Any ideas what could be causing this?

EDIT: Sorry for the noise, I realize I mixed up ROLLBACK and ROLLBACK TO, so entirely user error.

@AdamSLevy AdamSLevy added the enhancement New feature or request label Dec 12, 2019
@AdamSLevy AdamSLevy added the bug Something isn't working label Nov 6, 2021
@burdiyan
Copy link

I'm hitting this without shared cache enabled. I'm definitely using conn for less than the busy timeout value, but I still get SQLITE_BUSY in some cases. With shared cache enabled it doesn't happen as far as I'm aware.

@zombiezen
Copy link
Contributor

One common source of this error code is during the upgrade from a read transaction to a write transaction. Switching to BEGIN IMMEDIATE for code that can contain a write cuts down on the likelihood of this error.

@burdiyan
Copy link

@zombiezen thanks, I've been researching a lot this topic, and apparently this is indeed a solution. It's a bit inconvenient because the library doesn't offer anything to start the transaction easily. Similar to how SAVEPOINT can be started with Save() would be nice. Or did I miss it?

@zombiezen
Copy link
Contributor

There isn't anything in this library AFAIK that helps, but I did end up adding such a function in my fork: sqlitex.ImmediateTransaction.

burdiyan added a commit to seed-hypermedia/mintter that referenced this issue Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants