lemmy icon indicating copy to clipboard operation
lemmy copied to clipboard

[WIP] using db-pool library to create a pool of databases

Open momentary-lapse opened this issue 6 months ago • 11 comments

Addresses: #4979

momentary-lapse avatar Jul 04 '25 15:07 momentary-lapse

Update: I'm working on the topic; cannot devote much time for it, but it slowly going forward, and i keep the code in the branch up-to-date. I connected db-pool crate to our tests, and reworked most of them. Currently have a runtime error, planning to look at it and fix this week. After this, what is left is to change a few tests which are using build_db_pool function.

momentary-lapse avatar Jul 27 '25 23:07 momentary-lapse

Hey! I've a question about the replaceable schema — found out it's the reason tests are failing in the branch: the tests are being run in restricted privilege mode, and have no access to the r schema. I can do some workaround, but would like to ask about motivation for making that replaceable schema — for me it seems a bit unsafe, since there in theory might be migrations which require changes in these files. Also, am I right that the triggers and utils are separated into another schema because it's just easier to clean them up at once?

momentary-lapse avatar Aug 24 '25 14:08 momentary-lapse

Yes I believe the main reason is so that they're easier to clean up.

for me it seems a bit unsafe, since there in theory might be migrations which require changes in these files.

Everything in the r schema should only contain triggers and functions used at runtime, not by any migrations. They're only run once, at the end of all the migrations. The main reason to do it like this, is because it was extremely cumbersome to tweak every trigger and function within the migrations, copying and pasting every time, rather than doing tweaks to a simple sql file.

the tests are being run in restricted privilege mode

Is this a limitation of the db-pool library? Because regular cargo tests can already run the build_db_pool_for_tests function which runs the replaceable schema.

dessalines avatar Aug 25 '25 14:08 dessalines

Is this a limitation of the db-pool library?

Yep, it's designed the way you have privileged connection pool to prepare dbs and do migrations, and restricted connection pool to run tests. And the latest can only access public schema of each db. So, the library would work out of the box if we had these runtime functions in the same schema

momentary-lapse avatar Aug 25 '25 19:08 momentary-lapse

Maybe its just a matter of running GRANT for the current lemmy user on that schema, as part of the setup?

https://stackoverflow.com/questions/17338621/what-does-grant-usage-on-schema-do-exactly

cc @dullbananas

dessalines avatar Aug 25 '25 20:08 dessalines

that's an option i'm thinking about. but unfortunately it has certain caveats. what does the db-pool library do, it prepares multiple databases with names like:

  • db-pool-uuid1
  • db-pool-uuid2 etc. and it creates a separate role with restricted rights for each db named exactly as this db. so to grant permissions after the migrations are done here, i need to know the current db name, which is afaik impossible to retrieve from the connection only.

however, i can change it in a fork, so it will use the same role for all the restricted connections (e.g. _db_pool_test_role). then we can grant access to schema r for that user after migrations are done

momentary-lapse avatar Aug 25 '25 21:08 momentary-lapse

Maybe its just a matter of running GRANT for the current lemmy user on that schema, as part of the setup?

Maybe. The problem is almost certainly caused by something being different between the "public" schema and the "r" schema. Otherwise migrations would have failed before replaceable_schema runs.

dullbananas avatar Aug 29 '25 20:08 dullbananas

There's logic in the db-pool library which grants permission to restricted user fro public schema assuming it's the only one tests will deal with. Anyway, i reported that, and we agreed he'll take time to think how to change that. So, we're in touch. And if he decides to keep the logic, i have some ideas for a workaround for our case

momentary-lapse avatar Aug 29 '25 22:08 momentary-lapse

What's this needing still? This is one of our oldest PRs.

dessalines avatar Dec 01 '25 14:12 dessalines

  1. Make the config changes mentioned above by @Nutomic

  2. Check that tests with tokio_shared_rt work. I added it to db_views crate.

  3. Add that tokio shared runtime library to other tests.

Basically, that's it. But other errors might appear.

But yeah, apologies for horrendously delaying this PR. It required communication with another dev and adapting their library, but it's still too long

momentary-lapse avatar Dec 01 '25 18:12 momentary-lapse

Cool, no probs. I just know its a pain to maintain these things when we do a lot of refactor PRs, so its best to try to get the oldest ones merged first if possible.

dessalines avatar Dec 02 '25 16:12 dessalines