opensafely-core / opensafely-core/opencodelists

Improve concurrency requirements to avoid `database is locked` errors

Open
#2,251 13 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
60
Forks
16
Avg merge
4d 12h
Merged PRs (30d)
17

Description

Why are we doing this?

There are regular Sentry alerts for several Issues where requests fail with OperationalError: database is locked errors. The root cause is that our level of concurrency is currently too high for SQLite to reliably handle as currently configured. The database is locked while code writes to the database. This can result in failure after a long timeout of requests making updates, which is poor UX and could lead to their updates getting lost.

We know from Honeycomb several of our views can respond slowly, taking even 10s of seconds. Possibly they are holding a database lock for an extended period. Interacting with the builder to build a codelist or search for code can both write to the database, leading to updates that need the lock occurring frequently. Possibly a queue of updates builds up and some time out before processing and the request fails.

How will we know when it's done?

There are fewer instances of OperationalError: database is locked errors, as monitored through Sentry.

This Issue might have a number of sub-issues.

What are we doing?

According to https://docs.djangoproject.com/en/4.2/ref/databases/#database-is-locked-errors:

SQLite is meant to be a lightweight database, and thus can’t support a high level of concurrency. OperationalError: database is locked errors indicate that your application is experiencing more concurrency than sqlite can handle in default configuration. This error means that one thread or process has an exclusive lock on the database connection and another thread timed out waiting for the lock the be released.

Python’s SQLite wrapper has a default timeout value that determines how long the second thread is allowed to wait on the lock before it times out and raises the OperationalError: database is locked error.
If you’re getting this error, you can solve it by:

  • Switching to another database backend. At a certain point SQLite becomes too “lite” for real-world applications, and these sorts of concurrency errors indicate you’ve reached that point.
  • Rewriting your code to reduce concurrency and ensure that database transactions are short-lived.
  • Increase the default timeout value by setting the timeout database option:

According to the SQLite docs:

If your application has a need for a lot of concurrency, then you should consider using a client/server database. But experience suggests that most applications need much less concurrency than their designers imagine.

It's unclear if our concurrency needs are fundamentally so high that we should switch to another database backend or if we can resolve the issue by changing configuration or rewriting our code.

Possible actions:

  • #2252 We already increased the timeout to 45 seconds in opencodelists/settings.py. We could try increasing it further to 60 or 90 seconds and monitoring how this improves the situation.
  • #2268 Explore other configuration options that can help such as write-ahead log mode if we don't already use it.
  • Identify problematic views and code and improve them to require less concurrency by holding the lock for less time or taking the lock less often.
    - We know the builder sends a request to the server every time a checkbox is clicked, which may be multiple times a second, which should probably batch such requests. We know we want to improve or replace the builder in general.
    • It would be useful to be able to easily identify traces in Honeycomb that may write to the database, perhaps the URL+method where the errors are triggered can help with this. This could be a derived column for easy future reference. Possibly something could be added to the raw OTel data.
  • Consider splitting the main database into more separate databases, as each has a separate locking mechanism. So views updating some models need not necessarily block views updating different models. codelists/builder is where most of the work is done though.
  • Rather than changing the database, another approach would be to reduce the write load by not updating the database on every interaction with the builder. This would mean that users would have to explicitly save their changes, or do some more sophisticated batching queueing and retrying in the background in the SPA.
  • Consider whether we need to move to a server-client database like postgres. Notes from #1839

I have not thought in much detail about what replacing SQLite with Postgres would look like. Theoretically, replacing the main db should be pretty straightforward, and job-server can be used as a template, infrastructure-wise. This would likely solve the database-locking issues (at least I'd hope it would), because my assumption is that those are all related to the main db.
The issue is what to do about the coding system databases. SQLite is an easy option with the current setup, because it's trivial to create a new sqlite db for each coding system import. There may be a better solution to maintaining the historical releases. Django can certainly have multiple databases of different types, so switching to Postgres doesn't mean we can't also use sqlite dbs. If we were to keep the current solution, it may be possible to have the default db be postgres, and the coding system dbs continue to be sqlite dbs as they currently are. However, I've never done that before in a django project, so I'm not sure what pitfalls there might be when the main (postgres) db needs to talk to a coding system db (although, this shouldn't happen often - the bnfdmd mapping may be the only place still left - have a look at the database router in opencodelists/db_utils.py).


Defining delivery tasks guidance

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the SQLite configuration in opencodelists/settings.py and the database routing in opencodelists/db_utils.py, then read the related work in issues #2252 and #2268. Use the linked Sentry query and Honeycomb board to identify where locking errors occur. Done means fewer OperationalError: database is locked incidents, measured through Sentry.

Written by the indexing model from the issue text.

Assessment

Tech stack
django, python, sqlite
Domain
backend, databases, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.