Async inTransaction() and inDatabase()

[cfilipov]

Documentation states that inTransaction() and inDatabase() (and the database pool equivalents) block the current thread. But if you have an expensive query (or a big write) you probably don't want to block the main thread so you would want to dispatch async onto a background thread.

...
let backgroundQueue = DispatchQueue(label: "bg", attributes: .concurrent)
...
backgroundQueue.async {
    try dbQueue.inDatabase { db in
        self.favoritePois = PointOfInterest
            .filter(favoriteColumn)
            .order(titleColumn)
            .fetchAll(db)
    }
}  

This ends up requiring a lot of nesting. One thing I found myself doing is extending DatabaseQueue so I could do this:

...
try dbQueue.asyncInDatabase { db in
    self.favoritePois = PointOfInterest
        .filter(favoriteColumn)
        .order(titleColumn)
        .fetchAll(db)
}

Under the hood inTransaction() and inDatabase() use performSync() and I notice that you already have a performAsync() method but only DatabasePool.readFromWrite() uses it. I don't need a database pool though. I'm not trying to achieve concurrent reads, I just want to avoid blocking the main thread (also the database pool is still synchronous when writing).

This seems like it would be a common enough scenario that asyncInDatabase() and asyncInTransaction() should be part of GRDB. In fact, I think it's so common to create a thread to async your database calls that inTransaction() and inDatabase() should be async by default and the synchronous versions should be the special cases: syncInTransaction() and syncInDatabase().

Thoughts?

[groue]

Hello @cfilipov,

asyncInDatabase() and asyncInTransaction() should be part of GRDB

Here is an extension that you can reuse:

extension DatabaseQueue {
    func asyncInDatabase(_ block: (Database) -> ()) {
        DispatchQueue.global(attributes: [.qosDefault]).async {
            self.inDatabase(block)
        }
    }
}

asyncInTransaction can't be written this way because of commit/rollback error handling (that's why inTransaction() is throws, not rethrows).

I think it's so common to create a thread to async your database calls that inTransaction() and inDatabase() should be async by default and the synchronous versions should be the special cases: syncInTransaction() and syncInDatabase().

GRDB is sync by default because:

• FMDB has no async methods, and this did not prevent it from becoming the de-facto Obj-C wrapper for SQLite. Years of experience and thousands of adopters do count. I know that FMDB 3 will have async methods. I, so far, fail to understand the reasons. I have asked @ccgus for his opinion.
• Async programming nests code more often. More specifically, your sample code is too shiny because you have omitted the return to the main thread (one rarely fetches for nothing).
• Async programming is harder. More specifically, your sample code is too shiny because the self.favoritePois property is set in a background queue, which may lead to subtle threading issues. In the current state of GRDB, the self.favoritePois property would still set in a queue that is not the main queue, but at least there is less chance for issues since the main thread is blocked.
• Turning sync code into async code is cheap, when the opposite is not.

Now you know why sync accesses to the database is the default, and will remain so.

[groue]

This thread gives more context for async access in FMDB.

Quoting @robertmryan:

If a method can possibly block, it should be asynchronous. Apple has been religiously following this practice in all of their recent APIs and they just don't offer synchronous/blocking methods anymore. To be provocative, I think the question isn't "what's the advantage of putting this in FMDB?", but rather "should we have a synchronous rendition at all?"

It could have been written by you, @cfilipov :-) I don't follow this argument: inDatabase/inTransaction are not regular potentially long-running methods which return a result. They run an application closure, with the pitfalls I have listed above.

Last, the problem I see with ccgus/fmdb#469 (which adds async transactions to FMDB) is that transaction errors are not handled. FMDB makes it very easy to overlook database errors, and I think that's why this PR looks so nice at first sight.

[groue]

Hey, watching the WWDC conferences is always interesting. GCD in Swift 3 brings a (synchronous) pearl:

extension DispatchQueue {
    public func sync<T>(execute work: @noescape () throws -> T) rethrows -> T
}

Doesn't look wonderful at 1st sight, hu? Well, what's new is rethrows (but GRDB was emulating it before), and mainly @noescape, which was impossible to emulate. So now we have:

dbQueue.inDatabase { db in
    favoritePois = PointOfInterest.fetchAll(db) // No need for `self.`
}

Update your Swift3 branch!

[cfilipov]

Here is an extension that you can reuse:

I have this extension already. I was pointing out that it seems to be a common case to use it, such that everyone would end up implementing it, so it should probably be part of GRDB itself.

Async programming nests code more often. More specifically, your sample code is too shiny because you have omitted the return to the main thread (one rarely fetches for nothing).

Which would be yet another level of nesting. My point is that we're forced into one extra layer of nesting we don't need. Of course the extension asyncInDatabase solves that, and my suggestion is to make this part of the library rather than have everyone implement it.

Turning sync code into async code is cheap, when the opposite is not.

This is a good argument for keeping the sync api (and also keeping it the default), but not a good argument for excluding the async api.

I don't follow this argument: inDatabase/inTransaction are not regular potentially long-running methods which return a result.

I don't understand how they can not be considered long-running. These are apis for hitting a database. By definition they are likely to do I/O (if it's not an in-memory DB) and processing of the data. In all but the most trivial cases inDatabase/inTransaction will possibly block long enough to have an effect on the UI. Any app doing any real queries will inevitably have to dispatch to a background queue like they would for network requests.

[groue]

not a good argument for excluding the async api.

True. But the extension is trivial to add, I like my menus to be short (less documentation to write, less mental overhead for the documentation reader), and an equivalent asyncInTransaction can not be written (and I like symmetry).

inDatabase/inTransaction are not regular potentially long-running methods which return a result.

I don't understand how they can not be considered long-running.

Of course they can be long running, @cfilipov.

[groue]

It has been a long wait, but asynchronous database access methods (#550) will ship with GRDB 4.1 :-)