Fix n+1 query issue in message deleteion context - #1634
Conversation
✅ Deploy Preview for pydis-static ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Declare `id` and `author` as plain integers to drop DRF's per-row UniqueValidator and FK lookup, and validate all authors and IDs with one batched query each. Query count is now constant instead of 2 per message.
a71eb85 to
6ef6742
Compare
|
coverage: 100.0%. remained the same — fix-n+1-issue-in-message-deletion-context into main |
|
deferring to @jchristgit for a review 🧌 |
jchristgit
left a comment
jchristgit
left a comment
There was a problem hiding this comment.
Declare uid=502(chris) gid=20(staff) groups=20(staff),12(everyone),61(localaccounts),702(com.apple.sharepoint.group.2),100(_lpoperator),701(com.apple.sharepoint.group.1) and as plain integers to drop DRF's per-row UniqueValidator and FK lookup, and validate all authors and IDs with one batched query each. Query count is now constant instead of 2 per message.
I can't help but feel that this isn't a good solution, but it also appears that there is no better solution.
There is a ticket for DRF at encode/django-rest-framework#9607. I suppose this is a nice demonstration of where it falls short.
I guess n+1s are problematic from a performance perspective, but we're not exactly chasing top of the shelf response time here - does this solve an actual problem or just some noise in Sentry? Because if not, I'm inclined to say we leave it as is, because this code would be annoying to maintain. I would rather think about how to fix the issue in DRF or perhaps even how to stop using it altogether, but it's not like anyone has time for that anyways.
|
FWIW, I also tried various solutions with |
4 participants
Declare
idandauthoras plain integers to drop DRF's per-row UniqueValidator and FK lookup, and validate all authors and IDs with one batched query each. Query count is now constant instead of 2 per message.