Skip to content
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

[#148409] Reduce number of DB queries on Facility timeline page #1899

Merged
merged 9 commits into from
Mar 14, 2019

Conversation

vandrijevik
Copy link
Contributor

Release Notes

Reduce number of DB queries on Facility timeline page

Additional Context

Removes a few low-hanging N+1 fruits in the partials that we render for each instrument on the Facility’s timeline page, and reduces the typical number of queries per instrument from 10:

Before

Instrument Load (0.2ms)  SELECT `products`.* FROM `products` WHERE `products`.`type` IN ('Instrument') AND `products`.`schedule_id` = 2 AND `products`.`is_archived` = 0
 (0.3ms)  SELECT COUNT(*) FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`product_id` = 5 AND `reservations`.`reserve_end_at` IS NULL AND (reserve_start_at < '2019-03-14 21:04:48.751912')
InstrumentAlert Load (0.2ms)  SELECT  `instrument_alerts`.* FROM `instrument_alerts` WHERE `instrument_alerts`.`instrument_id` = 5 LIMIT 1
Reservation Load (0.9ms)  SELECT `reservations`.* FROM `reservations` INNER JOIN `order_details` ON `order_details`.`id` = `reservations`.`order_detail_id` INNER JOIN `orders` ON `orders`.`id` = `order_details`.`order_id` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`product_id` = 5 AND `orders`.`state` = 'purchased' AND (reserve_end_at >= '2019-03-14 05:00:00') AND (reserve_start_at < '2019-03-15 04:59:59.999999') ORDER BY `reservations`.`reserve_start_at` ASC
AdminReservation Load (0.2ms)  SELECT `reservations`.* FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('AdminReservation') AND `reservations`.`product_id` = 5 AND (reserve_end_at >= '2019-03-14 05:00:00') AND (reserve_start_at < '2019-03-15 04:59:59.999999')
OfflineReservation Load (0.2ms)  SELECT `reservations`.* FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`product_id` = 5 AND (reserve_end_at >= '2019-03-14 05:00:00') AND (reserve_start_at < '2019-03-15 04:59:59.999999')
ScheduleRule Load (0.1ms)  SELECT `schedule_rules`.* FROM `schedule_rules` WHERE `schedule_rules`.`product_id` = 5
Relay Load (0.3ms)  SELECT  `relays`.* FROM `relays` WHERE `relays`.`instrument_id` = 5 LIMIT 1
 (0.2ms)  SELECT COUNT(*) FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`product_id` = 5 AND `reservations`.`reserve_end_at` IS NULL AND (reserve_start_at < '2019-03-14 21:04:48.767382')
 (0.4ms)  SELECT COUNT(*) FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`product_id` = 5 AND `reservations`.`reserve_end_at` IS NULL AND (reserve_start_at < '2019-03-14 21:04:48.768619')

to 5:

After

  OfflineReservation Load (0.4ms)  SELECT `reservations`.* FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`product_id` = 8 AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`reserve_end_at` IS NULL AND (reserve_start_at < '2019-03-14 21:01:56.731311')
  Reservation Load (0.5ms)  SELECT `reservations`.* FROM `reservations` INNER JOIN `order_details` ON `order_details`.`id` = `reservations`.`order_detail_id` INNER JOIN `orders` ON `orders`.`id` = `order_details`.`order_id` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`product_id` = 8 AND `orders`.`state` = 'purchased' AND (reserve_end_at >= '2019-03-14 05:00:00') AND (reserve_start_at < '2019-03-15 04:59:59.999999') ORDER BY `reservations`.`reserve_start_at` ASC
  AdminReservation Load (0.3ms)  SELECT `reservations`.* FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('AdminReservation') AND `reservations`.`product_id` = 8 AND (reserve_end_at >= '2019-03-14 05:00:00') AND (reserve_start_at < '2019-03-15 04:59:59.999999')
  OfflineReservation Load (0.2ms)  SELECT `reservations`.* FROM `reservations` WHERE `reservations`.`deleted_at` IS NULL AND `reservations`.`type` IN ('OfflineReservation') AND `reservations`.`product_id` = 8 AND (reserve_end_at >= '2019-03-14 05:00:00') AND (reserve_start_at < '2019-03-15 04:59:59.999999')
  ScheduleRule Load (0.1ms)  SELECT `schedule_rules`.* FROM `schedule_rules` WHERE `schedule_rules`.`product_id` = 8

Unfortunately, the remaining N+1 queries that we have (for an instrument’s reservations) are not trivial to remove because they depend on the selected date, so they can’t be preloaded. If this PR is not enough of a performance boost for the facility timeline page, we can at least try to combine the queries which load reservations for an instrument into one (instead of 5 per instrument).

The indirection and hack (in ReservationsController, public_timeline calls timeline) here are not worth the DRY aspect, and the two pages are very different, so decoupling their implementation allows us to evolve and optimize the controller methods separately as it makes sense.
The method #offline? needs to check whether there are any current offline reservations. By refactoring to an association and using #present?, Rails will memoize the association and check without another DB query.
@vandrijevik vandrijevik force-pushed the va-improve-performance-on-facility-timeline-page branch from 73878b1 to 633bb82 Compare March 14, 2019 21:14
@vandrijevik vandrijevik merged commit 32c2509 into master Mar 14, 2019
@vandrijevik vandrijevik deleted the va-improve-performance-on-facility-timeline-page branch March 14, 2019 22:02
joshea0 pushed a commit to SquaredLabs/nucore-uconn that referenced this pull request Mar 18, 2020
…orks#1899)

* Refactor away Timelinable module

The indirection and hack (in ReservationsController, public_timeline calls timeline) here are not worth the DRY aspect, and the two pages are very different, so decoupling their implementation allows us to evolve and optimize the controller methods separately as it makes sense.

* Separate facility and public schedule partials and get rid of conditional

* Refactor association-like methods on Schedule to associations

* Use .size on association in _schedule so we don’t have to execute a count every time

* Eliminate N+1 for loading alerts when displaying timelines

* Fold single-use partial _relay_switch into only call site

* Avoid N+1 for relays on facility timeline

* Avoid two unnecessary queries just to check if an instrument is offline

The method #offline? needs to check whether there are any current offline reservations. By refactoring to an association and using #present?, Rails will memoize the association and check without another DB query.

* Appease rubocop’s style preferences
chrixp pushed a commit to SquaredLabs/nucore-uconn that referenced this pull request Oct 21, 2020
…orks#1899)

* Refactor away Timelinable module

The indirection and hack (in ReservationsController, public_timeline calls timeline) here are not worth the DRY aspect, and the two pages are very different, so decoupling their implementation allows us to evolve and optimize the controller methods separately as it makes sense.

* Separate facility and public schedule partials and get rid of conditional

* Refactor association-like methods on Schedule to associations

* Use .size on association in _schedule so we don’t have to execute a count every time

* Eliminate N+1 for loading alerts when displaying timelines

* Fold single-use partial _relay_switch into only call site

* Avoid N+1 for relays on facility timeline

* Avoid two unnecessary queries just to check if an instrument is offline

The method #offline? needs to check whether there are any current offline reservations. By refactoring to an association and using #present?, Rails will memoize the association and check without another DB query.

* Appease rubocop’s style preferences
joshea0 pushed a commit to SquaredLabs/nucore-uconn that referenced this pull request Oct 23, 2020
…orks#1899)

* Refactor away Timelinable module

The indirection and hack (in ReservationsController, public_timeline calls timeline) here are not worth the DRY aspect, and the two pages are very different, so decoupling their implementation allows us to evolve and optimize the controller methods separately as it makes sense.

* Separate facility and public schedule partials and get rid of conditional

* Refactor association-like methods on Schedule to associations

* Use .size on association in _schedule so we don’t have to execute a count every time

* Eliminate N+1 for loading alerts when displaying timelines

* Fold single-use partial _relay_switch into only call site

* Avoid N+1 for relays on facility timeline

* Avoid two unnecessary queries just to check if an instrument is offline

The method #offline? needs to check whether there are any current offline reservations. By refactoring to an association and using #present?, Rails will memoize the association and check without another DB query.

* Appease rubocop’s style preferences
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants