yapstudios / YapDatabase

YapDB is a collection/key/value store with a plugin architecture. It's built atop sqlite, for Swift & objective-c developers.
Other
3.35k stars 365 forks source link

YapDatabase should provide the application with some method of being informed of database errors and rollbacks #251

Open jsutwanttohelp opened 8 years ago

jsutwanttohelp commented 8 years ago

There are many calls such as 'YDBLogError(@"Error executing 'updateAllForRowidStatement': %d %s", status, sqlite3_errmsg(connection->db));'

The application should be able to handle db errors. Instead transactions seem to be silently (except for the log) discarded.

Also rollbacks are handled with just a log statement with no way for an application to report the problem to the user or to a server. YDBLogVerbose(@"YapDatabaseConnection(%p) rollback read-write transaction", self);

I am starting to wonder why I'm using this DB. I'm scared.

sbooth commented 8 years ago

One approach that might work is an error handling block registered with the database that could be invoked when an error condition occurs.

jsutwanttohelp commented 8 years ago

I think it would be better if if the application specifies the error handling block in the same call that it uses to create the transaction block.

This is because typically a commit failure handler needs to do something specific to the transaction or the source of the transaction.

robbiehanson commented 8 years ago

There are many calls such as 'YDBLogError(@"Error executing 'updateAllForRowidStatement': %d %s", status, sqlite3_errmsg(connection->db));'

Yes, YapDatabase has very extensive logging. My policy has always been to check for any possible errors, and add log statements to ensure such errors would be seen immediately. During the development of YapDatabase, and while running unit tests for it, this helps me identify the root cause of problems much quicker/easier.

However, you are confusing the presence of log statements with an expectation that they can/will happen. Would you feel better if I removed all the log statements? Sounds silly right? But this is kinda what you're saying.

The fact that the library has log statements does not mean any of these errors are ever expected to happen. In fact, they are expected to never happen. And the unit tests are there to ensure these code paths are tested.

Do you think there are log statements in Apple's array/dictionary implementations? Do you expect them to fail?

Also rollbacks are handled with just a log statement with no way for an application to report the problem to the user or to a server.

This is a good question. A rollback can only happen if YOU initiate it. That is, the rollback can only occur if you explicitly:

[databaseTransaction readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction){

    // save some object(s)
    if (changedMyMindForSomeReason) {
        [transaction rollback];
        return;
    }
}];

I am starting to wonder why I'm using [any database in the entire world].

Pardon my rephrasing. But I wanted to address the root issue of your concern, as it's obviously weighing on your heart.

Whenever one is performing a sufficiently complex operation in code, there is a potential that it may fail. The important questions to ask are:

Let's look at 2 extremes of the spectrum:

Obviously an HTTP request could fail for any number of reasons. No Internet access being an obvious one. And if the operation fails, we can simply perform it again. Perhaps after Internet access has been restored.

But what about allocating memory? If that fails, if fails because... the application is out of memory? So what are you going to do if your malloc fails? What possible recourse does the application have?

Chances are, you're not checking for malloc fails in your own application. (And most people don't.) Why not? Because the chances are so incredibly slim. And the recourse is ... most of the time nothing. Maybe kill the app?

I've chosen these extremes to serve as navigational beacons as we sail through this thorny issue.

In terms of a database system, you have the following risks:

The problem is: what can we possibly do if disk IO fails? What recourse do we have? If our application is heavily dependent on reading/writing from/to a disk-backed database, and the disk fails... what are we going to do?

The database layer is generally the same way. We don't expect sqlite to randomly fail for no reason. We don't expect it to be riddled with bugs. SQLite is the most widely deployed database engine in the world.

What I'm trying to say is this:

You can decide to not use YapDatabase. You can say to yourself "I don't trust any database except SQLite", and you can go off and write your own layer on top of it. But when you get done, you're going to have a class called something like MyDatabaseLayer, and it's going to contain all the logic for reading & writing to the database. And then you're going to realize that any of the write methods in MyDatabaseLayer could fail. How are you going to handle it?

I'm not being condescending. I really want to know your thoughts. Because I can imagine some situations in which a readWriteTransaction failing might not be the end of the world. Maybe I'm just saving an object after a user hits the "Save" button. And I suppose I could display an alert. Although it would be weird, at least it wouldn't crash the app. But then again, is that really any better than crashing the app? It's a completely unexpected failure, and either way the user has to go back and re-enter all the information.

But looking at most of my readWriteTransactions... the only safe thing I can consider doing is killing the database. And probably the app, since it will start going off the rails shortly after the database stops working.

One approach that might work is an error handling block registered with the database that could be invoked when an error condition occurs.

I agree. This is a good start.

I think it would be better if if the application specifies the error handling block in the same call that it uses to create the transaction block.

The problem we have is that if any single transaction fails, we cannot accept the risk of allowing future transactions to continue. That is, if commit 4 fails (for whatever reason), we shouldn't allow commit 5 to proceed. At least, not unless you explicitly say that its OK to continue. Basically, the only safe default thing to do would be to shut down the specific YapDatabase instance.

We could do both suggestions. That is, there could be a "catch-all" error block handler. And specific transactions could provide their own explicit error handler, that would override the "catch-all" block.

Thoughts ?

jsutwanttohelp commented 8 years ago

Thank you for the thoughtful response.

I think a catch all error handler would be helpful for many applications where the error recovery does not depend on the details of the transaction that failed. It is probably not sufficient for all applications. I think it would be good to implement both. I only worry that these partially redundant methods will confuse coders with excessive optionality. I haven't thought about what happens if the application registered both types of error handlers. Would both be called? Or just the most specific?

Sorry for the confusion. I didn't mean to suggest that the error log messages are an issue. I didn't fully articulate that I thought they were not sufficient.

I agree that many of the error cases that are logged are essentially dead code that can't be tested and exist primarily for validation during testing. I'm not worried about those. But I think there are cases where the transaction can fail that can't be prechecked by YapDatabase, for example there is the case you mentioned: when there is no space left on disk.

I think that you're right that in the majority of mobile app use cases whether the transaction succeeds or fails does not matter very much. Probably in most cases it would be helpful to at least report the error to the server. I know I want that.

My use case is guaranteed messaging to a device. If a device can't commit a message when it is received the device should not ack the message to the sender. Later on when the transient error on the device is resolved the device can request all unacked message.

Here is a imaginary but concrete example. A person using an iphone takes so much video on their birthday that they run out of disk space. They aren't paying attention and don't realize until the next day when they delete the video. This person also uses Whatsapp. Whatsapp acknowledges messages to the server when the device receives them, at which point the messages are deleted on the server. If whatsapp acks the messages upon receipt regardless of whether they are committed then the person will never receive any of the happy birthday messages that they were sent while the video was using up their disk space.