Classification: fix N+1 queries in the allele groupings grid and tasks #1729 - #1731
Open
davmlaw wants to merge 1 commit into
Open
Classification: fix N+1 queries in the allele groupings grid and tasks #1729#1731davmlaw wants to merge 1 commit into
davmlaw wants to merge 1 commit into
Conversation
…#1729 The allele groupings grid walked latest_classification_modification -> classification -> allele_info as lazy FK loads per grouping per row. ClassificationGrouping.update() already stores that as latest_allele_info, so read it directly and select_related the resolved builds. The labs column prefetched classificationgrouping_set but not lab, and sorting labs touches organization, so pull those through too. DiscordanceReport.update() dereferenced classification_original.classification per row, then fetched each newly added modification one at a time - now one query for the lot. Note this skips a classification with no is_last_published modification where the previous get() would have raised. Also: update_all_dirty allele_origin_grouping, and the nightly ClinVar prepare running two counts per allele.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Written by Claude.
Addresses part of #1729 — the classification fixes that are just
select_related/prefetch_relatedor hoisting a query out of a loop. Split out of #1727; the snpdb/annotation half is in a separate PR.Allele Groupings grid
The worst of them.
render_alleleruns aClassificationGroupingquery per row, thenc_hgvs_forwalkedlatest_classification_modification -> classification -> allele_info -> allele_info[gb] -> ri.genome_buildas lazy FK loads for every grouping of every row.ClassificationGrouping.update()setslatest_allele_infofrombest_classification.classification.allele_infoin the same breath aslatest_classification_modification, so the stored field is exactly what that chain resolves to — read it directly andselect_relatedthe resolved variant info for both builds.The Labs column had the same problem from the other end:
allele_origin_dictprefetchedclassificationgrouping_setbut notlab, andsorted(labs)callsLab.__lt__, which reaches fororganization. Both now come through the prefetch.DiscordanceReport.update()Runs on every classification publish/withdraw touching a discordant clinical context. Two problems:
classification_original.classificationwas dereferenced per row (two queries each, one of them the wideClassificationModification), and each newly added classification was fetched with its own.get(). Now oneselect_relatedqueryset for the existing rows and one bulk query for the added ones.Behaviour note for review: the old
.get(is_last_published=True, classification=vcm_id)raisedDoesNotExistif a classification had no published modification, failing the whole update. The bulk query skips it instead. I think skipping is the better behaviour, but it is a change, so flagging it.Others
update_all_dirty—update()dirtiesallele_origin_groupingimmediately, so it is worth bringing along; this runs over tens of thousands of groupings after a bulk import.COUNTqueries per allele; collapsed into oneaggregatewith a filteredCount.Not included
The remaining classification grid items in the issue (
imported_allele_info_view,clinvar_export_view) need thepre_render(qs)hook to bulk-load a page's objects into a dict. That is a consistent little pattern rather than a one-line change, so it is better as its own PR than bolted onto this one.Testing
python3 manage.py test --keepdb classification.tests— 98 tests pass.