Support psycopg3 RFC - Githubissues

element-hq / synapse

Synapse: Matrix homeserver written in Python/Twisted.

https://element-hq.github.io/synapse

GNU Affero General Public License v3.0

1.48k stars 181 forks source link

Support psycopg3 RFC #14586

Open matrixbot opened 10 months ago

matrixbot commented 10 months ago

This issue has been migrated from #14586.

Description:

The latest version of psycopg is psycopg3; it supports asyncio and also works in pure python for better pypy compatibility.

In a django app that I develop, it's almost 5% faster with more optimizations coming. I'm also a synapse user and would be interested in trying to port synapse to psycopg3 for the improved performance.

Would such a PR be of interest to the synapse project?
If so, could this be a complete migration to psycopg3, or would you want the codebase to support both 2 & 3?
- If it's critical to retain the support if psycopgcffi, then both would need to be supported.
- (Given the pure-python mode, perhaps pypy performance on psycopg3 would be equivalent to psycopgcffi?)

Any thoughts on this proposal? It might be biting off more than I can chew to do a migration, but I'd like to try if the synapse maintainers are open to it.

clokep commented 2 weeks ago

@realtyem and I were looking at this again from the old branch I had... I now have a branch which passes all unit tests using psycopg3. It isn't massive, but is pretty big. I do not know if it has any performance impact yet. (And it has a few lints to fix.)

From my POV it would be reasonable to propose this in a few stages:

Support both psycopg2 & psycopg as drivers for postgres, this works by adding another "engine" and some if-defs.
After ensuring the above works OK, drop support for psycopg2.
Eventually, rework some of the innards of synapse.storage.database to be async-native, bypassing the Twisted database threadpool when using an async-compatible database driver.
(Maybe?) Replace SQLite support with an async driver.

I know @erikjohnston at some pointed mentioned trying to by-pass the Python database driver completely, but I have no idea if there's any plan to do this.

erikjohnston commented 2 weeks ago

@clokep that sounds like a decent plan.

I have been investigating what a Rust based DB pool would look like, but it's sufficiently far away that I don't think we should block upgrading from psycopg2. If/when we change the internal database APIs to be async-native we should just try and make sure that it won't make replacing it with a Rust db pool harder.

(For others following: the reason for wanting a Rust DB pool is to allow Rust modules to access the DB without having to go back into Python).