From 123f86298c517d0b69a17e88606f67e38bb475e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Bu=C5=82at?= Date: Mon, 21 Feb 2022 21:38:25 +0100 Subject: [PATCH] fix: Improve Matrix request performance (#537) Fix some N+1 query issues by using sequel eager loading and similar preloading techniques. --- lib/pact_broker/domain/version.rb | 7 +++++++ lib/pact_broker/matrix/quick_row.rb | 20 +++++++++++--------- lib/pact_broker/matrix/repository.rb | 11 ++++++----- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index d0cd51801..25c3ea338 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -241,6 +241,13 @@ def for_selector(selector) end # rubocop: enable Metrics/CyclomaticComplexity + def pacticipants_set + from_self(alias: :v) + .select_group(Sequel[:v][:pacticipant_id]) + .collect(&:pacticipant_id) + .to_set + end + # private def calculate_max_version_order_and_join_back_to_versions(query, selector) diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb index 027808bb6..04f5b5f74 100644 --- a/lib/pact_broker/matrix/quick_row.rb +++ b/lib/pact_broker/matrix/quick_row.rb @@ -124,15 +124,17 @@ def order_by_pact_publication_created_at end def eager_all_the_things - eager(:consumer) - .eager(:provider) - .eager(consumer_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, { branch_versions: :branch_head }]) - .eager(provider_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, { branch_versions: :branch_head }]) - .eager(:verification) - .eager(:pact_publication) - .eager(:pact_version) - .eager(:consumer_version_tags) - .eager(:provider_version_tags) + eager( + :consumer, + :provider, + :verification, + :pact_publication, + :pact_version, + consumer_version: { current_deployed_versions: :environment, current_supported_released_versions: :environment, branch_versions: [:branch_head, :version, branch: :pacticipant] }, + provider_version: { current_deployed_versions: :environment, current_supported_released_versions: :environment, branch_versions: [:branch_head, :version, branch: :pacticipant] }, + consumer_version_tags: { version: :pacticipant }, + provider_version_tags: { version: :pacticipant } + ) end def default_scope diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index b35dcd81a..8348ed3b7 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/lib/pact_broker/matrix/repository.rb @@ -127,20 +127,19 @@ def integrations_where_specified_selector_is_consumer(resolved_specified_selecto def integrations_where_specified_selector_is_provider(resolved_specified_selectors, options) integrations_involving_specified_providers = PactBroker::Integrations::Integration .where(provider_id: resolved_specified_selectors.collect(&:pacticipant_id)) - .eager(:consumer) + .eager(:consumer, :provider) .all destination_selector = PactBroker::Matrix::UnresolvedSelector.new(options.slice(:latest, :tag, :branch, :environment_name).compact) + required = PactBroker::Domain::Version.for_selector(destination_selector).pacticipants_set integrations_involving_specified_providers.collect do | integration | - required = PactBroker::Domain::Version.where(pacticipant: integration.consumer).for_selector(destination_selector).any? - Integration.from_hash( consumer_id: integration.consumer.id, consumer_name: integration.consumer.name, provider_id: integration.provider.id, provider_name: integration.provider.name, - required: required + required: required.member?(integration.consumer.id) ) end end @@ -221,8 +220,10 @@ def base_model_for_integrations # Find the version number for selectors with the latest and/or tag specified def resolve_versions_and_add_ids(unresolved_selectors, selector_type, resolved_ignore_selectors = []) + names = unresolved_selectors.collect(&:pacticipant_name) + pacticipants = PactBroker::Domain::Pacticipant.where(name: names).to_a.group_by(&:name).transform_values(&:first) unresolved_selectors.collect do | unresolved_selector | - pacticipant = PactBroker::Domain::Pacticipant.find(name: unresolved_selector.pacticipant_name) + pacticipant = pacticipants[unresolved_selector.pacticipant_name] if pacticipant versions = find_versions_for_selector(unresolved_selector) build_resolved_selectors(pacticipant, versions, unresolved_selector, selector_type, resolved_ignore_selectors)