apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.85k stars 707 forks source link

SQLite: Ability to change journal mode #3399

Open pixelmatrix opened 1 week ago

pixelmatrix commented 1 week ago

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:

try db.execute("PRAGMA journal_mode = WAL;")

Describe the solution you'd like

Would you be open to exposing this as an option in some form?

A few API ideas:

  1. setJournalMode(_ mode: JournalMode) throws on SQLiteDatabase protocol
  2. execute(_ sql: String) throws on SQLiteDatabase, which would allow direct access to execute any PRAGMA command
  3. Add useWAL as an optional init parameter of SQLiteNormalizedCache

Alternatively, I could implement my own SQLiteDatabase, but that is currently not possible because DatabaseRow does not have a public initializer.

calvincestari commented 1 week ago

Hi @pixelmatrix,

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.

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.

pixelmatrix commented 1 week ago

@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.

calvincestari commented 1 week ago

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.