-
Notifications
You must be signed in to change notification settings - Fork 728
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
SQLite: Ability to change journal mode #3399
SQLite: Ability to change journal mode #3399
Comments
Hi @pixelmatrix,
Can you share more evidence that points to the change in journaling mode being the solution to the crash. We're not against making this, or other pragmas, available but something conclusive that demonstrates this change resolves the issue will be helpful. Apollo iOS can be forked so making the change on your own fork while proving out the theory should be possible. |
@calvincestari yes, definitely. I've forked the repo and I'm running it in WAL mode in a recent TestFlight release. So far the crashes have stopped, but it hasn't been long. I will continue to monitor for a week or so to see if we get any further crashes. Would that be enough evidence? I can also share some of the trace which points to the app being killed while SQLite is active. |
Yes, that sounds good. Monitor it, gather what you need to and then let's evaluate once it's had some time to prove itself. |
I wanted to share an update on this after living with the patch for some time. The crashes have decreased quite a bit, but haven't been entirely eradicated. After further research, it seems that even in WAL mode, we need to be careful not to leave the app in the middle of a write transaction when the app is suspended to prevent being killed by the system. It looks like using the database in WAL journal mode is going to be necessary to prevent these crashes. GRDB (a separate SQLite implementation) has a guide on how to successfully share a SQLite db between processes here, which mentions needing to use WAL: https://swiftpackageindex.com/groue/grdb.swift/v6.27.0/documentation/grdb/databasesharing. As far as addressing the transactions in the background, I'm still thinking about how to approach it. It seems like we need to wrap transactions in a background task, and cancel them if the task is expiring. Additionally, we need to prevent new transactions while the app is entering the background, ideally enqueuing them to run once the app is resumed. I think a lot of this stems from using Subscriptions which can trigger a lot of cache writes. If a subscription comes through just as the app is leaving the foreground, it can leave a transaction open. It would be nice to be able to pause a subscription when the app is entering the background, or at least defer the cache write. In summary, I still think supporting this mode is necessary for my use case, but there may be more work needed to properly support storing the SQLite db file in the App Group Container for sharing with other extensions. |
I'll look into exposing the ability to set the journal mode. As for pausing subscriptions, you're either subscribed and receiving events or not. I've never seen any server implementation that would keep the subscription alive but not deliver any events. I'm curious about your subscriptions behaviour when entering the background though. Surely the websocket connections are killed and you have to restart the subscription when the app comes to the foreground again? |
Yeah, the websocket connection gets disconnected while the app is in the background. The crash logs show that SQLite transactions are being left open. There are other ways for this to happen, but my theory is that a subscription update comes through right as the app is transitioning into the background, which leaves a SQLite transaction open. There are some noisy subscriptions that could be firing nearly constantly for some users. If background time is requested to complete the SQLite transaction, the socket may continue to live longer, so we may need to also manually disconnect the socket early to avoid further transactions. I think this is doable in the WebSocketTransport, but I haven't explored it quite yet. |
@pixelmatrix - I've got a draft PR up for this at apollographql/apollo-ios-dev#443. Please take a look and let me know if it's on the right track to satisfy what you're needing it to do. Thanks. |
That looks great! Thanks for looking into this |
Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better. |
The PR for this has been merged and the feature will go out in the next release, which we're planning to do this week still. |
Fantastic! Thank you |
Use case
I've been investigating a regular background crash in our app. After some research, it appears to be due to keeping the SQLite database file located in the App Group's container. It looks like other people have fixed these crashes by using the database in WAL mode. This is possibly by executing a PRAGMA statement:
Describe the solution you'd like
Would you be open to exposing this as an option in some form?
A few API ideas:
setJournalMode(_ mode: JournalMode) throws
onSQLiteDatabase
protocolexecute(_ sql: String) throws
onSQLiteDatabase
, which would allow direct access to execute any PRAGMA commanduseWAL
as an optional init parameter ofSQLiteNormalizedCache
Alternatively, I could implement my own
SQLiteDatabase
, but that is currently not possible becauseDatabaseRow
does not have a public initializer.The text was updated successfully, but these errors were encountered: