From 8bc5844245567bcf5ebfe952df3f7264ecdcdf70 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 3 Aug 2018 21:25:13 +1000 Subject: [PATCH] feat: optimise query for loading latest verification for the latest pacts for each tag on index page --- .../20180311_optimise_head_matrix.rb | 2 +- ...0720_create_latest_pact_publication_ids.rb | 2 + ...721_migrate_latest_pact_publication_ids.rb | 2 +- ...20180723_create_latest_verification_ids.rb | 7 +-- ...0180724_migrate_latest_verification_ids.rb | 10 ++-- db/migrations/20180726_recreate_views.rb | 2 +- ...ification_ids_for_consumer_version_tags.rb | 49 +++++++++++++++++++ lib/pact_broker/api/resources/verification.rb | 2 +- lib/pact_broker/pacts/repository.rb | 15 +++++- lib/pact_broker/repositories/helpers.rb | 21 ++++++++ lib/pact_broker/verifications/repository.rb | 15 +++++- 11 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 db/migrations/20180727_recreate_latest_verification_ids_for_consumer_version_tags.rb diff --git a/db/migrations/20180311_optimise_head_matrix.rb b/db/migrations/20180311_optimise_head_matrix.rb index fc3db6d59..41a02f4ee 100644 --- a/db/migrations/20180311_optimise_head_matrix.rb +++ b/db/migrations/20180311_optimise_head_matrix.rb @@ -18,7 +18,7 @@ # Add provider_version_order to original definition # The most recent verification for each pact_version # provider_version column is DEPRECATED, use provider_version_number - # Think this can be replaced by latest_verification_id_for_pact_version_and_provider_version? + # Think this can be replaced by latest_verif_id_for_pact_version_and_provider_version? v = :verifications create_or_replace_view(:latest_verifications, from(v) diff --git a/db/migrations/20180720_create_latest_pact_publication_ids.rb b/db/migrations/20180720_create_latest_pact_publication_ids.rb index c9e26c123..71c02a82d 100644 --- a/db/migrations/20180720_create_latest_pact_publication_ids.rb +++ b/db/migrations/20180720_create_latest_pact_publication_ids.rb @@ -8,10 +8,12 @@ # for a pact are deleted together when you delete the pact resource for that # consumer version, and when that happens, this row will cascade delete. create_table(:latest_pact_publication_ids_by_consumer_versions, charset: 'utf8') do + foreign_key :consumer_id, :pacticipants, nil: false, on_delete: :cascade # redundant, but speeds up queries by removing need for extra join foreign_key :consumer_version_id, :versions, nil: false, on_delete: :cascade foreign_key :provider_id, :pacticipants, nil: false, on_delete: :cascade foreign_key :pact_publication_id, :pact_publications, nil: false, on_delete: :cascade, unique: true index [:provider_id, :consumer_version_id], unique: true, name: "unq_latest_ppid_prov_conver" + index [:provider_id, :consumer_id], name: "lpp_provider_id_consumer_id_index" end end diff --git a/db/migrations/20180721_migrate_latest_pact_publication_ids.rb b/db/migrations/20180721_migrate_latest_pact_publication_ids.rb index c3743febd..c56d9f5ff 100644 --- a/db/migrations/20180721_migrate_latest_pact_publication_ids.rb +++ b/db/migrations/20180721_migrate_latest_pact_publication_ids.rb @@ -1,7 +1,7 @@ Sequel.migration do up do # The danger with this migration is that a pact publication created by an old node will be lost - rows = from(:latest_pact_publications_by_consumer_versions).select(:consumer_version_id, :provider_id, :id) + rows = from(:latest_pact_publications_by_consumer_versions).select(:consumer_id, :consumer_version_id, :provider_id, :id) from(:latest_pact_publication_ids_by_consumer_versions).insert(rows) end diff --git a/db/migrations/20180723_create_latest_verification_ids.rb b/db/migrations/20180723_create_latest_verification_ids.rb index 969e3c83d..1934ef69f 100644 --- a/db/migrations/20180723_create_latest_verification_ids.rb +++ b/db/migrations/20180723_create_latest_verification_ids.rb @@ -5,16 +5,17 @@ # latest revision speeds queries up. # There is no way to delete an individual verification result yet, but when there # is, we'll need to re-calculate the latest. - create_table(:latest_verification_id_for_pact_version_and_provider_version, charset: 'utf8') do + create_table(:latest_verif_id_for_pact_version_and_provider_version, charset: 'utf8') do + foreign_key :consumer_id, :pacticipants, nil: false, on_delete: :cascade # not required, but useful to avoid extra joins foreign_key :pact_version_id, :pact_versions, nil: false, on_delete: :cascade - foreign_key :provider_version_id, :versions, nil: false, on_delete: :cascade foreign_key :provider_id, :pacticipants, nil: false, on_delete: :cascade # not required, but useful to avoid extra joins + foreign_key :provider_version_id, :versions, nil: false, on_delete: :cascade foreign_key :verification_id, :verifications, nil: false, on_delete: :cascade, unique: true index [:pact_version_id, :provider_version_id], unique: true, name: "unq_latest_verifid_pvid_provid" end end down do - drop_table(:latest_verification_id_for_pact_version_and_provider_version) + drop_table(:latest_verif_id_for_pact_version_and_provider_version) end end diff --git a/db/migrations/20180724_migrate_latest_verification_ids.rb b/db/migrations/20180724_migrate_latest_verification_ids.rb index 86bef55e3..0b989043e 100644 --- a/db/migrations/20180724_migrate_latest_verification_ids.rb +++ b/db/migrations/20180724_migrate_latest_verification_ids.rb @@ -1,15 +1,15 @@ Sequel.migration do up do - # Not sure if we need the provider_id, but it might come in handy + # consumer_id and provider_id are redundant by avoid making extra joins when creating views rows = from(:verifications).select_group( + Sequel[:verifications][:consumer_id], Sequel[:verifications][:pact_version_id], - Sequel[:verifications][:provider_version_id], - Sequel[:versions][:pacticipant_id].as(:provider_id)) + Sequel[:verifications][:provider_id], + Sequel[:verifications][:provider_version_id]) .select_append{ max(verifications[id]).as(verification_id) } - .join(:versions, { Sequel[:verifications][:provider_version_id] => Sequel[:versions][:id] }) # The danger with this migration is that a verification created by an old node will be lost - from(:latest_verification_id_for_pact_version_and_provider_version).insert(rows) + from(:latest_verif_id_for_pact_version_and_provider_version).insert(rows) end down do diff --git a/db/migrations/20180726_recreate_views.rb b/db/migrations/20180726_recreate_views.rb index 32266bf56..8e02ff6ea 100644 --- a/db/migrations/20180726_recreate_views.rb +++ b/db/migrations/20180726_recreate_views.rb @@ -8,7 +8,7 @@ from latest_pact_publication_ids_by_consumer_versions lpp inner join pact_publications pp on pp.id = lpp.pact_publication_id - left outer join latest_verification_id_for_pact_version_and_provider_version lv + left outer join latest_verif_id_for_pact_version_and_provider_version lv on lv.pact_version_id = pp.pact_version_id" ) diff --git a/db/migrations/20180727_recreate_latest_verification_ids_for_consumer_version_tags.rb b/db/migrations/20180727_recreate_latest_verification_ids_for_consumer_version_tags.rb new file mode 100644 index 000000000..444ea9f4f --- /dev/null +++ b/db/migrations/20180727_recreate_latest_verification_ids_for_consumer_version_tags.rb @@ -0,0 +1,49 @@ +Sequel.migration do + up do + ltcvo = :latest_tagged_pact_consumer_version_orders + versions_join = { + Sequel[ltcvo][:consumer_id] => Sequel[:cv][:pacticipant_id], + Sequel[ltcvo][:latest_consumer_version_order] => Sequel[:cv][:order] + } + lpp_join = { + Sequel[:lpp][:consumer_version_id] => Sequel[:cv][:id], + Sequel[ltcvo][:provider_id] => Sequel[:lpp][:provider_id] + } + # todo add pact_version_id to latest_pact_publication_ids_by_consumer_versions? + pp_join = { + Sequel[:pp][:id] => Sequel[:lpp][:pact_publication_id] + } + verifications_join = { + Sequel[:v][:pact_version_id] => Sequel[:pp][:pact_version_id] + } + view = from(ltcvo).select_group( + Sequel[ltcvo][:provider_id], + Sequel[ltcvo][:consumer_id], + Sequel[ltcvo][:tag_name].as(:consumer_version_tag_name)) + .select_append{ max(v[id]).as(latest_verification_id) } + .join(:versions, versions_join, { table_alias: :cv } ) + .join(:latest_pact_publication_ids_by_consumer_versions, lpp_join, { table_alias: :lpp }) + .join(:pact_publications, pp_join, { table_alias: :pp }) + .join(:verifications, verifications_join, { table_alias: :v }) + + create_or_replace_view(:latest_verification_ids_for_consumer_version_tags, view) + end + + down do + # The latest verification id for each consumer version tag + create_or_replace_view(:latest_verification_ids_for_consumer_version_tags, + "select + pv.pacticipant_id as provider_id, + lpp.consumer_id, + t.name as consumer_version_tag_name, + max(v.id) as latest_verification_id + from verifications v + join latest_pact_publications_by_consumer_versions lpp + on v.pact_version_id = lpp.pact_version_id + join tags t + on lpp.consumer_version_id = t.version_id + join versions pv + on v.provider_version_id = pv.id + group by pv.pacticipant_id, lpp.consumer_id, t.name") + end +end diff --git a/lib/pact_broker/api/resources/verification.rb b/lib/pact_broker/api/resources/verification.rb index f2150aec6..b89ac595f 100644 --- a/lib/pact_broker/api/resources/verification.rb +++ b/lib/pact_broker/api/resources/verification.rb @@ -14,7 +14,7 @@ def content_types_provided [["application/hal+json", :to_json], ["application/json", :to_json]] end - # Remember to update latest_verification_id_for_pact_version_and_provider_version + # Remember to update latest_verif_id_for_pact_version_and_provider_version # if/when DELETE is implemented def allowed_methods ["GET", "OPTIONS"] diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb index 240ac9a6a..960e0d853 100644 --- a/lib/pact_broker/pacts/repository.rb +++ b/lib/pact_broker/pacts/repository.rb @@ -49,8 +49,19 @@ def update id, params end def update_latest_pact_publication_ids(pact_publication) - latest_pact_publication_params = { consumer_version_id: pact_publication.consumer_version_id, provider_id: pact_publication.provider_id, pact_publication_id: pact_publication.id } - AllPactPublications.db[:latest_pact_publication_ids_by_consumer_versions].insert_ignore.insert(latest_pact_publication_params) + key = { + consumer_version_id: pact_publication.consumer_version_id, + provider_id: pact_publication.provider_id, + } + + other = { + pact_publication_id: pact_publication.id, consumer_id: pact_publication.consumer_id + } + + row = key.merge(other) + + table = AllPactPublications.db[:latest_pact_publication_ids_by_consumer_versions] + PactBroker::Repositories::Helpers.upsert(table, key, other) end def delete params diff --git a/lib/pact_broker/repositories/helpers.rb b/lib/pact_broker/repositories/helpers.rb index 65b752d0a..027498d81 100644 --- a/lib/pact_broker/repositories/helpers.rb +++ b/lib/pact_broker/repositories/helpers.rb @@ -20,6 +20,10 @@ def mysql? Sequel::Model.db.adapter_scheme.to_s =~ /mysql/ end + def postgres? + Sequel::Model.db.adapter_scheme.to_s == "postgres" + end + def select_all_qualified select(Sequel[model.table_name].*) end @@ -31,6 +35,23 @@ def select_for_subquery column select(column) end end + + # TODO refactor to use proper dataset module + def upsert table, key, other + row = key.merge(other) + if postgres? + table.insert_conflict(update: other, target: key.keys).insert(row) + elsif mysql? + table.on_duplicate_key_update.insert(row) + else + # Sqlite + if table.where(key).count == 0 + table.insert(row) + else + table.where(key).update(row) + end + end + end end end end diff --git a/lib/pact_broker/verifications/repository.rb b/lib/pact_broker/verifications/repository.rb index 1c3cabf0b..f708f389b 100644 --- a/lib/pact_broker/verifications/repository.rb +++ b/lib/pact_broker/verifications/repository.rb @@ -33,8 +33,18 @@ def create verification, provider_version_number, pact end def update_latest_verification_id verification - latest_verification_params = { pact_version_id: verification.pact_version_id, provider_version_id: verification.provider_version_id, provider_id: verification.provider_version.pacticipant_id, verification_id: verification.id } - PactBroker::Domain::Verification.db[:latest_verification_id_for_pact_version_and_provider_version].insert_ignore.insert(latest_verification_params) + key = { + pact_version_id: verification.pact_version_id, provider_version_id: verification.provider_version_id + } + + other = { + provider_id: verification.provider_version.pacticipant_id, + verification_id: verification.id, + consumer_id: verification.consumer_id + } + + table = PactBroker::Domain::Verification.db[:latest_verif_id_for_pact_version_and_provider_version] + PactBroker::Repositories::Helpers.upsert(table, key, other) end def find consumer_name, provider_name, pact_version_sha, verification_number @@ -100,6 +110,7 @@ def find_latest_verification_for_tags consumer_name, provider_name, consumer_ver .tag(consumer_version_tag) .provider_version_tag(provider_version_tag) + query.reverse_order( Sequel[:latest_pact_publications_by_consumer_versions][:consumer_version_order], Sequel[:latest_pact_publications_by_consumer_versions][:revision_number],