[WIP] using db-pool library to create a pool of databases
Addresses: #4979
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.
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?
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.
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
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
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
Maybe its just a matter of running
GRANTfor 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.
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
What's this needing still? This is one of our oldest PRs.
-
Make the config changes mentioned above by @Nutomic
-
Check that tests with tokio_shared_rt work. I added it to
db_viewscrate. -
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
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.