From d5c60665b15da0f7fa6f902dde5fb5e433648dbe Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 19 Jan 2024 08:40:41 -0700 Subject: [PATCH] don't bother unnecessarily setting max_update_limit avoids 1 to 3 extra db round trips if we're already doing batches of 1000 Change-Id: I0b17a1072ad0a54c01ba0b9778f94bf3255ef014 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/338146 Tested-by: Service Cloud Jenkins Reviewed-by: Jacob Burroughs QA-Review: Cody Cutrer Product-Review: Cody Cutrer Migration-Review: Cody Cutrer --- config/initializers/postgresql_adapter.rb | 15 +++++++++++++++ .../20230222192524_guard_excessive_updates.rb | 2 +- db/migrate/20230317213850_better_guard_logs.rb | 4 ++-- .../20230330125148_even_better_guard_logs.rb | 4 ++-- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/config/initializers/postgresql_adapter.rb b/config/initializers/postgresql_adapter.rb index 31cc6d9533f..2af24545954 100644 --- a/config/initializers/postgresql_adapter.rb +++ b/config/initializers/postgresql_adapter.rb @@ -23,6 +23,14 @@ class QuotedValue < String end module PostgreSQLAdapterExtensions + # when changing this, you need to re-apply the latest migration creating the guard_excessive_updates function + DEFAULT_MAX_UPDATE_LIMIT = 1000 + + def initialize(*) + super + @max_update_limit = DEFAULT_MAX_UPDATE_LIMIT + end + def configure_connection super @@ -258,6 +266,11 @@ module PostgreSQLAdapterExtensions end def with_max_update_limit(limit) + return yield if limit == @max_update_limit + + old_limit = @max_update_limit + @max_update_limit = limit + if transaction_open? execute("SET LOCAL inst.max_update_limit = #{limit}") ret = yield @@ -269,6 +282,8 @@ module PostgreSQLAdapterExtensions end end ret + ensure + @max_update_limit = old_limit if old_limit end def quote(*args) diff --git a/db/migrate/20230222192524_guard_excessive_updates.rb b/db/migrate/20230222192524_guard_excessive_updates.rb index c73702f8dbf..2f903de5bbf 100644 --- a/db/migrate/20230222192524_guard_excessive_updates.rb +++ b/db/migrate/20230222192524_guard_excessive_updates.rb @@ -55,7 +55,7 @@ class GuardExcessiveUpdates < ActiveRecord::Migration[7.0] max_record_count integer; BEGIN SELECT count(*) FROM oldtbl INTO record_count; - max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '1000'); + max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '#{PostgreSQLAdapterExtensions::DEFAULT_MAX_UPDATE_LIMIT}'); IF record_count > max_record_count THEN IF current_setting('inst.max_update_fail', true) IS NOT DISTINCT FROM 'true' THEN RAISE EXCEPTION 'guard_excessive_updates: % to %.% failed. Would update % records but max is %', TG_OP, TG_TABLE_SCHEMA, TG_TABLE_NAME, record_count, max_record_count; diff --git a/db/migrate/20230317213850_better_guard_logs.rb b/db/migrate/20230317213850_better_guard_logs.rb index 97d589cf0a4..b09f08888d9 100644 --- a/db/migrate/20230317213850_better_guard_logs.rb +++ b/db/migrate/20230317213850_better_guard_logs.rb @@ -29,7 +29,7 @@ class BetterGuardLogs < ActiveRecord::Migration[7.0] max_record_count integer; BEGIN SELECT count(*) FROM oldtbl INTO record_count; - max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '1000'); + max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '#{PostgreSQLAdapterExtensions::DEFAULT_MAX_UPDATE_LIMIT}'); IF record_count > max_record_count THEN IF current_setting('inst.max_update_fail', true) IS NOT DISTINCT FROM 'true' THEN RAISE EXCEPTION 'guard_excessive_updates: % to %.% failed', TG_OP, TG_TABLE_SCHEMA, TG_TABLE_NAME USING DETAIL = 'Would update ' || record_count || ' records but max is ' || max_record_count; @@ -52,7 +52,7 @@ class BetterGuardLogs < ActiveRecord::Migration[7.0] max_record_count integer; BEGIN SELECT count(*) FROM oldtbl INTO record_count; - max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '1000'); + max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '#{PostgreSQLAdapterExtensions::DEFAULT_MAX_UPDATE_LIMIT}'); IF record_count > max_record_count THEN IF current_setting('inst.max_update_fail', true) IS NOT DISTINCT FROM 'true' THEN RAISE EXCEPTION 'guard_excessive_updates: % to %.% failed. Would update % records but max is %', TG_OP, TG_TABLE_SCHEMA, TG_TABLE_NAME, record_count, max_record_count; diff --git a/db/migrate/20230330125148_even_better_guard_logs.rb b/db/migrate/20230330125148_even_better_guard_logs.rb index 5ad8d6a2be7..a6e1e939be3 100644 --- a/db/migrate/20230330125148_even_better_guard_logs.rb +++ b/db/migrate/20230330125148_even_better_guard_logs.rb @@ -29,7 +29,7 @@ class EvenBetterGuardLogs < ActiveRecord::Migration[7.0] max_record_count integer; BEGIN SELECT count(*) FROM oldtbl INTO record_count; - max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '1000'); + max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '#{PostgreSQLAdapterExtensions::DEFAULT_MAX_UPDATE_LIMIT}'); IF record_count > max_record_count THEN IF current_setting('inst.max_update_fail', true) IS NOT DISTINCT FROM 'true' THEN RAISE EXCEPTION 'guard_excessive_updates: % to %.% failed', TG_OP, TG_TABLE_SCHEMA, TG_TABLE_NAME USING DETAIL = 'Would update ' || record_count || ' records but max is ' || max_record_count || ', orig query: ' || current_query(); @@ -52,7 +52,7 @@ class EvenBetterGuardLogs < ActiveRecord::Migration[7.0] max_record_count integer; BEGIN SELECT count(*) FROM oldtbl INTO record_count; - max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '1000'); + max_record_count := COALESCE(setting_as_int('inst.max_update_limit.' || TG_TABLE_NAME), setting_as_int('inst.max_update_limit'), '#{PostgreSQLAdapterExtensions::DEFAULT_MAX_UPDATE_LIMIT}'); IF record_count > max_record_count THEN IF current_setting('inst.max_update_fail', true) IS NOT DISTINCT FROM 'true' THEN RAISE EXCEPTION 'guard_excessive_updates: % to %.% failed', TG_OP, TG_TABLE_SCHEMA, TG_TABLE_NAME USING DETAIL = 'Would update ' || record_count || ' records but max is ' || max_record_count;