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

Implement support for busy handler #290

Open
jmmv opened this issue Mar 24, 2016 · 12 comments · May be fixed by #1278
Open

Implement support for busy handler #290

jmmv opened this issue Mar 24, 2016 · 12 comments · May be fixed by #1278

Comments

@jmmv
Copy link

jmmv commented Mar 24, 2016

I think it'd be good for these bindings to support https://www.sqlite.org/c3ref/busy_handler.html .

Use case: I'm currently writing an app that causes quite a bit of traffic to an SQLite database and am encountering contention. I wrote my own wrappers around the queries I'm issuing to handle SQLITE_BUSY errors and retry them... but I just discovered that SQLite3 has a built-in mechanism for this via sqlite3_busy_handler. It would be nice if we could register a hook via SQLiteConn or something like that.

@sqweek
Copy link
Contributor

sqweek commented Apr 18, 2016

Note that if your response to SQLITE_BUSY is to retry the statement, its simpler to use the sqlite3_busy_timeout(n) API (which will have sqlite3 retry for up to n milliseconds before returning SQLITE_BUSY - it uses the busy_handler mechanism internally).

The bindings don't support the sqlite3_busy_timeout call directly, but you can enable it via something like sql.Open("sqlite3", "foo.db?_busy_timeout=5000") (for a 5-second timeout).

@jmmv
Copy link
Author

jmmv commented Apr 18, 2016

Thanks for the advice @sqweek; I'll keep the timeout parameter in mind, which may suffice for my use case.

I know the sqlite3_busy_timeout call is not directly supported, hence this bug. Would there be a reason not to implement it?

@sqweek
Copy link
Contributor

sqweek commented Apr 19, 2016

There's no reason not to directly support sqlite3_busy_timeout, but I don't think it adds a lot of value. The timeout is generally something you set once and forget about, and its desirable to set it as early as possible to avoid errors when two processes try to create the same database concurrently. The current ?_busy_timeout=N API is quite acceptable IMO.

This issue was originally about sqlite3_busy__handler_ though, which adds a bit more value in terms of flexibility. Also the groundwork is already there for RegisterFunc.I didn't mean to suggest that's not worth having, just that its unnecessary complex for many use cases.

@zombiezen
Copy link
Contributor

Now that database/sql supports Context, it seems like the busy handler could be implemented by waiting for the Context to be Done.

@gjrtimmer
Copy link
Collaborator

@jmmv @sqweek @zombiezen help wanted could some please write a PR for this ?

@gjrtimmer
Copy link
Collaborator

Any update ?

@israel-lugo
Copy link
Contributor

israel-lugo commented Mar 13, 2021

Hi,

?_busy_timeout=N can be useful for simple needs. However, if you have multiple different queries and use cases (e.g. a service exporting an API that relies on the DB for storage), a single timeout is insufficient.

Having a busy handler that waits for context cancellation would provide the necessary flexibility for clients to specify their own deadlines on a case by case basis. This is widely done e.g. in gRPC servers.

sqlite3's own implementation of sqlite3_busy_timeout might be useful as a starting point. This uses sqlite3_busy_handler to register sqliteDefaultBusyCallback. Looking at sqliteDefaultBusyCallback, we can see this is basically doing incremental sleeps and tracking the running total against the desired timeout.

I'm not very familiar with the go-sqlite codebase, unfortunately. However, assuming a Go function can be hooked in and fed to sqlite3_busy_handler, implementing the handler itself is simple.

The handler below sleeps in exponentially incremental intervals of up to 256 ms, selecting against context.Context.Done() to catch cancellation:

func sqliteContextBusyHandler(ctx context.Context, count int) int {
        const maxShift = 8 // 2**8 = 256 ms
        if count > maxShift {
                count = maxShift
        }
        t := time.NewTimer((1 << count) * time.Millisecond)
        select {
        case <-ctx.Done():
                 // cancelled; cleanup and stop retrying
                t.Stop()
                return 0
        case <-t.C:
                return 1
        }
}

This is assuming we can receive an arbitrary argument, i.e. the third argument to sqlite3_busy_handler. That would be ctx in this case.

It would be even better to store the timer between passes and call t.Reset() instead of repeatedly recreating it. The first argument in this case would be a pointer to struct { context.Context, *time.Timer} instead.

Could someone more familiar with go-sqlite please see if they can pipe this in?

@rittneje
Copy link
Collaborator

@israel-lugo The SQLite busy handler is used for the entire database connection. There wouldn't be anywhere to get a per-request context.Context as you would need for your proposed implementation. You could potentially have this library attempt to set the busy handler internally on each API call and have the context recorded off to the side, but that would be fairly complicated and error prone.

There seems to be some overlap between this desire and the need to interrupt a long running query even if it isn't blocking due to SQLITE_BUSY. For that this library is currently leveraging sqlite3_interrupt, which itself is problematic and error prone.

In short, the SQLite C API is flawed because none of the cancellation-related functions have any mechanism for tying it back to the specific operation you are performing (i.e., the call to sqlite3_step).

You could try to do something with sqlite3_progress_handler which at least seems less error prone than sqlite3_interrupt. But as I alluded to, this will require some fairly involved bookkeeping within this library as to which context belongs to the current operation.

@israel-lugo
Copy link
Contributor

Thank you @rittneje, you're right of course. I'm trying out https://pkg.go.dev/crawshaw.io as an alternative; it's not an interface for database/sql so can afford to be more SQLite-specific.

@HappyBinbin
Copy link

Note that if your response to SQLITE_BUSY is to retry the statement, its simpler to use the sqlite3_busy_timeout(n) API (which will have sqlite3 retry for up to n milliseconds before returning SQLITE_BUSY - it uses the busy_handler mechanism internally).

The bindings don't support the sqlite3_busy_timeout call directly, but you can enable it via something like sql.Open("sqlite3", "foo.db?_busy_timeout=5000") (for a 5-second timeout).

it is useful for me , thanks !

@polyscone
Copy link

Is anyone working on this?

If not I can implement it and submit a PR.

@polyscone
Copy link

I'm just going to assume no one's working on this, so I've created a PR here: #1278

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants