Fix N+1 query on registration_policy/buckets in SignupCountPresenter - #11880
Merged
nbudin merged 1 commit intoAug 5, 2026
Merged
Conversation
…for_runs Sentry showed a registration_policies + registration_policy_buckets query pair repeating per run (33-100x per page load) across AppRootLayoutQuery/CmsPageQuery, Convention.event_categories, and similar entry points -- all funnel through SignupCountPresenter.for_runs (directly, or via RunAvailabilityPresenter, which wraps it), which batches the signup-count SQL across runs but never preloaded run.event.registration_policy.buckets before constructing a presenter per run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
nbudin
deleted the
fix-signup-count-presenter-registration-policy-n-plus-one
branch
August 5, 2026 20:33
This was referenced Aug 6, 2026
nbudin
added a commit
that referenced
this pull request
Aug 6, 2026
…#signups Sentry showed a new registration_policy_buckets-by-id N+1 (INTERCODE-186/187/189) after #11880 shipped -- it had been masked by the much larger registration_policy/buckets N+1 that #11880 fixed, and became the dominant N+1 once that one was gone. Traced it to the "My Schedule" widget (the user_signups/signup_bucket_description CMS partials, present by default on most conventions' sites), which calls signup.bucket.name / signup.requested_bucket.name per signup. UserConProfileDrop#signups already preloads several associations but never picked up :bucket/:requested_bucket -- these only became real belongs_to associations in #11871 (they used to be plain string columns needing no extra query at all), so this gap predates #11880 but was previously hidden under the louder N+1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
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.
Summary
Sentry surfaced a cluster of N+1 queries after #11871/#11872 shipped (INTERCODE-176/178/17C/179/17D/17B/17E), all showing the same repeated pair, 33-100 times per request:
across page-load queries (
AppRootLayoutQuery/CmsPageQuery, rendered via CMS Liquid tags),Convention.event_categories, and signup mutations.Root cause
SignupCountPresenter.for_runs(runs)batches the signup-count SQL across all runs in one query, but constructs aSignupCountPresenterper run without preloadingrun.event.registration_policy.bucketsfirst. Each presenter's#registration_policy/#bucketsthen hits that association fresh — once per run.RunAvailabilityPresenter.for_runswrapsSignupCountPresenter.for_runsand has the same exposure through its own#registration_policy/#bucketsdelegation, so it inherits the fix for free once the underlying batch is preloaded.This is a sibling gap to the N+1 #11869 already fixed: that PR wired
registration_policy/bucketsinto the GraphQL dataloader (association_loaders) forEventType/EventProposalType/RegistrationPolicyType, butSignupCountPresenter/RunAvailabilityPresenter(used by CMS Liquid drops and theSources::SignupCountdataloader source) never got the same treatment.Fix
SignupCountPresenter.for_runsnow preloadsevent: { registration_policy: :buckets }across the full batch of runs before constructing any presenters, viaActiveRecord::Associations::Preloader(the same primitive the GraphQL dataloader'sSources::ActiveRecordAssociationuses). This fixes all callers that go through.for_runs:RunAvailabilityPresenter.for_runs(CMSrun_availabilities/runs_with_openingsLiquid tags) and theSources::SignupCountGraphQL dataloader source (RunType#confirmed_limited_signup_count,#grouped_signup_counts).Added a regression test (verified it fails without the fix — 20 queries for 10 runs — and passes with it, ≤2 queries regardless of run count).
Out of scope
Sentry also showed a smaller, lower-volume version of the same query pair in
Mutation.createSignupRequestand sibling mutations (createMySignup,createUserSignup,updateSignupBucket,forceConfirmSignup,createSignupRequest) -- 4-11 occurrences per event vs. 33-100 for the page-load path above. I wasn't able to pin down a concrete root cause for that one (the registration_policy/buckets association should already be memoized within a single mutation call), so I'm leaving it for a follow-up rather than guessing at a fix.Test plan
test/presenters/signup_count_presenter_test.rb(new N+1 regression test + full existing suite)test/liquid_drops/convention_drop_test.rb,test/models/run_test.rb🤖 Generated with Claude Code