Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Give the Rust portion of Synapse access to the database #16501

Open
reivilibre opened this issue Oct 16, 2023 · 0 comments
Open

Give the Rust portion of Synapse access to the database #16501

reivilibre opened this issue Oct 16, 2023 · 0 comments
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Rust concerns the Rust portions of Synapse

Comments

@reivilibre
Copy link
Contributor

reivilibre commented Oct 16, 2023

Eventually it seems like we will want to get the Rust parts of Synapse some access to the database so we can offload some of the expensive processing (including state resolution) to Rust, sidestepping the GIL and likely improving performance generally.

This is not exactly trivial.

Option 1: expose existing connections to Rust (i.e. Rust calls Python)

Good:

  • reuses our existing, well-understood database machinery

Bad:

  • calling Python whenever Rust wants to access the database means we will have to lock the GIL for database accesses (and there is probably other interop overhead too)

Option 2: move database machinery into Rust and have Python call Rust

Good:

  • Rust doesn't have to lock the GIL for database access

Bad:

  • probably means implementing a DB-API2 wrapper around the Rust client libraries (not convinced this is very hard though, but beware the unknown)
  • sacrifices our well-understood machinery; we won't necessarily be immediately confident in the transition
  • likely interop overhead from Python calling Rust (which will be the majority of the time unless we dramatically move all our DB transactions to Rust) — that said, we already pay this cost moving C-struct Postgres results across to Python, it's just done by psycopg2. It would be quite interesting to do a benchmark of a naïve Rust DBAPI2 wrapper for Postgres/SQLx/something against Psycopg2 and see if this would hurt our perf or not.

Option 3: maintain connections in both languages

Good:

  • No interop overhead

Bad:

  • We would now have two versions of each database connector in play
  • It's probably impossible to use :memory: SQLite databases (e.g. in testing) this way. Though we could put the file on a tmpfs as a likely suitable workaround.
  • Someone has to decide the connection limits for both connection pools and it's not going to be obvious to us or to end-users what that split should be.

Any others?

@reivilibre reivilibre added Z-Rust concerns the Rust portions of Synapse T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. labels Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Rust concerns the Rust portions of Synapse
Projects
None yet
Development

No branches or pull requests

1 participant