perf: optimize WorkshopInvitationController#accept (1940ms → <500ms target)#2701
Open
mroderick wants to merge 5 commits into
Open
perf: optimize WorkshopInvitationController#accept (1940ms → <500ms target)#2701mroderick wants to merge 5 commits into
mroderick wants to merge 5 commits into
Conversation
Added comprehensive test coverage for RSVP actions: Controller spec (spec/controllers/workshop_invitation_controller_spec.rb): - GET #show - POST #accept: happy path, already attending, RSVPs closed, workshop full, missing tutorial - POST #reject: happy path, already rejected, past deadline, waiting list promotion - PATCH #update: valid params, invalid params Feature spec additions (spec/features/accepting_invitation_spec.rb): - Workshop full scenario shows waiting list option Tests pass against master and all 4 performance commits from perf/async-rsvp-email.
WorkshopInvitationController#accept was sending confirmation emails synchronously with deliver_now, blocking the response for 2-10s. Changed to deliver_later in WorkshopPresenter and VirtualWorkshopPresenter. Email delivery now runs in background via Delayed Job. - Production: requires Delayed Job worker running - Tests: updated to verify deliver_later is called (allow/have_received pattern)
`attendee?` and `waitlisted?` loaded all attendees/waitlisted members into memory with includes(:member) just to check membership of one person. For a workshop with 30 students + 10 coaches, that was 80 AR objects loaded per check. Replaced with single EXISTS queries: - `attendee?`: `invitations.accepted.exists?(member_id:)` - `waitlisted?`: `WaitingList.by_workshop(self).joins(:invitation).where(...).exists?` Both now use a single SQL query regardless of attendance count.
WorkshopInvitationController accesses @invitation.workshop and @invitation.member multiple times per request. Added includes() to eager load both associations in the initial query, eliminating 2-3 lazy loads per request.
Both update and accept actions called assign_attributes + valid? + update, running validation twice. Replaced with single update calls. Also fixes: - update action was setting attending: true for validation but not persisting it (update was called with just invitation_params) - accept action showed "no available seats" when validation failed instead of the actual error message
65d85c4 to
1732242
Compare
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
WorkshopInvitationController#accepthas a reported mean response time of 1940ms, with outliers up to 16s. This is the RSVP acceptance flow used by workshop attendees, so slow response times directly impact user experience.Analysis
Traced the
acceptaction end-to-end and identified four bottlenecks:deliver_nowblocks response for 2-10s waiting on SMTPvalid?+updateruns validation twice1. Synchronous email (
deliver_now)Estimated improvement: -2000ms to -10000ms (removes SMTP latency from request)
2. O(N) attendee/waitlist checks
Same optimization for
waitlisted?. For a workshop with 30 students + 10 coaches, this eliminates loading 80 AR objects per check.Estimated improvement: -50ms to -200ms (depends on attendance count)
3. Eager loading
Estimated improvement: -20ms to -50ms
4. Redundant validation
Also fixes a bug where
updateaction setattending: truefor validation but didn't persist it.Estimated improvement: -5ms to -10ms
Estimated Total Improvement
Target: ≤500ms mean ✅
Changes
deliver_now→deliver_laterin bothWorkshopPresenterandVirtualWorkshopPresenterattendee?andwaitlisted?methods with single EXISTS queriesincludes(:workshop, :member)toset_invitationupdateandacceptactionsRequirements
bin/delayed_job startor Heroku worker dyno)Testing
All tests pass at each commit:
deliver_later