Desktop App: SQLite Database Locked by Background Tasks

There's currently these tasks run asynchronously in the background.

1. updating and publishing local repository
2. updating remote repositories (e.g. f-droid.org or guardianproject)
3. when adding an app from a remote repository

• downloading the APK for the remote app
• downloading remote graphic assets for the remote app
• downloading app screenshots for the remote app

While they are run, the SQLite database is typically locked for write operations. When the user tries to do anything that requires writing to the database during this time, they are currently presented with this error page:

Some of the above background tasks can take a very long time. E.g. updating the f-droid.org repo (2) initially can take several hours (partly due to admin#35).

This is a really bad user experience (especially on first app start), so we need to solve this problem. Below possible solutions are sketched out.

Use a database that supports concurrency

Other databases such as MySQL or PostgreSQL are a lot better at dealing with concurrency. The problem practically doesn't exist there. However, requiring such databases makes the deployment procedure much more complex and the memory footprint a lot larger. Therefore, this should be avoided if other solutions are possible.

A way forward with this might be to deploy the app as flatpak or snappy image (#160 (closed)) that comes with a database pre-configured.

Shorten the time the database is locked

At the moment, it looks like the first write operation locks the database and only releases the lock once the background tasks finishes. Maybe, there is a way to release the lock earlier, ideally after every write operation and then let the task acquire it again.

This would still not be an optimal solution, because the database would still get locked a lot in short intervals (2). The user might be lucky and get their operation done by pressing Try Again. However, it might also happen that a write operation initiated by the user prevents a running background task from acquiring a lock and thus stop the processing of background tasks entirely. That is a scenario that should be avoided by preventing the background processor from crashing and by making it try again later.

The lock hold by task (1) could be shortened more easily, because there is only one write operation happening at the beginning and one at the end of the task. Both do not need to be atomic.

Allow the user to control background tasks

Run and stop all tasks on demand

When running repomaker as a GUI application in a WebView, it might be possible to add an extra button and status indicator for background tasks. This would allow the user to stop the background tasks for as long as they need to work with their repository.

The downside is that this is also not a good user experience as it makes using repomaker more difficult. The user gets error messages that require understanding the complexities of database locking and gets told to click buttons in various situations. A local repository for example does not get published (1) when the background tasks are not running.

Run tasks synchronously and let user wait

All current background tasks could also be run in the foreground. A local repository would only get published (1) when the user presses a to be added Publish button. Then the user needs to wait until the publishing completes.

Similarly, there could be a button to update remote repositories (2). The downside of this is that it can take a long time to update those and users would rarely update them. This would lead to outdated repository information and errors when the user tries to add apps from the remote repositories, because the referenced apps, their APKs or graphic assets might not be available anymore.

Adding apps from remote repositories (3) could only complete once all remote data has been downloaded. We might run into issues with HTTP requests timing out here.

Overall, these user-centric solution would only advance the status quo a little bit by making it clearer to the users why they need to wait, but introduce other problems such as outdated information.

Use several databases

With Django it is possible to use more than one database. The most problematic models related to remote repositories (2) could be moved to this database, so that they can be locked while the user can freely interact with their local repositories.

However, it is not possible to have relationships across databases and repomaker's current database design uses these relationships:

• RemoteRepository references User
• RemoteApp references Category
• RemoteApkPointer references Apk

It does not seem to be possible to split the models without breaking Django's ORM and manually gluing things back together by using the object's primary keys and trying to keep things in sync between the two databases.

However, the documentation says that in the case of SQLite

there is no enforced referential integrity; as a result, you may be able to ‘fake’ cross database foreign keys. However, this configuration is not officially supported by Django.

So we might get away with doing a split. The full consequences of this are still unknown. However, in single-user desktop mode and the current feature set, there's no other users being added and removed. The same goes for categories. So the problem space might be limited. The only exception is with Apk that are for example removed when not referenced anymore. Checking the references might not work reliably across databases.

Also, later (e.g. for #15 where App would reference RemoteApp) we might want to introduce new inter-database references. These would then increase the risk of things going wrong. So this route should only be taken with great care and some prior tests of how things behave with a split database.

Even if the database can be split into two, this would to resolve background tasks (1) and (3). However, possible solutions for those have been sketched above.

@eighthave @pserwylo I tried to sketch all possible solutions to this problem that we have discussed already several times. It would be great if you could have a look and see if you maybe have new ideas for how to solve the problem. I am also interested in your opinion on how to move forward here.

Hmm, I was hoping for the multiple database option, but you make a compelling argument that it will likely be unmanageable.

My next favourite is "Shorten the time the database is locked". You mention that this is still suboptimal because the database will still be locked for small time periods. Lets think about the following:

• 1. [background]: save icon A.
• 2a. [background]: save icon B.
• 2b. [foreground]: add app X.
• 3. [background]: save icon C.

Under these circumstances, where 2a. and 2b. happen approximately concurrently, there will indeed be lock contention.

However each task is small (a fraction of a second?), and so hopefully we can set it up (or send patches upstream to the background task library) so that each task waits a certain timeout before failing due to waiting for a write lock. Then, each should only ever have to wait far less than a second before it is able to write to the database.

Even in the case where "add app X" is actually several queries which should indeed be wrapped in a transaction, then we are still only talking a tiny fraction of a second. Especially because this is on a proper computer, not just an Android device.

As for what is involved in fixing upstream, that is a different matter :(

Thanks for your quick feedback @pserwylo!

I was thinking about experimenting with a split database, but I think you are right and we should try if we can shorten the lock time first.

So I dove into Django's database code and into the code of the background_tasks library. What I found is that the library is explicitly and intentionally running each task within one atomic transaction. It looks like changing this is neither easy nor desired.

The reason is that the library locks a task when it is started and only releases this lock when the tasks completes. When not running the task within one database transaction, the task's code might crash and never release the lock. Probably worse, the database might be left in an inconsistent state afterwards.

Long story short, it is probably not a good idea to hack into how the background tasks handle database transactions.

However each task is small (a fraction of a second?)

Unfortunately, that is not the case. There's several tasks that download (or upload) stuff. These can take awhile. Currently, task (2) runs for hours when updating the f-droid.org repo initially.

As mentioned above, most of that time is being spent on downloading the app icons. So independent of the locking problem, it is probably a good idea to outsource these downloads into dedicated tasks that run after the repo has been updated.

The update then runs for a couple of seconds and takes just a little bit longer than downloading one app icon. There is a configurable delay of 5 seconds between each task. While not ideal, this might result in a high chance that user changes fall it the non-locked delay time or that one of the Try Again presses do.

So I'll implement this now and make some usability tests afterwards.

Alrighty, now we're getting somewhere (though these timezones cause quite a laggy discussion here sorry0.

The reason is that the library locks a task when it is started and only releases this lock when the tasks completes. When not running the task within one database transaction, the task's code might crash and never release the lock. Probably worse, the database might be left in an inconsistent state afterwards.

That seems like a weird argument, in the sense that a task is very similar to a web request. A web request can crash, and leave the database in an inconsistent state. However, it is up to the web developer to ensure that transactions are used appropriately throughout the lifetime of a web request. By the logic of the background_tasks design, that means that all web requests should lock the database too, right?

Anyhow, if that is a hard restriction with that library than so be it.

Unfortunately, that is not the case. There's several tasks that download (or upload) stuff. These can take awhile

Sorry, what I was referring to was not the entire task, but the bit of it which requires database writing. So the beginning of the task starts to download the icon, which of course takes a long time. Even without admin#35 it is a long time in computer terms. However, the time which it requires to write the resulting image path to the database is miniscule.

But, if it is a hard design requirement that the background_tasks plugin locks the database, then so be it.

it is probably a good idea to outsource these downloads into dedicated tasks that run after the repo has been updated

Agreed. One final suggestion which builds on this:

If the task only reads from the DB and then downloads an icon to disk, does it still start a transaction? If not, then we could have one set of tasks which does just that: Figures out the next icon to download and then downloads it to a well-known directory.

Then another task which runs much more infrequently (e.g. every 30 seconds) scans this well-known directory for files it hasn't seen before, and then bulk updates the database with the relevant paths.

Under these circumstances:

• The slow network IO never locks the DB.
• The scanning for new icons is fast-ish (still hits the disk, but magnitudes faster than downloading a single icon), and still in the sub-one-second timeframe.
• The user still gets to see their icons get refreshed almost in real time (depending on the delay between each scan-for-new-icons-in-well-known-directory).
• The locking window is reduced to only a few ms - 100s of ms while we scan the well known directory.

Thoughts?

@grote: If I understood this issue correctly, a circumvention could be to use a custom management command instead of background_tasks and schedule such a command via cron-job. Custom management commands can perform long running tasks without locking a sqlite database, because they are not wrapped in an transaction.atomic() decorator/context manager. I've assembled a minimal demo project for illustration: https://gitlab.com/uniqx/playground-django-sqlite-db-lock

By the logic of the background_tasks design, that means that all web requests should lock the database too, right?

Anyhow, if that is a hard restriction with that library than so be it.

In Django a web request can run within an atomic transaction, but this behavior is off by default for performance reasons.

Please note that I was speculating on the reasons why the background_tasks library is running its tasks within an atomic transaction. We didn't talk to them yet, but can of course open a discussion with them. However, I am skeptical they will run their tasks in auto commit mode to support our exotic use case when it carries severe risk of breaking other use-cases.

If the task only reads from the DB and then downloads an icon to disk, does it still start a transaction?

I ran a quick test with @uniqx's playground app and it seems that a transaction locks the SQLite database no matter whether you do only execute read-only queries in it or not.

So it looks like no matter what we do in a task, it will always block the DB for the entire duration of the task. In that case, the idea of splitting the app icon downloading into yet smaller tasks, does not work.

I've assembled a minimal demo project for illustration: https://gitlab.com/uniqx/playground-django-sqlite-db-lock

Thanks a lot @uniqx for your feedback and this demo project! It's very nice for quickly experimenting with the problem.

Custom management commands can perform long running tasks without locking a sqlite database, because they are not wrapped in an transaction.atomic() decorator/context manager.

I think the problem is not so much with the management command. The one used by the background_tasks library does not lock the DB. What it does is lock the DB as soon as the task is started and only unlock it when the task completes.

So the proper way to reproduce the problem is to increase the sleep time of the dbOp() to at least 3 seconds. We do have tasks that run that long and longer. If the user tries to write to the DB during this time, they will get the DB locked error, because dbOp() is annotated with @atomic.

If I understood this issue correctly, a circumvention could be to use a custom management command instead of background_tasks and schedule such a command via cron-job.

I hadn't considered replacing the entire background_tasks library, yet. However, I am a little hesitant to do this, because of the amount of work involved. The only other candidate would be Celery, but this is a heavy weight and does not really seem to be suitable to be used in the context of a desktop app.

What we could try however is to somehow hook into the background_tasks processing and remove the @atomic annotations from run_task() and run_next_task(). There is a certain danger that there are unintended side effects of doing this such as tasks throwing exception never getting unlocked. I tried to simulate the latter by throwing a RuntimeError() and observed the task the be rescheduled as expected. So it is probably worth looking into how to remove the annotations and then give this some more testing.

But even without this, splitting the icon downloading into individual tasks already improved the situation a lot by having smaller tasks that give the opportunity to execute write transactions during the task pauses. This is implemented in !148 (merged), but during the initial remote repository update, the user still gets the database locked screen from time to time and needs to press "Try Again".

So I will look into removing the @atomic annotations from the relevant background_tasks methods now and then report back.

I managed to hack out the @atomic decorators in grote/repomaker@f3f78c274183faf904b11ce84143de4d060e5501 and it generally works great. I also opened a ticket against the background_tasks library. Let's see what they say.

The only problem I see is that tasks might get locked forever if the unlocking code doesn't run for some reason. Such a reason could be an ordinary KeyboardInterrupt, but also a database that was locked by the user. For the latter case, I built in a little workaround. There might be others, so the solution is certainly not perfect.

Another issue I ran into while testing this approach is that the initial update of the f-droid repository takes way longer when not run within one transaction. I suspect this is because instead of locking the database once, it is done n times for each app and locking the database seems to be an expensive operation. For the user, this is even worse than before, because most requests take forever without any visible feedback. Sometimes they also fail with a database locked error.

Because of this, I decided to manually add an @atomic decorator to the function that updates the remote repository. This way, it is a lot faster, but the user might get the above 'please wait' (database locked) screen and then needs to press "Try Again". So that's another reason why the solution isn't perfect.

On the bright side, the other tasks (1) and (3) now seem not to disturb the user at all, since they only do small write operations once or twice per task. So once the remote repositories have been updated initially, the user should almost never see the database locked screen.

I updated the Debian package and invite you to try it out for yourself.

Cool. I just tired the deb on a clean debian stretch vm but I ran into this issue:

pkg_resources.ContextualVersionConflict: (pyasn1 0.1.9 (/usr/lib/python3/dist-packages), Requirement.parse('pyasn1==0.3.2'), {'pyasn1-modules'})

Apparently the version of pyasn1 on stretch is too old to pass this check, but I'm using 0.1.9 for development and it works fine for me.

On a side note, pyasn1-modules was not installed at all, maybe that dependency needs to be added at some point.

@uniqx thanks a lot for testing and the bug report!

I uploaded a new version that should fix the problem. Can you confirm?

Back to the topic of this ticket, what do you think about the locked database problem? Is this sufficiently taken care of with the latest modifications?

I uploaded yet another version that already comes with its own window (#174 (closed)). After installing the package, you should just type repomaker and get the app starting up.

The initial database locking issue is improved there further by waiting 30 seconds after starting the app before running background tasks.

Testing feedback is welcome!

Seems I am faster than the feedback. repomaker_0.0.4_all.deb should now add an entry to the start menu for Repomaker, so every user can easily start it without needing to resort to the command line.