perf: paginate combined events with DB-level UNION ALL instead of in-memory sort#2666
Open
mroderick wants to merge 1 commit into
Open
perf: paginate combined events with DB-level UNION ALL instead of in-memory sort#2666mroderick wants to merge 1 commit into
mroderick wants to merge 1 commit into
Conversation
…memory sort Extract fetch_upcoming_events and fetch_past_events into two new methods: - paginated_events(upcoming:) — builds a UNION ALL across workshops, meetings, and events with LIMIT/OFFSET at the database level - load_events(rows) — fetches only the 20 visible rows with full eager loading, preserving UNION sort order Previously, all past workshops (~1100) were loaded into AR objects and sorted/paginated in Ruby. Now only the current page's 20 rows ever leave the database. Benchmark (first load / cached): Past events page: 11.3s → 1.3s DB queries count: ~119 → 118 (same — view N+1s unchanged) Allocations: 25M → 2.9M (cached)
2 tasks
Collaborator
Author
|
The numbers listed above are from local development. |
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.
Problem
Past events page loads ALL workshops (~1100), meetings, and events into ActiveRecord objects, then sorts and paginates to 20 in Ruby via
#sort_by(&:date_and_time).reverse+pagy(sorted, items: 20). The remaining ~1080 rows are instantiated and immediately discarded.Same pattern exists for upcoming events, though less impactful at lower volumes.
This cost grows linearly with total records regardless of page size.
Solution
Replace the in-memory merge with a
UNION ALLat the database level:paginated_events(upcoming:)— builds three Arel subqueries (workshops x chapters.active, meetings, events), combines them with UNION ALL, runs a COUNT(*) for Pagy, then fetches only 20 (id, event_type) rows via LIMIT/OFFSET.load_events(rows)— dispatches the 20 IDs to their respective model queries with full eager loading (includes), preserving the UNION's sort order via filter_map.Pure Arel AST nodes (not extracted
.arelfrom scopes) to avoid bind parameter numbering issues in UNION context.Both
fetch_upcoming_eventsandfetch_past_eventsshare the same machinery; theupcoming:flag controls sort direction and date comparison.Benchmark
Tested on
GET /events/past(1056 past workshops in DB):The DB runtime is stable because LIMIT 20 fetches 20 rows regardless of total data volume — this scales to 10k, 100k, or more.
The remaining ~118 queries come from per-workshop N+1s in the view layer (sponsors, host, permissions, organisers) — unchanged between both versions, so comparison is apples-to-apples.
Tech Notes
Pagy::Offset.new(Pagy v43.5 API) with a manual Pagy::Request for URL generation:hostfrom Workshop includes (Bullet AVOID warning); uses:workshop_hostinsteadorigin/bullet-n-plus-one-fixes(PR Fix N+1 queries detected by Bullet #2665) for fair benchmarking — excludes N+1 query noise from the comparison