Grdb.swift: NSOperation chaining and the use of an explicit DB Transaction

[martindufort]

Hi Groue,

I have a database transaction usage strategy question today...

What did you do?

In our MacOS application, the execution of database-related functions are divided into multiple dependent NSOperations which are chained together in a dependency graph.

They are executed using the OperationQueue.addOperations(_:waitUntilFinished:) function.

Each operation is responsible for a discrete operation on the underneath database.
We want to ensure that this chain of operations is executed in a single database transaction.

So we defined a StartTransaction operation and a CommitTransaction operation which are attached at the start and at the end of this chain.

The StartTransaction explicitly starts the Transaction.

MyApp.dbQueue.inDatabase { db in do { try db.beginTransaction(.deferred) } ...

Then each NSOperation that requires a write to the Database does something like this:

MyApp.dbQueue.inDatabase { (db) in do { try email.update(db) } ...

Then the CommitTransaction operation would perform the explicit commit

MyApp.dbQueue.inDatabase { db in do { try db.commit() } ...

This was made to work by enabling the configuration flag allowsUnsafeTransactions which is not the best mechanism to enable this.

Note: The execution of all NSOperations chained together are performed on a separate concurrent DispatchQueue.

Questions ?

1. Regarding above, what is the best way to control the lifecycle of a single DB transaction while using multiple chained operations ?
2. Would this strategy work with a DatabasePool ?

Environment

GRDB flavor(s): GRDB
GRDB version: 4.14.0
Installation method: CocoaPods
Xcode version: 12.0.1
Swift version: 5.0
Platform(s) running GRDB: macOS
macOS version running Xcode: 10.15.7

[groue]

Hello @martindufort,

Thanks for the detailed description of your setup. I surely recognize a need, if not a use case. It is frequent that apps want to compose a series of distinct database updates into a single transaction.

The simplest case is when all those updates can be performed synchronously, in a single database write:

// Compose multiple updates in a database transaction
try dbQueue.write { db in // or dbPool.write
    try performFirstUpdate(db)
    try performSecondUpdate(db)
}

This simple case is also the safest regarding concurrency:

1. Because all database writes are serialized, there is never more than one thread which has write access to the database, at any time, and this removes all risks of write conflicts (more precisely, apps have all information they need in order to deal with the "conflicts" they want to deal with).
2. Because database updates are grouped in a database transaction, the application can make sure that the database file never ends up in an invalid or inconsistent state. In other words, the app can "safely" crash.
3. The transaction also guarantees that other eventual database accesses, reads and writes, eventually concurrent, will never see the database in an intermediate state, as if the transaction were only halfway through its completion.
4. The first error rollbacks the whole transaction, as if nothing ever happened to the database.

Those guarantees apply to both database queues and pools. They are the foundation for easy reasoning about database concurrency, where you can most of the time think locally, with little to no regards to other application components and their own database needs.

...And those guarantees only apply when database updates are grouped in a single call to a database access method such as dbQueue.write { db in ... }.

Now you understand why the allowsUnsafeTransactions flag you have set has "unsafe" in its name. Splitting a transaction into several calls to database access methods breaks the first, third, and fourth guarantee above:

Any concurrent write performed by some other application component can mess with your operations. Even if your app has no such concurrent write today, it is hard to guarantee that such need won't appear in the future.

Any concurrent access performed by some other application component may see the database in an intermediate state, as if the transaction were only halfway through its completion (this risk applies to database queues).

Any error has to be carefully handled by your application.

I would personally consider those risks as unacceptable. Kudos for preserving the transaction, @martindufort. But spanning a transaction accross several NSOperations is a recipe for concurrency-related troubles, in the present, or in the future.

---

Enough critics. Now let's aim at a solution.

And it was just described above. Have your operations gather the needed data along the way, and group all database updates into a unique dbQueue.write { db in ... } call as the conclusion of those operations. You'll get back all the concurrency guarantees that make GRDB a pleasant library to reason with, for you and other members of your team.

It is not a goal of GRDB to evolve any support for "long-lived" transactions. Supporting them would imply supporting concurrent writes, because a "tool" that would block all writes for an arbitrary amount of time would be a tool of very poor quality. Alas, concurrent writes are not supported natively by SQLite. And implementing them would introduce one of the worst (IMHO) Core Data warts, namely conflict errors, and merge strategies. For an overview of what GRDB preserves you from, see https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreData/ChangeManagement.html

[groue]

cc @pakko972, because we had a similar conversation a few days ago, which has greatly helped answering this issue: thanks 😉

[martindufort]

Hi @groue,

Thanks a lot for this detailed explanation. And I totally agree with your recommendations.

Everything was fine in our app until our enhanced beta test started showing analytics reports about DB issues: "A Transaction could not be started where one was already ongoing".

Which is exactly what you stated :

Even if your app has no such concurrent write today, it is hard to guarantee that such need won't appear in the future.

So I'm trying to rethink how to refactor all this to ensure that all NSOperations are wrapped within a single dbQueue.write { db in ... }.

This would take care of the transaction management and prevent other threads stepping onto each others toes.

Given our current structure, here's my thinking so far _(and this has not been verified yet)_.

executeOperations() {
  let dispatchQueue = DispatchQueue(label: "db.operations.concurrent", qos: .background, attributes: .concurrent)
  let op1 = MarkEmailOp()
  let op2 = UpdateIndexingOp()
  let op3 = UpdateUserOp()
  op3.addDependency([op1, op2])
  dbQueue.write { db in
      // Each op will have a db reference which they can use to perform the proper DB operations (insert, update, delete)
     dispatchQueue.addDatabaseOperations(db, [op1, op2, op3], waitUntilFinished: true)
     // Ensure we wait for all operations to finish 
     // If we don't then the TxN will commit before the execution of the operations are completed
  } 

So all NSOperations will be dispatched on a specialized queue. Once they are completed, the dbQueue.write block will be notified and the Transaction will be commited.

If you see any side-effects or things I have missed, please let me know.
Thanks a lot

[groue]

I'm glad we agree!

Your code snippet grabs exclusive access to the DatabaseQueue, for the whole duration of the operations.

This is not exactly what I initially meant:

Have your operations gather the needed data along the way, and group all database updates into a unique dbQueue.write { db in ... } call as the conclusion of those operations.

Indeed you don't perform the database write as the conclusion of the operations. You make the operations part of the database write.

Well, there's nothing inherently wrong here.

Beware that if your operations depend on some external resources like the network, you could block your database for an arbitrary long duration. In the other hand, if all of your operations could be replaced with plain function calls, then I'd clearly take the simplest route:

// I'd call it a day
try dbQueue.write { db in
    try markEmail(db)
    try updateIndexing(db)
    try updateUser(db)
}

As I said above, you are at the best place to decide: you know the desired behavior of your app, and you now have all the necessary information to make a choice.

For the record, and the interest of other readers, it should be possible to gather the results of asynchronous operations into a final transaction operation with something like:

let op1 = Op1()
let op2 = Op2()
let saveOp = BlockOperation {
    do {
        try dbQueue.write { db in
            try performStuff(with: op1.result)
            try performOtherStuff(with: op2.result)
        }
    } catch {
        // Handle database error
    }
}
saveOp.addDependency(op1)
saveOp.addDependency(op2)
OperationQueue.addOperation(op1)
OperationQueue.addOperation(op2)
OperationQueue.addOperation(saveOp)

[martindufort]

Thanks for the clarification. We now have a better path at making this more reliable. Cheers.

[martindufort]

It looks like I have hit a roadblock trying to perform db write operations on a different dispatch queue than the one that started the db.write { db in ... } statement.

I'm getting the following error after updating our code to something like this:

executeOperations() {
  let dispatchQueue = DispatchQueue(label: "db.operations.concurrent", qos: .background, attributes: .concurrent)
  let op1 = MarkEmailOp()
  let op2 = UpdateIndexingOp()
  let op3 = UpdateUserOp()
  op3.addDependency([op1, op2])
  dbQueue.write { db in
      // Each op will have a db reference which they can use to perform the proper DB operations (insert, update, delete)
     dispatchQueue.addDatabaseOperations(db, [op1, op2, op3], waitUntilFinished: true)
     // Ensure we wait for all operations to finish 
     // If we don't then the TxN will commit before the execution of the operations are completed
  } 

And the error is:

Fatal error: Database was not used on the correct thread.: file /Users/mdufort/dev/project/ocean-osx/Ocean/Pods/GRDB.swift/GRDB/Core/Database+Schema.swift, line 83

This is coming from

/// - preconditionValidQueue() crashes whenever a database is used in an invalid
///   dispatch queue.

[groue]

Fatal error: Database was not used on the correct thread

Yes. You must use a Database (db) inside the closure argument of a database access methods (dbQueue.write, read). You must not grab a Database and use it later.

// CORRECT
try dbQueue.write { db in
    try Player(...).insert(db)
}

// WRONG
let db = try dbQueue.write { db in
    return db
}
try Player(...).insert(db) // 💣

// WRONG
try dbQueue.write { db in
    DispatchQueue.main.async {
        try Player(...).insert(db) // 💣
    }
}

// WRONG
try dbQueue.write { db in
    let operation = MyOperation(db: db)
    operationQueue.add(operation) // 💣
}

Please see the end of https://github.com/groue/GRDB.swift/issues/853#issuecomment-712149873

[martindufort]

Hey @groue,
Thanks again for the support.

I understand that the best course of action would be to gather all operations in a single update as stated in this comment.

However for us to do this would require MAJOR code refactoring.

So I've been exploring a few things.

1- I tried setting the targetQueue within the database Configuration

config.label = "Ocean OSX Database"
config.targetQueue = self.dbOperationDispatchQueue

and reuse that same DispatchQueue when my DB operations are scheduled for execution.

let queue = OperationQueue()
queue.underlyingQueue = self.dbOperationDispatchQueue
queue.addOperations([a,b,c], waitUntilFinished: true)

2- Then I put out debug prints in the SchedulingWatchDog class to see which DispatchQueue was authorized.

GRDB DEBUG: Allowing database on queue: <OS_dispatch_queue_serial: Ocean OSX Database.writer[0x600002c4e300] = { xref = 3, ref = 1, sref = 1, target = ocean.database.operator[0x600002c3e480], width = 0x1, state = 0x001ffe0000000000, in-flight = 0}>

So I see that the target is the proper one.

3- When I execute my operations later, the watchDog still fails with my following debug print

GRDB DEBUG: Checking Valid Queue: No key in current DispatchQueue

because it is not considering the target queue as the executing queue.

Problems:

1. I don't think it is possible to extract the targetQueue from a DispatchQueue thus we woud be unable to check for queue validity with the watchDogKey

Questions:
1: Any mechanism I can use to obtain the SerializedDatabase queue (from a Database instance) so I can set this to my OperationQueue.underlyingQueue before execution
2: Is this safe / possible ?
3: I know this is far from perfect but we are trying to make this better before we move onto a V2.

Thanks

[groue]

However for us to do this would require MAJOR code refactoring

I'm sorry to hear that.

Any mechanism I can use to obtain the SerializedDatabase queue (from a Database instance) so I can set this to my OperationQueue.underlyingQueue before execution

No, there is not.

Is this safe / possible

It would be possible to grab the serial dispatch queue of a DatabaseQueue, or the serial dispatch queue of the writer connection of a DatabasePool.

Would it be safe?

Well, SQLite concurrency is hard. This is why I wrote GRDB. So that my apps can use robust concurrency primitives, and are not littered with threading database bugs and subtle workarounds or assumptions that are just waiting to break once I or a coworker starts working on a new feature.

GRDB assumes total ownership of the database scheduling, in a respectful intimacy with the frame given by SQLite itself. It has built tons of features on this assumption, from schema cache management to database observation, and many other application tools that users love to trust.

So no, it would not be "safe", and it would be totally unsupported (meaning: you'd be on your own).

I know this is far from perfect but we are trying to make this better before we move onto a V2.

Fork and expose the serial queue, maybe?

I'm sorry if I sound unsupportive. When you start working on your V2, maybe catch up with the GRDB concurrency best practices. They are abundantly documented, in nearly every piece of documentation. There's a lot to win from a peaceful relationship with this library.

[martindufort]

Thanks again.

Just to clarify and close this thread. ;-)

Seems our problem is that we decided to structure our application logic operations (along with database updates) using the OperationQueue / Operation paradigm. These operations are independent but linked together in a dependency graph.

I understand why you decided to move GRDB in the way it's currently architected... and that's why we decided to move away from Core Data. But unfortunately it seems our decision to use Operations is bitting us back big time.

We will try to see if we can grab the serial queue and work with that.
Cheers - Martin