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

Corrupt database not being detected by API #519

Open
cipherCOM opened this issue Jun 19, 2020 · 5 comments
Open

Corrupt database not being detected by API #519

cipherCOM opened this issue Jun 19, 2020 · 5 comments

Comments

@cipherCOM
Copy link

cipherCOM commented Jun 19, 2020

We're currently in the situation that some users experience crashes due to corrupt databases. The cause is still unclear, also it seems not every user is experiencing crashes but also just misbehaviour. While trying to reproduce this issue I simply truncated an existing working database to just 37KB (totally random for a starter) and tried to open it as usual.

To my surprise everything just 'worked'. No errors or the YapDatabaseCorruptAction kicking in. Trying to further pin down what is happening inside of YapDatabase I found the following:

  • -[YapDatabase initWith...] is still working aka not returning nil
  • No YapDatabaseCorruptAction kicking in
  • Every extension could be registered aka not NO being returned (we're using YapDatabaseRelationship, YapDatabaseFullTextSearch and YapDatabaseSecondaryIndex)
  • Even -[YapDatabaseTransaction allCollections] is 'working' aka it just returns an array with 0 elements.
  • These log lines occur, coming presumably from the sqlite library
2020-06-19 17:01:54.227682+0200 AppName[75001:6475454] [logging] database corruption page 6 of .../db.sqlite at line 71377 of [378230ae7f]
...
2020-06-19 17:01:54.228419+0200 AppName[75001:6475454] [logging] database corruption at line 69644 of [378230ae7f]

After inspecting the source code I found that only SQLITE_ERROR is being tested for with allCollections and during this event SQLITE_CORRUPT occurs. This is why no API method registers anything wrong. Also I have to call an arbitrary method (I chose allCollections as it hopefully the one with the smallest footprint) to surface the corruption at all during setup before the actual business logic takes over. And even if SQLITE_CORRUPT is being checked for in allCollections this would still only result in a log statement and not even changing the return value.

Am I missing something in the setup procedure? What would I do in such a scenario? Of course this is still a test case and just truncating the DB isn't realistic, but if this case isn't found, I think any other corruption our users experience... would also be running unnoticed by the system.

@robbiehanson
Copy link
Contributor

Great question, and excellent contribution. You've given us deeper insight into the issue.

My understanding is that sqlite does some basic sanity checks when opening the database, but cannot guarantee there's no corruption in the file. Doing so would be too expensive and slow.

During runtime, sqlite keeps all data in "pages". And any page in the database could theoretically be corrupt. Which means, technically, any database call could fail.

From an API perspective, it's not really useful to have every method in the framework potentially throw an error. Even if it's theoretically possible. Because it would make the API annoying to use.

Perhaps a better solution would be to have the database post a notification to the running app? Something like YapDatabaseCorruptionDetectedNotification. And then your app could decide how it wants to go about handling the issue.

@cipherCOM
Copy link
Author

I agree that adding an error case for every call is annoying.

One could think about raising the YapDatabaseCorruptAction given during -init to the complete lifecycle, but I think this idea of automatic actions by YapDB during access is somewhat off. I also think a notification is more appropriate for this type of error case.

Though without having a random storage location not much can be done when receiving the notification, because one would need to close the DB, delete the files manually and create a new DB. This isn't possible due to the locking mechanism of the path if it's constant. So a random path must be used to have options when corruption occurs.

@robbiehanson
Copy link
Contributor

I think that's the crux of the problem. How should an app react if it discovers, in the middle of runtime, that the database is partially corrupt?

One strategy that may work is something like this:

  • if your app receives the YapDatabaseCorrupt notification, it sets a persistent flag (e.g. in UserDefaults), and then force-crashes the app (if your crashes are logged, you'll see them pop-up in your analytics)
  • during launch it checks for this flag
  • if found, it forks from the normal startup path

The easy fork would be to simply delete the database file. A more advanced fork would attempt to recover a minimum amount of info from the database. For example, if user authentication is stored in the database, then one could attempt to recover this before deleting the database files.

The recovery option is something that works well when the majority of the user's data can be recovered from the cloud. And if you can restore the user's login credentials, the recovery process becomes smoother for the end user.

@robbiehanson
Copy link
Contributor

one would need to close the DB, delete the files manually and create a new DB. This isn't possible due to the locking mechanism of the path if it's constant.

It is actually possible to delete the database files during runtime. First, you have to deallocate all YapDatabase & YapDatabaseConnection instances in your app. Once this is done, and the underlying sqlite machinery is all shut down, a YapDatabaseClosedNotification is posted. At that point you can delete the files.

However, in practice, this is easier said than done. I think the pragmatic approach for many will be the one I detailed above.

@cipherCOM
Copy link
Author

  • if your app receives the YapDatabaseCorrupt notification, it sets a persistent flag (e.g. in UserDefaults), and then force-crashes the app (if your crashes are logged, you'll see them pop-up in your analytics)
  • during launch it checks for this flag
  • if found, it forks from the normal startup path

This maybe could get moved into the wiki explaining the problem and offering one solution for it? I found the wiki as a start for YapDatabase quite helpful and with this added I would consider it complete for a basic but thorough start :)

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

No branches or pull requests

2 participants