From 3dc8cc78c58b574a7b673300e577feb7235c0969 Mon Sep 17 00:00:00 2001 From: Marcello Rocha Date: Mon, 2 Dec 2024 12:22:13 +0100 Subject: [PATCH 1/4] Update GoodJob to 3.99 --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 34cdfce5411f..2585bd68a511 100644 --- a/Gemfile +++ b/Gemfile @@ -126,7 +126,7 @@ gem "multi_json", "~> 1.15.0" gem "oj", "~> 3.16.0" gem "daemons" -gem "good_job", "= 3.26.2" # update should be done manually in sync with saas-openproject version. +gem "good_job", "= 3.99.1" # update should be done manually in sync with saas-openproject version. gem "rack-protection", "~> 3.2.0" diff --git a/Gemfile.lock b/Gemfile.lock index 0e0235003f74..ea1eee3ba16b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -601,7 +601,7 @@ GEM i18n (>= 0.7) multi_json request_store (>= 1.0) - good_job (3.26.2) + good_job (3.99.1) activejob (>= 6.0.0) activerecord (>= 6.0.0) concurrent-ruby (>= 1.0.2) @@ -1261,7 +1261,7 @@ DEPENDENCIES friendly_id (~> 5.5.0) fuubar (~> 2.5.0) gon (~> 6.4.0) - good_job (= 3.26.2) + good_job (= 3.99.1) google-apis-gmail_v1 googleauth grape (~> 2.2.0) From 0257c60b352a744af2f62ca24db75c64530ff46f Mon Sep 17 00:00:00 2001 From: Marcello Rocha Date: Mon, 2 Dec 2024 18:08:23 +0100 Subject: [PATCH 2/4] Adds the necessary migrations --- ...eate_good_job_execution_error_backtrace.rb | 15 ++++++ ...112407_create_good_job_process_lock_ids.rb | 20 +++++++ ...08_create_good_job_process_lock_indexes.rb | 52 +++++++++++++++++++ ...2409_create_good_job_execution_duration.rb | 15 ++++++ 4 files changed, 102 insertions(+) create mode 100644 db/migrate/20241202112406_create_good_job_execution_error_backtrace.rb create mode 100644 db/migrate/20241202112407_create_good_job_process_lock_ids.rb create mode 100644 db/migrate/20241202112408_create_good_job_process_lock_indexes.rb create mode 100644 db/migrate/20241202112409_create_good_job_execution_duration.rb diff --git a/db/migrate/20241202112406_create_good_job_execution_error_backtrace.rb b/db/migrate/20241202112406_create_good_job_execution_error_backtrace.rb new file mode 100644 index 000000000000..6b054b640601 --- /dev/null +++ b/db/migrate/20241202112406_create_good_job_execution_error_backtrace.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateGoodJobExecutionErrorBacktrace < ActiveRecord::Migration[7.1] + def change + reversible do |dir| + dir.up do + # Ensure this incremental update migration is idempotent + # with monolithic install migration. + return if connection.column_exists?(:good_job_executions, :error_backtrace) + end + end + + add_column :good_job_executions, :error_backtrace, :text, array: true + end +end diff --git a/db/migrate/20241202112407_create_good_job_process_lock_ids.rb b/db/migrate/20241202112407_create_good_job_process_lock_ids.rb new file mode 100644 index 000000000000..901fb607e95c --- /dev/null +++ b/db/migrate/20241202112407_create_good_job_process_lock_ids.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateGoodJobProcessLockIds < ActiveRecord::Migration[7.1] + def change + reversible do |dir| + dir.up do + # Ensure this incremental update migration is idempotent + # with monolithic install migration. + return if connection.column_exists?(:good_jobs, :locked_by_id) + end + end + + # rubocop:disable Rails/BulkChangeTable + add_column :good_jobs, :locked_by_id, :uuid + add_column :good_jobs, :locked_at, :datetime + add_column :good_job_executions, :process_id, :uuid + add_column :good_job_processes, :lock_type, :integer, limit: 2 + # rubocop:enable Rails/BulkChangeTable + end +end diff --git a/db/migrate/20241202112408_create_good_job_process_lock_indexes.rb b/db/migrate/20241202112408_create_good_job_process_lock_indexes.rb new file mode 100644 index 000000000000..89fbe6e00369 --- /dev/null +++ b/db/migrate/20241202112408_create_good_job_process_lock_indexes.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +class CreateGoodJobProcessLockIndexes < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + # rubocop:disable Metrics/AbcSize + def change + reversible do |dir| + dir.up do + unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked) + add_index :good_jobs, %i[priority scheduled_at], + order: { priority: "ASC NULLS LAST", scheduled_at: :asc }, + where: "finished_at IS NULL AND locked_by_id IS NULL", + name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked, + algorithm: :concurrently + end + + unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_locked_by_id) + add_index :good_jobs, :locked_by_id, + where: "locked_by_id IS NOT NULL", + name: :index_good_jobs_on_locked_by_id, + algorithm: :concurrently + end + + unless connection.index_name_exists?(:good_job_executions, :index_good_job_executions_on_process_id_and_created_at) + add_index :good_job_executions, %i[process_id created_at], + name: :index_good_job_executions_on_process_id_and_created_at, + algorithm: :concurrently + end + end + + dir.down do + if connection.index_name_exists?( + :good_jobs, :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked + ) + remove_index(:good_jobs, name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked) + end + + if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_locked_by_id) + remove_index(:good_jobs, name: :index_good_jobs_on_locked_by_id) + end + + if connection.index_name_exists?( + :good_job_executions, :index_good_job_executions_on_process_id_and_created_at + ) + remove_index(:good_job_executions, name: :index_good_job_executions_on_process_id_and_created_at) + end + end + end + end + # rubocop:enable Metrics/AbcSize +end diff --git a/db/migrate/20241202112409_create_good_job_execution_duration.rb b/db/migrate/20241202112409_create_good_job_execution_duration.rb new file mode 100644 index 000000000000..fef37f07bc1f --- /dev/null +++ b/db/migrate/20241202112409_create_good_job_execution_duration.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateGoodJobExecutionDuration < ActiveRecord::Migration[7.1] + def change + reversible do |dir| + dir.up do + # Ensure this incremental update migration is idempotent + # with monolithic install migration. + return if connection.column_exists?(:good_job_executions, :duration) + end + end + + add_column :good_job_executions, :duration, :interval + end +end From f75d808d8da86e7fe2dfa2f5f8f9236fa2795ee5 Mon Sep 17 00:00:00 2001 From: Marcello Rocha Date: Tue, 3 Dec 2024 10:26:03 +0100 Subject: [PATCH 3/4] Updates config as we had the wrong behaviour around and tests --- Gemfile | 5 +++-- Gemfile.lock | 5 +---- config/application.rb | 2 +- .../storages/manage_storage_integrations_job_spec.rb | 6 +++++- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index 2585bd68a511..68c558264d76 100644 --- a/Gemfile +++ b/Gemfile @@ -333,8 +333,9 @@ group :development, :test do # Output a stack trace anytime, useful when a process is stuck gem "rbtrace" - # REPL with debug commands - gem "debug" + # REPL with debug commands, Debug changed to byebug due to the issue below + # https://github.com/puma/puma/issues/2835#issuecomment-2302133927 + gem "byebug" gem "pry-byebug", "~> 3.10.0", platforms: [:mri] gem "pry-doc" diff --git a/Gemfile.lock b/Gemfile.lock index ea1eee3ba16b..aa6b663d2854 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -453,9 +453,6 @@ GEM date_validator (0.12.0) activemodel (>= 3) activesupport (>= 3) - debug (1.9.2) - irb (~> 1.10) - reline (>= 0.3.8) deckar01-task_list (2.3.4) html-pipeline (~> 2.0) declarative (0.0.20) @@ -1222,6 +1219,7 @@ DEPENDENCIES brakeman (~> 6.2.0) browser (~> 6.2.0) budgets! + byebug capybara (~> 3.40.0) capybara-screenshot (~> 1.0.17) capybara_accessible_selectors! @@ -1239,7 +1237,6 @@ DEPENDENCIES dalli (~> 3.2.0) dashboards! date_validator (~> 0.12.0) - debug deckar01-task_list (~> 2.3.1) disposable (~> 0.6.2) doorkeeper (~> 5.8.0) diff --git a/config/application.rb b/config/application.rb index 0b5c8cff7a79..01330dc51641 100644 --- a/config/application.rb +++ b/config/application.rb @@ -243,7 +243,7 @@ class Application < Rails::Application config.good_job.max_cache = OpenProject::Configuration[:good_job_max_cache] config.good_job.enable_cron = OpenProject::Configuration[:good_job_enable_cron] config.good_job.shutdown_timeout = 30 - config.good_job.smaller_number_is_higher_priority = false + config.good_job.smaller_number_is_higher_priority = true config.action_controller.asset_host = OpenProject::Configuration::AssetHost.value diff --git a/modules/storages/spec/workers/storages/manage_storage_integrations_job_spec.rb b/modules/storages/spec/workers/storages/manage_storage_integrations_job_spec.rb index e76ed531522a..663f9b022ee5 100644 --- a/modules/storages/spec/workers/storages/manage_storage_integrations_job_spec.rb +++ b/modules/storages/spec/workers/storages/manage_storage_integrations_job_spec.rb @@ -76,11 +76,15 @@ expect(good_job_setting.key).to eq("cron_keys_disabled") expect(good_job_setting.value).to eq(["Storages::ManageStorageIntegrationsJob"]) - expect { described_class.disable_cron_job_if_needed }.not_to change(GoodJob::Setting, :count).from(1) + expect { described_class.disable_cron_job_if_needed }.to change(GoodJob::Setting, :count).from(1).to(2) good_job_setting.reload expect(good_job_setting.key).to eq("cron_keys_disabled") expect(good_job_setting.value).to eq([]) + + good_job_setting = GoodJob::Setting.second + expect(good_job_setting.key).to eq("cron_keys_enabled") + expect(good_job_setting.value).to eq(["Storages::ManageStorageIntegrationsJob"]) end it "does nothing if the cron_job is not disabled" do From c9c271e08eddc7e5a5a463d165084e3660b91541 Mon Sep 17 00:00:00 2001 From: Marcello Rocha Date: Tue, 3 Dec 2024 11:30:17 +0100 Subject: [PATCH 4/4] Tries to satisfy rubocop --- db/migrate/20241202112407_create_good_job_process_lock_ids.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/migrate/20241202112407_create_good_job_process_lock_ids.rb b/db/migrate/20241202112407_create_good_job_process_lock_ids.rb index 901fb607e95c..f1b70a8f2e43 100644 --- a/db/migrate/20241202112407_create_good_job_process_lock_ids.rb +++ b/db/migrate/20241202112407_create_good_job_process_lock_ids.rb @@ -10,11 +10,9 @@ def change end end - # rubocop:disable Rails/BulkChangeTable add_column :good_jobs, :locked_by_id, :uuid add_column :good_jobs, :locked_at, :datetime add_column :good_job_executions, :process_id, :uuid add_column :good_job_processes, :lock_type, :integer, limit: 2 - # rubocop:enable Rails/BulkChangeTable end end