dendrite icon indicating copy to clipboard operation
dendrite copied to clipboard

Add SQLite WAL and busy_timeout

Open sfPlayer1 opened this issue 1 year ago • 1 comments

SQLite has rather unfortunate defaults for concurrent accesses, leading to database locked errors. https://github.com/matrix-org/dendrite/pull/1290 appears to try to fix some of this, but doesn't appear to cover a concurrent read+write and the errors still happened in large quantities when I joined the dendrite room.

This PR enables WAL for concurrent read+write and much better performance. It also sets busy_timeout to 10 seconds to not immediately fail operations while the database is locked, providing a DB level solution to PR 1290's objective with additional coverage.

I chose this particular location to add the two statements as they should apply to every SQLite DB connection.

From a quick test it looks as if the db locked errors are gone with this change.

Pull Request Checklist

  • [ ] I have added added tests for PR or I have justified why this PR doesn't need tests.
  • [ ] Pull request includes a sign off

Signed-off-by: Your Name <[email protected]>

sfPlayer1 avatar Aug 11 '22 06:08 sfPlayer1

Oh, additionally, if we are to accept this PR, you need to update the sign-off line as per the PR template.

neilalexander avatar Aug 11 '22 13:08 neilalexander

We recently updated our contributing guidelines. This PR is being closed because it isn't a feature we want to maintain going forwards. Supporting WAL is nice but adds extra risks with the extra concurrency it allows. We currently have a mechanism to limit concurrency on SQLite which we are happy with overall. We feel that landing this PR would increase the overall maintenance burden of SQLite mode due to the additional race conditions exposed with extra concurrency.

When we have more bandwidth as a team, we would be interested in adding support for this.

kegsay avatar Dec 05 '22 17:12 kegsay