Repository navigation
Conversation
The series list, series detail, and publisher series_list endpoints annotated num_issues with Count("issues", distinct=True). That joins and groups every issue row before sorting and paginating, and the pagination COUNT(*) repeats the join. On production-sized data (184k issues) this took ~450ms for /api/series/ and ~200ms for a large publisher's series_list on a cache miss.
Use a correlated subquery instead. Postgres evaluates it only for the rows on the requested page, and the pagination COUNT(*) leaves it out. On the same data /api/series/ dropped to ~5ms of SQL and series_list to ~4ms. Add a test covering issue_count for series with and without issues.
/api/series/ and publisher series_list ordered by (sort_name, year_began), which isn't unique: 506 series share a value with at least one other series. Postgres may return tied rows in a different order on each query, so a client paginating the list could see a series twice and miss another. Add id as a final tiebreaker. It has no measurable cost, since the existing (sort_name, year_began) index still drives the sort.
Arc, Character, Creator, Imprint, Publisher, Team, Universe, Series, and Issue all ordered by fields that aren't unique (e.g. 129 characters share a name with another character). Postgres may return tied rows in
a different order on each query, so paginated API and web lists could show a row twice and miss another. Add id as a final ordering key on each model.
This also covers the issue_list actions, whose order_by("series") expands to Series' Meta.ordering. The migration only changes model options and runs no SQL.
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
1 participant
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.
Summary
Profiling the API against a production-sized database found one slow query pattern and one pagination bug.
Faster series issue counts
/api/series/, series detail, and/api/publisher/<id>/series_list/annotatednum_issueswithCount("issues", distinct=True). That joins and groups every issue row before sorting and paginating, and the paginationCOUNT(*)repeats the join. It now uses a correlated subquery, which Postgres evaluates only for the rows on the requested page./api/series//api/publisher/1/series_list//api/series/?name=spider-manCounts are unchanged; a series with no issues still reports
0.List ordering
Core metadata models were ordered by fields that aren't unique. For example, 506 series share
(sort_name, year_began)with another series, and 129 characters share a name. Postgres can return tied rows in a different order on each query, so clients paginating a list could see a row twice and miss another.idis now the finalMeta.orderingkey on Arc, Character, Creator, Imprint, Publisher, Team, Universe, Series, and Issue. This fixes the API and web views together. It also covers theissue_listactions, whoseorder_by("series")expands to Series' ordering. The series API views now rely onMeta.orderinginstead of their ownorder_by.Migration
0061_ordering_tiebreak_by_idonly changes model options and runs no SQL. Addingidhad no measurable effect on query times.Testing
test_issue_count(series with and without issues, across list,series_list, and detail) andtest_list_orders_ties_by_id.