From 7cf2e40f26a0b76c8c1a406f9b9e597438a2ee4d Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Thu, 17 Dec 2020 15:16:49 +0530 Subject: [PATCH 1/3] Optimize loading block_region_ids when syncing facilities There are ~4k facilities in production, fetching the block_region itself takes 2 queries + 1 query where we fetch the facility for the page. This means a grand total of potentially 8000 region-related queries get fire just for inserting a block_region_id. This preloads the block_regions for all facilities that need to be shipped and that brings us back to 4 queries for a pagesize of 1000. --- app/controllers/api/v3/facilities_controller.rb | 6 +++--- app/models/facility.rb | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v3/facilities_controller.rb b/app/controllers/api/v3/facilities_controller.rb index feb13fdb5c..1225d1c4c0 100644 --- a/app/controllers/api/v3/facilities_controller.rb +++ b/app/controllers/api/v3/facilities_controller.rb @@ -44,8 +44,8 @@ def force_resync? end def records_to_sync - Facility - .updated_on_server_since(other_facilities_processed_since, limit) + other_facility_records + .with_block_region_id .includes(:facility_group) .where.not(facility_group: nil) end @@ -54,7 +54,7 @@ def records_to_sync def sync_region_id(facility) if current_user&.block_level_sync? - facility.region.block_region.id + facility.block_region_id else facility.facility_group_id end diff --git a/app/models/facility.rb b/app/models/facility.rb index b9f4758706..fd786f8875 100644 --- a/app/models/facility.rb +++ b/app/models/facility.rb @@ -49,6 +49,13 @@ class Facility < ApplicationRecord foreign_key: "assigned_facility_id" pg_search_scope :search_by_name, against: {name: "A", slug: "B"}, using: {tsearch: {prefix: true, any_word: true}} + scope :with_block_region_id, -> { + return all unless Flipper.enabled?(:regions_prep) + + joins("INNER JOIN regions facility_regions ON facility_regions.source_id = facilities.id") + .joins("INNER JOIN regions block_region ON block_region.path @> facility_regions.path AND block_region.region_type = 'block'") + .select("block_region.id AS block_region_id, facilities.*") + } enum facility_size: { community: "community", From 0b18ad0130b0d247f59d780cdc686d3a83ea2aef Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Thu, 17 Dec 2020 15:54:34 +0530 Subject: [PATCH 2/3] Memoize the block_sync_enabled? call for FacilitiesController --- app/controllers/api/v3/facilities_controller.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v3/facilities_controller.rb b/app/controllers/api/v3/facilities_controller.rb index 1225d1c4c0..64aa9230da 100644 --- a/app/controllers/api/v3/facilities_controller.rb +++ b/app/controllers/api/v3/facilities_controller.rb @@ -52,8 +52,14 @@ def records_to_sync private + # Memoize this call here so that we don't end up making thousands of calls to check user for each facility + def block_level_sync? + @is_block_level_sync_enabled = current_user&.block_level_sync? if @is_block_level_sync_enabled.nil? + @is_block_level_sync_enabled + end + def sync_region_id(facility) - if current_user&.block_level_sync? + if block_level_sync? facility.block_region_id else facility.facility_group_id From 2f3af87a36d1e06617c6a27847ef7f5ba68bd44d Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Fri, 18 Dec 2020 11:40:48 +0530 Subject: [PATCH 3/3] Address feedback --- .../api/v3/facilities_controller.rb | 20 ++++++++++++------- app/models/facility.rb | 2 -- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/v3/facilities_controller.rb b/app/controllers/api/v3/facilities_controller.rb index 64aa9230da..a398036e0f 100644 --- a/app/controllers/api/v3/facilities_controller.rb +++ b/app/controllers/api/v3/facilities_controller.rb @@ -44,18 +44,24 @@ def force_resync? end def records_to_sync - other_facility_records - .with_block_region_id - .includes(:facility_group) - .where.not(facility_group: nil) + if Flipper.enabled?(:regions_prep) + other_facility_records + .with_block_region_id + .includes(:facility_group) + .where.not(facility_group: nil) + else + other_facility_records + .includes(:facility_group) + .where.not(facility_group: nil) + end end private - # Memoize this call here so that we don't end up making thousands of calls to check user for each facility + # Memoize this call so that we don't end up making thousands of calls to check user for each facility def block_level_sync? - @is_block_level_sync_enabled = current_user&.block_level_sync? if @is_block_level_sync_enabled.nil? - @is_block_level_sync_enabled + return @block_level_sync_enabled if defined? @block_level_sync_enabled + @block_level_sync_enabled = current_user&.block_level_sync? end def sync_region_id(facility) diff --git a/app/models/facility.rb b/app/models/facility.rb index fd786f8875..fd6faed54c 100644 --- a/app/models/facility.rb +++ b/app/models/facility.rb @@ -50,8 +50,6 @@ class Facility < ApplicationRecord pg_search_scope :search_by_name, against: {name: "A", slug: "B"}, using: {tsearch: {prefix: true, any_word: true}} scope :with_block_region_id, -> { - return all unless Flipper.enabled?(:regions_prep) - joins("INNER JOIN regions facility_regions ON facility_regions.source_id = facilities.id") .joins("INNER JOIN regions block_region ON block_region.path @> facility_regions.path AND block_region.region_type = 'block'") .select("block_region.id AS block_region_id, facilities.*")