Skip to content
This repository has been archived by the owner on Aug 9, 2024. It is now read-only.

perf: move heavy queries to prepared statements #78

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ItzDerock
Copy link

@ItzDerock ItzDerock commented Aug 7, 2024

I noticed that many of the feed queries end up being huge SQL statements -- nearly 4,600 characters for the main feed query. Every time someone visited the site, hit the feed API, etc, this query would be re-sent to the postgres server and the server would have to re-parse the query, re-plan and re-analyze the query, re-build the query tree, and execute.

From the postgres docs:

Prepared statements potentially have the largest performance advantage when a single session is being used to execute a large number of similar statements. The performance difference will be particularly significant if the statements are complex to plan or rewrite, e.g., if the query involves a join of many tables or requires the application of several rules.

and I'd say that definitely applies to the feed queries which have multiple subqueries, joins, etc etc.

This PR addresses this by moving all feed queries and a few other longish queries into prepared statements. I did some very preliminary testing with this, and it all seemed to work fine, but I'd recommend running some more extensive testing before pushing to prod.

@face-hh
Copy link
Owner

face-hh commented Aug 7, 2024

Hey! Thanks for the contribution - we will look over it soon.

Are the nix files required? If not, could you remove them from the PR?

@ItzDerock
Copy link
Author

Yeah I can remove those, the nix files are for a dev environment. I think I also may have left a stray console.log somewhere, will clean that up too.

@xyzeva
Copy link
Collaborator

xyzeva commented Aug 7, 2024

hey! any updates?

@ItzDerock
Copy link
Author

ItzDerock commented Aug 7, 2024

Currently away from my computer, but you all should have push perms on this PR so feel free to checkout, delete nix files, and push.

The GitHub Mobile app doesn't let me delete files, and I don't have my signing key set up on phone.

@ItzDerock
Copy link
Author

@xyzeva / @face-hh the extra files should be removed, PR should be good to go.

@face-hh
Copy link
Owner

face-hh commented Aug 7, 2024

I'll check this tomorrow

@face-hh
Copy link
Owner

face-hh commented Aug 9, 2024

Hey, sorry for not checking it out yesterday.

This pull request has some conflicts on api/comment and api/feed. Could you @ me whenever you resolve them?

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

Successfully merging this pull request may close these issues.

3 participants