perf(api): scope session event aggregation to the listed page#6433
Merged
gustavosbarreto merged 1 commit intoJun 6, 2026
Merged
Conversation
The session list built event_types and event_seats from derived tables that ran string_agg over the whole session_events table with no filter (GROUP BY session_id). Postgres scanned and grouped every event row on each request even though the list returns a single page, so the cost scaled with the total event count instead of the page size. On a busy namespace this becomes a multi-second full scan of session_events. Replace the derived tables with correlated subqueries scoped to each session row. They run only for the page's sessions and use the existing session_events_session_id_created_at_idx index. On a 100k-session, 5M-event dataset the list dropped from ~12s to ~3ms, and the time no longer grows with the table.
|
Claude finished @gustavosbarreto's task in 3m 33s —— View job Code Review CompleteReviewed 1 file across code quality, security, testing, Go patterns, and architecture — no issues found. The code looks good as-is. The change correctly replaces the derived-table LEFT JOINs with correlated subqueries scoped to each session row, eliminating the full-table scan on To request another review round, comment |
otavio
approved these changes
Jun 6, 2026
otavio
approved these changes
Jun 6, 2026
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.
What
The session list (
SessionList) built theevent_typesandevent_seatscolumns from derived tables that aggregate the entire
session_eventstable on every request:
Because the derived table isn't correlated, Postgres has to scan and
GROUP BYevery row insession_eventsbefore it can join and apply theLIMIT. The cost scales with the total number of events, not thepage size, so the query degrades into a full scan (two of them, plus
external-merge sorts that spill to disk) as the table grows.
This replaces the derived tables with correlated subqueries scoped to
each session row, so the aggregation runs only for the sessions on the
page and uses the existing
session_events_session_id_created_at_idxindex.
Why
session_eventsis one of the largest tables in a busy deployment. Thelist query was taking several seconds and getting slower the more the
namespace was used, even though every page only returns a handful of
rows. The pagination total + page count made the full scan visible.
Measured (local, synthetic dataset)
EXPLAIN (ANALYZE)of the list query, page of 10:Before: 2×
Seq Scan on session_events+ external-merge sorts to disk.After: index scan on
sessions(LIMIT 10) + correlated index lookups.The "after" time stays flat as the table grows; the "before" grows with it.
How to verify
Open the sessions list on a namespace with a large
session_eventstable and compare response time, or run
EXPLAIN ANALYZEon thegenerated query before/after — the
Seq Scan on session_eventsshouldbe gone.
Note
SessionSelectQuerylost itssessionIDparameter (the correlated formmakes it unnecessary). The cloud repo calls this exported helper and is
updated in a paired PR: shellhub-io/cloud.