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

[momentary-lapse]

Addresses: #4979

[momentary-lapse]

I created this WIP PR to share the progress and the issue I'm stuck with currently. The crate I use operates with its own structure wrapping connection pools: code
And we have our own ActualDbPool. They are kinda same, but it's not obvious for me how to correctly convert one to another.
I had an idea to make ActualDbPool a enum with two possible values: RegularPool and ReusablePool, but stuck on trying to adapt stuff like LemmyContext, which also requires pool struct to be clone-able (and ReusablePool is not). And it seems a lot of changes to the main codebase for purely test changes.
Do you folks have any ideas how to manage that? Or should I stick to the initial plan without using this library?

[dessalines]

Our ActualDbPool is just a type alias for deadpool Pool<AsyncPgConnection>.

Their crate should be able to work with deadpool pools, but I'm not familiar with how to plug that into their crate... you'll have to ask them.

[momentary-lapse]

Yeah, I see. I returned to this issue today after a week of a break. I'm in contact with the db-pool author and they're helping to understand a lot of moments and really willing to collaborate, so i think we'll make this work.

I'd like to clarify one moment: do we want build_db_pool_for_tests to return still ActualDbPool? db-pool has its own wrapper ReusableConnectionPool which works like a deadpool Pool, but a bit different and needs adaptation. And it might be easier to adapt tests for working with ReusableConnectionPool than converting ReusableConnectionPool to ActualDbPool

[dullbananas]

The return type of build_db_pool_for_tests may be changed. Also, a DbPool variant may be added if needed.

[momentary-lapse]

This is the moment which currently blocks me, and i think it's better to consult with you again. LemmyContext structure must be cloneable, therefore all the fields, therefore the pool. But unfortunately, reusable pool from db-pool crate is not, and i don't have access to its fields to implement the trait here.
But before asking db-pool developer, i'd like to be sure we really need this pool cloning stuff, especially for the tests. Cloning the pool seems a bit strange to me, but i may miss something. I'm looking at the code now, but maybe you folks already have some insights on this

[dullbananas]

Wrap it in Arc for now.

[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.

[Nutomic]

@dessalines Lets merge this now so I dont have to keep resolving conflicts. I did some cleanup and tried some changes to make tests faster (but not much difference).

Only remaining problem is the git dependency for db-pool so we have to work on getting those changes merged upstream (#6583). So crates.io publishing will not work for now. Still I want to make some more changes to db code and avoid creating more conflicts.

[momentary-lapse]

Thanks much for driving this lately while I was absent because some health problems. Yeah, would be nice to merge it to stop syncing the branch as the changes here cover literally whole repo... I'll find time (probably this weekend or Monday next week) to prepare PRs for db-pool

[dessalines]

Its failing clippy

[Nutomic]

Clippy is passing, but there is a problem with tests...

https://woodpecker.join-lemmy.org/repos/129/pipeline/20897/17

Edit: The tests in crates/apub/send/src/ dont complete. Maybe because some of these tests open a socket on localhost, and running in parallel breaks that. Strangely 074d3c9 also has those changes and tests are passing there.

[momentary-lapse]

Clippy is passing, but there is a problem with tests...

https://woodpecker.join-lemmy.org/repos/129/pipeline/20897/17

Edit: The tests in crates/apub/send/src/ dont complete. Maybe because some of these tests open a socket on localhost, and running in parallel breaks that. Strangely 074d3c9 also has those changes and tests are passing there.

Yeah, as I remember they try to open the same port and conflict with each other. I tried to make a more complex setup where each test chooses their own port, but tests started hanging without any error messages, and I decided to left them sequential for now. Maybe it's worth to fix them out of scope of this ticket.

[momentary-lapse]

FYI I also created a first PR into db-pool library: yasamoka/db-pool#4
Waiting for review and approval of the repo maintainer

[momentary-lapse]

I'm also trying to reduce my changes on the side of db-pool. Currently errors during the tables cleanup were suppressed, because some of the tables couldn't been cleaned without activated triggers.
So, I have couple of questions:

• Do we need utils schema and deps_saved_ddl?
• Can we remove forbid_diesel_cli trigger or at lease allow to make TRUNCATE operation?

UPD. Looks like we can leave it like that, it's just not necessary to truncate all the tables before acquiring a db in our case.

[dessalines]

FYI I also created a first PR into db-pool library: yasamoka/db-pool#4
Waiting for review and approval of the repo maintainer

We can also use a git dependency on your branch if necessary.

Do we need utils schema and deps_saved_ddl

We do need utils and replaceable_schema, because a lot of the tests depend on the triggers.

[Nutomic]

This time theres a different test failure:

failures:

---- site::registration_applications::tests::test_application_approval stdout ----

thread 'site::registration_applications::tests::test_application_approval' (15835) panicked at crates/api/api/src/site/registration_applications/tests.rs:149:3:
assertion `left == right` failed
  left: 1
 right: 0

https://github.com/LemmyNet/lemmy/blob/main/crates/api/api/src/site/registration_applications/tests.rs#L151

So if you dont clear tables between test runs, some tests will fail because counts are wrong.