go-sqlite3 icon indicating copy to clipboard operation
go-sqlite3 copied to clipboard

Implement support for busy handler

Open jmmv opened this issue 9 years ago • 10 comments

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.

jmmv avatar Mar 24 '16 02:03 jmmv

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).

sqweek avatar Apr 18 '16 11:04 sqweek

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?

jmmv avatar Apr 18 '16 21:04 jmmv

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.

sqweek avatar Apr 19 '16 01:04 sqweek

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

zombiezen avatar Apr 01 '17 15:04 zombiezen

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

gjrtimmer avatar May 30 '18 14:05 gjrtimmer

Any update ?

gjrtimmer avatar Jul 21 '18 16:07 gjrtimmer

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?

israel-lugo avatar Mar 13 '21 22:03 israel-lugo

@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.

rittneje avatar Mar 13 '21 23:03 rittneje

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.

israel-lugo avatar Mar 27 '21 16:03 israel-lugo