From 494f5533371d730fa1106dbb86a1d5c802588ee6 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 30 Jan 2018 12:01:09 +1100 Subject: [PATCH] feat(dashboard api): optimise dashboard query by creating manual materialized views for the matrix Dashboard query was taking 20 seconds. --- .../20180130_create_materialized_matrix.rb | 35 ++++++++++++ ...80131_create_materialized_latest_matrix.rb | 35 ++++++++++++ .../api/resources/base_resource.rb | 3 + lib/pact_broker/matrix/actual_latest_row.rb | 3 +- lib/pact_broker/matrix/repository.rb | 5 ++ lib/pact_broker/matrix/row.rb | 31 ++++++++++- lib/pact_broker/matrix/service.rb | 6 ++ .../pact_broker/api/resources/group_spec.rb | 1 + spec/lib/pact_broker/matrix/row_spec.rb | 55 +++++++++++++++++++ spec/migrations/23_pact_versions_spec.rb | 4 +- .../change_migration_strategy_spec.rb | 2 +- spec/support/test_data_builder.rb | 16 ++++++ 12 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 db/migrations/20180130_create_materialized_matrix.rb create mode 100644 db/migrations/20180131_create_materialized_latest_matrix.rb diff --git a/db/migrations/20180130_create_materialized_matrix.rb b/db/migrations/20180130_create_materialized_matrix.rb new file mode 100644 index 000000000..7788c5e63 --- /dev/null +++ b/db/migrations/20180130_create_materialized_matrix.rb @@ -0,0 +1,35 @@ +Sequel.migration do + up do + create_table(:materialized_matrix, charset: 'utf8') do + Integer :consumer_id, null: false + String :consumer_name, null: false + Integer :consumer_version_id, null: false + String :consumer_version_number, null: false + Integer :consumer_version_order, null: false + Integer :pact_publication_id, null: false + Integer :pact_version_id, null: false + String :pact_version_sha, null: false + Integer :pact_revision_number, null: false + DateTime :pact_created_at, null: false + Integer :provider_id, null: false + String :provider_name, null: false + Integer :provider_version_id + String :provider_version_number + Integer :provider_version_order + Integer :verification_id + Boolean :success + Integer :verification_number + DateTime :verification_executed_at + String :verification_build_url + index [:consumer_id], name: 'ndx_mm_consumer_id' + index [:provider_id], name: 'ndx_mm_provider_id' + index [:consumer_version_order], name: 'ndx_mm_cv_ord' + end + + from(:materialized_matrix).insert(from(:matrix).select_all) + end + + down do + drop_table(:materialized_matrix) + end +end diff --git a/db/migrations/20180131_create_materialized_latest_matrix.rb b/db/migrations/20180131_create_materialized_latest_matrix.rb new file mode 100644 index 000000000..4dff74bae --- /dev/null +++ b/db/migrations/20180131_create_materialized_latest_matrix.rb @@ -0,0 +1,35 @@ +Sequel.migration do + up do + create_table(:materialized_latest_matrix, charset: 'utf8') do + Integer :consumer_id, null: false + String :consumer_name, null: false + Integer :consumer_version_id, null: false + String :consumer_version_number, null: false + Integer :consumer_version_order, null: false + Integer :pact_publication_id, null: false + Integer :pact_version_id, null: false + String :pact_version_sha, null: false + Integer :pact_revision_number, null: false + DateTime :pact_created_at, null: false + Integer :provider_id, null: false + String :provider_name, null: false + Integer :provider_version_id + String :provider_version_number + Integer :provider_version_order + Integer :verification_id + Boolean :success + Integer :verification_number + DateTime :verification_executed_at + String :verification_build_url + index [:consumer_id], name: 'ndx_mlm_consumer_id' + index [:provider_id], name: 'ndx_mlm_provider_id' + index [:consumer_version_order], name: 'ndx_mlm_cv_ord' + end + + from(:materialized_latest_matrix).insert(from(:latest_matrix).select_all) + end + + down do + drop_table(:materialized_latest_matrix) + end +end diff --git a/lib/pact_broker/api/resources/base_resource.rb b/lib/pact_broker/api/resources/base_resource.rb index f4a55c931..cc8532921 100644 --- a/lib/pact_broker/api/resources/base_resource.rb +++ b/lib/pact_broker/api/resources/base_resource.rb @@ -41,6 +41,9 @@ def initialize end def finish_request + if !request.get? && !request.head? + matrix_service.refresh(identifier_from_path) + end PactBroker.configuration.after_resource.call(self) end diff --git a/lib/pact_broker/matrix/actual_latest_row.rb b/lib/pact_broker/matrix/actual_latest_row.rb index 0eadd6c97..51890f537 100644 --- a/lib/pact_broker/matrix/actual_latest_row.rb +++ b/lib/pact_broker/matrix/actual_latest_row.rb @@ -2,9 +2,8 @@ module PactBroker module Matrix - # Latest pact revision for each consumer/provider => latest verification class ActualLatestRow < Row - set_dataset(:latest_matrix) + set_dataset(:materialized_latest_matrix) # For some reason, with MySQL, the success column value # comes back as an integer rather than a boolean diff --git a/lib/pact_broker/matrix/repository.rb b/lib/pact_broker/matrix/repository.rb index 8e9755fa3..a2637b9d0 100644 --- a/lib/pact_broker/matrix/repository.rb +++ b/lib/pact_broker/matrix/repository.rb @@ -18,6 +18,11 @@ class Repository GROUP_BY_PROVIDER = [:consumer_name, :consumer_version_number, :provider_name] GROUP_BY_PACT = [:consumer_name, :provider_name] + def refresh params + PactBroker::Matrix::Row.refresh(params) + PactBroker::Matrix::ActualLatestRow.refresh(params) + end + # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number def find selectors, options = {} # The group with the nil provider_version_numbers will be the results of the left outer join diff --git a/lib/pact_broker/matrix/row.rb b/lib/pact_broker/matrix/row.rb index 528fb14cf..51de12204 100644 --- a/lib/pact_broker/matrix/row.rb +++ b/lib/pact_broker/matrix/row.rb @@ -4,7 +4,7 @@ module PactBroker module Matrix - class Row < Sequel::Model(:matrix) + class Row < Sequel::Model(:materialized_matrix) associate(:one_to_many, :latest_triggered_webhooks, :class => "PactBroker::Webhooks::LatestTriggeredWebhook", primary_key: :pact_publication_id, key: :pact_publication_id) associate(:one_to_many, :webhooks, :class => "PactBroker::Webhooks::Webhook", primary_key: [:consumer_id, :provider_id], key: [:consumer_id, :provider_id]) @@ -14,6 +14,35 @@ class Row < Sequel::Model(:matrix) dataset_module do include PactBroker::Repositories::Helpers + def refresh params + source_view_name = model.table_name.to_s.gsub('materialized_', '').to_sym + if params[:consumer_name] || params[:provider_name] + criteria = {} + if params[:consumer_name] + pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:consumer_name])).single_record + criteria[:consumer_id] = pacticipant.id if pacticipant + end + + if params[:provider_name] + pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:provider_name])).single_record + criteria[:provider_id] = pacticipant.id if pacticipant + end + if criteria.any? + db[model.table_name].where(criteria).delete + db[model.table_name].insert(db[source_view_name].where(criteria)) + end + end + + if params[:pacticipant_name] + pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:pacticipant_name])).single_record + if pacticipant + db[model.table_name].where(consumer_id: pacticipant.id).or(provider_id: pacticipant.id).delete + new_rows = db[source_view_name].where(consumer_id: pacticipant.id).or(provider_id: pacticipant.id) + db[model.table_name].insert(new_rows) + end + end + end + def matching_selectors selectors if selectors.size == 1 where_consumer_or_provider_is(selectors.first) diff --git a/lib/pact_broker/matrix/service.rb b/lib/pact_broker/matrix/service.rb index 97aa3fd86..a5cb50fef 100644 --- a/lib/pact_broker/matrix/service.rb +++ b/lib/pact_broker/matrix/service.rb @@ -1,4 +1,6 @@ require 'pact_broker/repositories' +require 'pact_broker/matrix/row' +require 'pact_broker/matrix/actual_latest_row' module PactBroker module Matrix @@ -8,6 +10,10 @@ module Service extend PactBroker::Repositories extend PactBroker::Services + def refresh params + matrix_repository.refresh params + end + def find criteria, options = {} matrix_repository.find criteria, options end diff --git a/spec/lib/pact_broker/api/resources/group_spec.rb b/spec/lib/pact_broker/api/resources/group_spec.rb index 1a702394d..e1c4b2ec1 100644 --- a/spec/lib/pact_broker/api/resources/group_spec.rb +++ b/spec/lib/pact_broker/api/resources/group_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'pact_broker/api/resources/group' +require 'pact_broker/groups/service' require 'rack/test' module PactBroker::Api diff --git a/spec/lib/pact_broker/matrix/row_spec.rb b/spec/lib/pact_broker/matrix/row_spec.rb index 684ff71b7..3b258dd52 100644 --- a/spec/lib/pact_broker/matrix/row_spec.rb +++ b/spec/lib/pact_broker/matrix/row_spec.rb @@ -3,6 +3,61 @@ module PactBroker module Matrix describe Row do + describe "refresh" do + let(:td) { TestDataBuilder.new(auto_refresh_matrix: false) } + + before do + td.create_pact_with_hierarchy("Foo", "1", "Bar") + end + + context "with only a consumer_name" do + subject { Row.refresh(consumer_name: "Foo") } + + it "refreshes the data for the consumer" do + subject + expect(Row.all.collect(&:values)).to contain_hash(provider_name: "Bar", consumer_name: "Foo") + end + end + + context "with only a provider_name" do + subject { Row.refresh(provider_name: "Bar") } + + it "refreshes the data for the provider" do + subject + expect(Row.all.collect(&:values)).to contain_hash(provider_name: "Bar", consumer_name: "Foo") + end + end + + context "with both consumer_name and provider_name" do + subject { Row.refresh(provider_name: "Bar", consumer_name: "Foo") } + + it "refreshes the data for the consumer and provider" do + subject + expect(Row.all.collect(&:values)).to contain_hash(provider_name: "Bar", consumer_name: "Foo") + end + end + + context "when there was existing data" do + it "deletes the existing data before inserting the new data" do + Row.refresh(provider_name: "Bar", consumer_name: "Foo") + expect(Row.count).to eq 1 + td.create_consumer_version("2") + .create_pact + Row.refresh(provider_name: "Bar", consumer_name: "Foo") + expect(Row.count).to eq 2 + end + end + + context "with a pacticipant_name" do + subject { Row.refresh(pacticipant_name: "Bar") } + + it "refreshes the data for both consumer and provider roles" do + subject + expect(Row.all.collect(&:values)).to contain_hash(provider_name: "Bar", consumer_name: "Foo") + end + end + end + describe "<=>" do let(:row_1) do Row.new( diff --git a/spec/migrations/23_pact_versions_spec.rb b/spec/migrations/23_pact_versions_spec.rb index a1d3be34c..3ccd104e9 100644 --- a/spec/migrations/23_pact_versions_spec.rb +++ b/spec/migrations/23_pact_versions_spec.rb @@ -14,7 +14,7 @@ let!(:pact_2) { create(:pacts, {version_id: consumer_version_2[:id], provider_id: provider[:id], pact_version_content_sha: '1234', created_at: now, updated_at: pact_updated_at}) } - subject { PactBroker::Database.migrate(34) } + subject { PactBroker::Database.migrate } it "keeps the same number of pacts" do subject @@ -65,7 +65,7 @@ subject PactBroker::Pacts::Service.create_or_update_pact( - consumer_id: consumer[:id], provider_id: provider[:id], consumer_version_number: '1.2.3', json_content: load_fixture('a_consumer-a_provider.json') + consumer_name: consumer[:name], provider_name: provider[:name], consumer_version_number: '1.2.3', json_content: load_fixture('a_consumer-a_provider.json') ) end end diff --git a/spec/migrations/change_migration_strategy_spec.rb b/spec/migrations/change_migration_strategy_spec.rb index 71f800c37..cfcc27656 100644 --- a/spec/migrations/change_migration_strategy_spec.rb +++ b/spec/migrations/change_migration_strategy_spec.rb @@ -45,7 +45,7 @@ def execute command it "allows data to be inserted" do consumer_id = @db[:pacticipants].insert(name: 'Foo', created_at: DateTime.now, updated_at: DateTime.now) provider_id = @db[:pacticipants].insert(name: 'Bar', created_at: DateTime.now, updated_at: DateTime.now) - version_id = @db[:versions].insert(number: '1.2.3', pacticipant_id: consumer_id, created_at: DateTime.now, updated_at: DateTime.now) + version_id = @db[:versions].insert(number: '1.2.3', order: 1, pacticipant_id: consumer_id, created_at: DateTime.now, updated_at: DateTime.now) pact_json = {consumer: {name: 'Foo'}, provider: {name: 'Bar'}, interactions: []}.to_json pact_version_id = @db[:pact_versions].insert(sha: '123', content: pact_json, created_at: DateTime.now, consumer_id: consumer_id, provider_id: provider_id) pact_publication_id = @db[:pact_publications].insert(consumer_version_id: version_id, provider_id: provider_id, revision_number: 1, pact_version_id: pact_version_id, created_at: DateTime.now) diff --git a/spec/support/test_data_builder.rb b/spec/support/test_data_builder.rb index 035cd4118..adee888ed 100644 --- a/spec/support/test_data_builder.rb +++ b/spec/support/test_data_builder.rb @@ -37,6 +37,20 @@ class TestDataBuilder attr_reader :webhook attr_reader :webhook_execution attr_reader :triggered_webhook + attr_accessor :auto_refresh_matrix + + def initialize(params = {}) + @auto_refresh_matrix = params.fetch(:auto_refresh_matrix, true) + end + + def refresh_matrix + if auto_refresh_matrix + params = {} + params[:consumer_name] = consumer.name if consumer + params[:provider_name] = provider.name if provider + matrix_service.refresh(params) + end + end def create_pricing_service @pricing_service_id = pacticipant_repository.create(:name => 'Pricing Service', :repository_url => 'git@git.realestate.com.au:business-systems/pricing-service').save(raise_on_save_failure: true).id @@ -185,6 +199,7 @@ def create_pact params = {} set_created_at_if_set params[:created_at], :pact_publications, {id: @pact.id} set_created_at_if_set params[:created_at], :pact_versions, {sha: @pact.pact_version_sha} @pact = PactBroker::Pacts::PactPublication.find(id: @pact.id).to_domain + refresh_matrix self end @@ -253,6 +268,7 @@ def create_verification parameters = {} PactBroker::Domain::Tag.create(name: tag_name, version: provider_version) end end + refresh_matrix self end