New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix N+1 queries on SchedulesController #2038

Merged
merged 2 commits into from May 9, 2018

Conversation

Projects
None yet
2 participants
@zvkemp
Copy link
Contributor

zvkemp commented Apr 12, 2018

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the upstream master branch.
  • The tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works(if appropriate).
  • I have added necessary documentation (if appropriate).

Short description of what this resolves/which issues does this fix?:

N+1 queries on SchedulesController

https://oss.skylight.io/app/applications/qsmDLnzbof0d/recent/6h/endpoints/SchedulesController%23show?responseType=html
https://oss.skylight.io/app/applications/qsmDLnzbof0d/recent/6h/endpoints/SchedulesController%23events?responseType=html

  • Both endpoints on SchedulesController currently load the entry points into the data, then rely on ActiveRecord relationships to fetch the rest as needed.

Changes proposed in this pull request:

  • Do not load all conferences on every request (move this before_action to the Admin::BaseController instead)
  • Split SchedulesController#show into format-based responses, and only load the data necessary for each format
  • Optimize the replacement? detection by pre-loading the list of withdrawn or canceled events (avoids 2 queries per event per css breakpoint, e.g. ~ 240 queries for a 30-event conference)
  • provide a means to eager load associations from Program#selected_event_schedules
  • improve readability of the schedules/_carousel partial

paired with @lbaillie

@bear454

bear454 approved these changes May 9, 2018

Copy link
Member

bear454 left a comment

Thanks for contributing!

@@ -4,8 +4,9 @@ class ConferencesController < ApplicationController
load_and_authorize_resource find_by: :short_title, except: :show

def index
@current = Conference.where('end_date >= ?', Date.current).reorder(start_date: :asc)
@antiquated = @conferences - @current
@current, @antiquated = Conference.reorder(start_date: :asc).all.partition do |conference|

This comment has been minimized.

@bear454

@bear454 bear454 merged commit 3f2f352 into openSUSE:master May 9, 2018

3 checks passed

Hakiri No security warnings were found.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.4%) to 82.928%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment