From 96364408e27e7c0660799fb0a623512bdb039b85 Mon Sep 17 00:00:00 2001 From: Simon Tomlinson Date: Fri, 29 Apr 2022 11:37:00 -0500 Subject: [PATCH 1/2] Display excluded query aggregate timing --- notifier/background_migration.rb | 4 ++ notifier/migration.rb | 4 ++ notifier/spec/background_migration_spec.rb | 7 +++ notifier/spec/migration_spec.rb | 46 +++++++++++-------- .../templates/background_migration_detail.erb | 2 +- notifier/templates/detail.erb | 2 +- notifier/templates/pgss_table.erb | 3 +- 7 files changed, 46 insertions(+), 22 deletions(-) diff --git a/notifier/background_migration.rb b/notifier/background_migration.rb index 4ae89a36..1f478e05 100644 --- a/notifier/background_migration.rb +++ b/notifier/background_migration.rb @@ -40,4 +40,8 @@ class BackgroundMigration Query.new(query_data) end end + + def excluded_query_duration + queries.select(&:excluded?).sum(&:total_time) + end end diff --git a/notifier/migration.rb b/notifier/migration.rb index 718858e8..917ac715 100644 --- a/notifier/migration.rb +++ b/notifier/migration.rb @@ -180,4 +180,8 @@ class Migration def sort_key [type == TYPE_REGULAR ? 0 : 1, version] end + + def excluded_query_duration + queries.select(&:excluded?).sum(&:total_time) + end end diff --git a/notifier/spec/background_migration_spec.rb b/notifier/spec/background_migration_spec.rb index f21e8a54..393e779f 100644 --- a/notifier/spec/background_migration_spec.rb +++ b/notifier/spec/background_migration_spec.rb @@ -55,5 +55,12 @@ RSpec.describe BackgroundMigration do expect(aggregate_query_1.max_time).to eq(100) expect(aggregate_query_2.max_time).to eq(200) end + + describe '#excluded_query_duration' do + it 'is the sum of the duration of all excluded queries' do + # SELECT $1 is an excluded query + expect(background_migration.excluded_query_duration).to eq(aggregate_query_1.total_time) + end + end end end diff --git a/notifier/spec/migration_spec.rb b/notifier/spec/migration_spec.rb index 35d76ccd..8c1ff845 100644 --- a/notifier/spec/migration_spec.rb +++ b/notifier/spec/migration_spec.rb @@ -3,27 +3,29 @@ require 'spec_helper' RSpec.describe Migration do - let(:good_queries) do - [ - { - "query" => "select pg_database_size(current_database()) /*application:test*/", - "calls" => 1, - "total_time" => 87.621755, - "max_time" => 87.621755, - "mean_time" => 87.621755, - "rows" => 1 - }, - { - "query" => "select * from users limit 1 /*application:test*/", - "calls" => 1, - "total_time" => 80.51234, - "max_time" => 80.51234, - "mean_time" => 80.51234, - "rows" => 1 - } - ] + let(:excluded_query) do + { + "query" => "select pg_database_size(current_database()) /*application:test*/", + "calls" => 1, + "total_time" => 87.621755, + "max_time" => 87.621755, + "mean_time" => 87.621755, + "rows" => 1 + } end + let(:included_good_query) do + { + "query" => "select * from users limit 1 /*application:test*/", + "calls" => 1, + "total_time" => 80.51234, + "max_time" => 80.51234, + "mean_time" => 80.51234, + "rows" => 1 + } + end + let(:good_queries) { [excluded_query, included_good_query]} + let(:bad_query) do { "query" => "select * from users limit 1000 /*application:test*/", @@ -168,4 +170,10 @@ RSpec.describe Migration do expect(post_migration.sort_key).to eq([1, 42]) end end + + describe '#excluded_query_duration' do + it 'is the sum of the duration of all excluded queries' do + expect(subject.excluded_query_duration).to eq(excluded_query['total_time']) + end + end end diff --git a/notifier/templates/background_migration_detail.erb b/notifier/templates/background_migration_detail.erb index 84748519..85c45719 100644 --- a/notifier/templates/background_migration_detail.erb +++ b/notifier/templates/background_migration_detail.erb @@ -3,7 +3,7 @@ Sampled <%= background_migration.batches.count %> batches % if background_migration.queries.any? -<%= render_pgss_table(background_migration.queries) %> +<%= render_pgss_table(background_migration) %> % end <%= render_background_migration_batch_histogram(background_migration) %> diff --git a/notifier/templates/detail.erb b/notifier/templates/detail.erb index 2e4a6aab..8268e46c 100644 --- a/notifier/templates/detail.erb +++ b/notifier/templates/detail.erb @@ -5,7 +5,7 @@ * Database size change: <%= total_size_change(migration) %> % if migration.queries.any? -<%= render_pgss_table(migration.queries) %> +<%= render_pgss_table(migration) %> % end <%= render_migration_histogram(migration) %> \ No newline at end of file diff --git a/notifier/templates/pgss_table.erb b/notifier/templates/pgss_table.erb index 2118af46..e912058e 100644 --- a/notifier/templates/pgss_table.erb +++ b/notifier/templates/pgss_table.erb @@ -1,4 +1,5 @@ -% pgss_rows = pgss.reject(&:excluded?) +% pgss_rows = pgss.queries.reject(&:excluded?) +Executed <%= format_time(pgss.excluded_query_duration) %> of common ignored queries. % unless pgss_rows.empty? | Query | Calls | Total Time | Max Time | Mean Time | Rows | | ----- | ----- | ---------- | -------- | --------- | ---- | -- GitLab From 08ea7612d8538900206b36b2ff21fb6a6786d0e1 Mon Sep 17 00:00:00 2001 From: Simon Tomlinson Date: Fri, 29 Apr 2022 11:47:29 -0500 Subject: [PATCH 2/2] Regenerate end-to-end comment --- .../spec/fixtures/migration-testing/expected-comment.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/notifier/spec/fixtures/migration-testing/expected-comment.txt b/notifier/spec/fixtures/migration-testing/expected-comment.txt index 23ad4215..d077fb12 100644 --- a/notifier/spec/fixtures/migration-testing/expected-comment.txt +++ b/notifier/spec/fixtures/migration-testing/expected-comment.txt @@ -50,6 +50,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 1.6 s * Database size change: +24.00 KiB +Executed 131.7 ms of common ignored queries. | Query | Calls | Total Time | Max Time | Mean Time | Rows | | ----- | ----- | ---------- | -------- | --------- | ---- | | 
CREATE TABLE "test_tables" ("id" bigserial primary key, "stars" bigint DEFAULT 0 NOT NULL, "created_at" timestamp(6) NOT NULL, "updated_at" timestamp(6) NOT NULL, "title" text, "notes" text) /*application:test,db_config_name:main*/
| 1 | 60.2 ms | 60.2 ms | 60.2 ms | 0 | @@ -75,6 +76,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 6.1 s * Database size change: +0.00 B +Executed 120.5 ms of common ignored queries. | Query | Calls | Total Time | Max Time | Mean Time | Rows | | ----- | ----- | ---------- | -------- | --------- | ---- | | 
SELECT pg_sleep($1)
| 1 | 5003.7 ms | 5003.7 ms | 5003.7 ms | 1 | @@ -98,6 +100,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 1.1 s * Database size change: +0.00 B +Executed 140.1 ms of common ignored queries. #### Migration: 20220223163519 - EnsureGitlabComInMigrations @@ -105,6 +108,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 1.1 s * Database size change: +0.00 B +Executed 133.3 ms of common ignored queries. #### Migration: 20220318174439 - QueueTestBackgroundMigration @@ -112,6 +116,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 1.2 s * Database size change: +0.00 B +Executed 135.4 ms of common ignored queries. #### :warning: Migration: 20201015154529 - AddMigrationThatRequiresBackground @@ -126,6 +131,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 1.2 s * Database size change: -24.00 KiB +Executed 135.8 ms of common ignored queries. | Query | Calls | Total Time | Max Time | Mean Time | Rows | | ----- | ----- | ---------- | -------- | --------- | ---- | | 
DROP TABLE "test_tables" /*application:test,db_config_name:main*/
| 1 | 6.7 ms | 6.7 ms | 6.7 ms | 0 | @@ -149,6 +155,7 @@ Migrations included in this change have been executed on gitlab.com data for tes * Duration: 0.9 s * Database size change: +0.00 B +Executed 133.5 ms of common ignored queries. --- @@ -159,6 +166,7 @@ Migrations included in this change have been executed on gitlab.com data for tes Sampled 5 batches +Executed 714.4 ms of common ignored queries. | Query | Calls | Total Time | Max Time | Mean Time | Rows | | ----- | ----- | ---------- | -------- | --------- | ---- | | 
select pg_sleep($1) /*application:test,db_config_name:main*/
| 5 | 15013.9 ms | 5005.1 ms | 3002.8 ms | 5 | -- GitLab