Rank the quick-switch list on ids and pins, then load only the members it returns - #319
Merged
Merged
Conversation
…s it returns GET /v1/members/top-fronters scored the roster in the database already (one aggregate query, tens of milliseconds against fifty thousand fronts), then loaded every member of the system as a full ORM row, encrypted bio and note included, to sort by pin and score and return at most eight. On a large system with long bios that hydration was the entire cost of the request, and it ran on every quick-switch poll: the route's p50 sat at 40 ms while a slow tail of about one request in six took two seconds, which is one big system's client polling. The ordering now runs on a projection of id and quick_switch_pin for the whole roster, and the full rows are loaded afterwards for the ids that survive the limit, in ranking order. Same result, same ordering rules, same tiebreakers; the existing endpoint and parity tests pin that. Measured on a scratch database with 1,500 members carrying 20 KB bios: 151 ms for the old load, 1.7 ms for the projection. Also adds sheaf_system_member_count_max and sheaf_systems_by_member_count next to the other per-system maxima and distributions. The roster is what every load-everyone endpoint scales with, and there was no way to see the largest one without a scratch database.
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.
GET /v1/members/top-fronters(the quick-switch list) has had a slow tail for a while: p50 around 40 ms, p99 parked at the 2.5 s bucket edge for hours at a stretch, at about fifty requests an hour. That shape is one client's every request being slow, not a slow query for everyone.What it wasn't. The scoring query. Seeded a scratch database with 51k fronts over two years and ran EXPLAIN ANALYZE on the exact statement: 19 ms, using the
(system_id, ended_at)index through a BitmapOr, 12.6k rows in the window.What it was. The line after it. The handler loaded every member of the system as a full ORM row, encrypted bio and note included, to sort by pin and score and return at most eight. With 1,500 members carrying 20 KB bios that load is 151 ms on a fast local box; production's largest system has 15,682 fronts and 1,641 groups, and a roster to match, on a slower box, polled every time the quick-switch list opens.
The change. The ordering runs on a projection of
idandquick_switch_pinfor the whole roster (1.7 ms for the same 1,500 members), then the full rows are loaded for the ids that survive the limit, in ranking order. Same result, same ordering rules, same tiebreakers; the existing endpoint tests and the SQL/Python parity test pin that, and a new test checks the returned rows are fully hydrated (bio present) rather than the projection leaking through.Plus one gauge.
sheaf_system_member_count_maxand itssheaf_systems_by_member_countdistribution, next to the other per-system maxima. The roster size is what every load-everyone endpoint scales with, and until now there was no way to see the largest one without a scratch database.Not confirmed on production (that would need
log_min_duration_statementfor a day); the fix is correct and cheap regardless, so ship it and watch the route's p99.