Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support psycopg3 RFC #14586

Open
matrixbot opened this issue Dec 20, 2023 · 3 comments
Open

Support psycopg3 RFC #14586

matrixbot opened this issue Dec 20, 2023 · 3 comments

Comments

@matrixbot
Copy link
Collaborator

matrixbot commented Dec 20, 2023

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
Copy link
Contributor

clokep commented Oct 10, 2024

@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:

  1. Support both psycopg2 & psycopg as drivers for postgres, this works by adding another "engine" and some if-defs.
  2. After ensuring the above works OK, drop support for psycopg2.
  3. 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.
  4. (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
Copy link
Member

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants