-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try reverting prefetch changes for project/version listing views #11621
base: main
Are you sure you want to change the base?
Conversation
agjohnson
commented
Sep 26, 2024
•
edited
Loading
edited
- Reverts Add support for successful build prefetch #11613
- Reverts Prefetch build and project on version list #11616
- Disables previously used prefetch on the project dashboard view
- Refs Try to tune project listing query again #11620
This comment was marked as off-topic.
This comment was marked as off-topic.
I'm testing this and it's actually not solving the problem with query time. I'm still seeing 6s response time. This is still because of the prefetch query that we were previously using. Something seems like it regressed here and I'm guessing it probably solves the underlying problem with these changes above too. |
Noted in chat, but it seems there is a missing foreign key on builds_build for project. I have this locally, but it is not in production:
It's definitely not clear if this helps the planner or not though, this might be another dead end. I would sooner merge in #11620 than this PR as it at least dropped the view time to 3s for my user. |
With the revert of the additional prefetch not fixing the response time, I've disabled all prefetching on the dashboard view. If there are no other ideas on resolving the response timing issues here, we can revisit prefetching to reduce query load later. Undoing the prefetch we've had in place for a while now, the response time is <1s and the query response time is instant:
There are still 120 queries for each dashboard load however. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I'm 👍 on rolling this back and thinking about some other approaches here, like perhaps the normal Django cache across processes or similar.
To be clear, this isn't just rolling back my changes. This wasn't enough to reduce response time (it's still 6s response time). This revert is removing all prefetching from the project dashboard view, added years ago here #5637. If there were performance benefits to these, maybe they got lost over time? Caching is maybe on the table, I might also lean towards making this an explicit model field or relationship. |
Yea, denormalizing the latest build ID or similar onto the model is another really obvious answer 👍 |