improve admin and mod check to not do seq scans and return unnecessary data

[phiresky]

the PersonView::admins query currently does a seq scan on all persons because admin is not indexed.

Also, the CommunityView::is_mod_or_admin function does a seq scan on all persons because it uses PersonView::admins.

is_mod_or_admin is used in tons of places, e.g. createpost.

This PR adds an index on the admin field to improve the api/v3/site and community fetches.

It also replaces the is_mod_or_admin function to just return a boolean and also not do any seq scans.

This should improve page load times that fetch admins (e.g. / ) and also everything.

The PersonView::admins is the second-most expensive query after #3482 on lemmy.world with 1.5 million calls returning 10 million rows and 27 hours of execution time in one day. This should reduce that to almost zero.

I did not test this.

[Nutomic]

Clippy is failing, run ./scripts/fix-clippy.sh

[phiresky]

There's an inconsistency where the fix-clippy script removes wildcard imports but the CI does not (clippy::wildcard_imports). Which one is correct?

[Nutomic]

wildcard_imports should be denied everywhere. Related to #3293

[phiresky]

I've removed the new migration and changed the other one.

[dessalines]

I've removed the new migration and changed the other one.

You're gonna hate me for saying this, but add the new migration again, and drop that index before re-creating it. The reason is because some of our test instances, as well as our dev setups, have already run that migration. In general, its a bad idea to edit old migrations, especially if they have been merged into main.

[phiresky]

Yeah I know. The other one already on lemmy.world. That's why I made it use IF NOT EXISTS. I don't think the difference is that important for dev setups?

I also didn't really want every instance operator doing an upgrade to have to go through two index creations. since that increases the downtime. especially the previous one that has to insert all person rows.

let me know

[dessalines]

I'd still prefer the new migration I think, especially because the old one has a lot of other additions in it. The index didn't take more than 10s to create on a prod database locally, so it should be fine.

[phiresky]

ok i guess. i've readded the other migration. hopefully this will cover all cases.

[dessalines]

This is still editing that older migration, make sure this file isn't changed.

[phiresky]

that's intentional. i have to change it because some instances already have the other one applied.

[phiresky]

it's not a content change, it just makes it not throw if the other migration is already applied.

[dessalines]

Needs a merge from main, but then its good.

[Nutomic]

cargo fmt is also failing.

Looks like you sneaked in a fix for the markdown-it crash as well, that as fast! markdown-it-rust/markdown-it#26

[phiresky]

i swear i checked the damn formatting. fixed

[dessalines]

I've seen that error before on CI and locally, its because you added the Cargo.lock to this PR, which is unecessary.

[phiresky]

i've pushed the reformat of api tests with prettier 3.0 to the same branch. all other prs will also fail because of this i guess

[phiresky]

oh god i think it's non-deterministic, sometimes using prettier 2 and sometimes prettier 3? fixed by specifying specific prettier version