From 253a084af7f6161f5e7d6c5c1f12bef6cfd469ff Mon Sep 17 00:00:00 2001 From: "Ben Sheldon [he/him]" Date: Sun, 26 Jun 2022 09:32:49 -0700 Subject: [PATCH] Remove database migration shims and old migrations (#642) --- .../good_job/processes_controller.rb | 2 +- app/views/good_job/processes/index.html.erb | 10 +------- .../migrations/01_create_good_jobs.rb.erb | 9 +++++++ .../02_add_cron_at_to_good_jobs.rb.erb | 14 ----------- ...cron_key_cron_at_index_to_good_jobs.rb.erb | 20 --------------- .../04_create_good_job_processes.rb.erb | 17 ------------- ..._index_good_job_jobs_on_finished_at.rb.erb | 25 ------------------- lib/good_job/notifier/process_registration.rb | 4 --- lib/models/good_job/execution.rb | 9 +------ lib/models/good_job/process.rb | 9 ------- 10 files changed, 12 insertions(+), 107 deletions(-) delete mode 100644 lib/generators/good_job/templates/update/migrations/02_add_cron_at_to_good_jobs.rb.erb delete mode 100644 lib/generators/good_job/templates/update/migrations/03_add_cron_key_cron_at_index_to_good_jobs.rb.erb delete mode 100644 lib/generators/good_job/templates/update/migrations/04_create_good_job_processes.rb.erb delete mode 100644 lib/generators/good_job/templates/update/migrations/04_index_good_job_jobs_on_finished_at.rb.erb diff --git a/app/controllers/good_job/processes_controller.rb b/app/controllers/good_job/processes_controller.rb index 05d4fe541..16043ca95 100644 --- a/app/controllers/good_job/processes_controller.rb +++ b/app/controllers/good_job/processes_controller.rb @@ -2,7 +2,7 @@ module GoodJob class ProcessesController < GoodJob::ApplicationController def index - @processes = GoodJob::Process.active.order(created_at: :desc) if GoodJob::Process.migrated? + @processes = GoodJob::Process.active.order(created_at: :desc) end end end diff --git a/app/views/good_job/processes/index.html.erb b/app/views/good_job/processes/index.html.erb index a7203f1d3..3a81eb695 100644 --- a/app/views/good_job/processes/index.html.erb +++ b/app/views/good_job/processes/index.html.erb @@ -3,15 +3,7 @@
- <% if !GoodJob::Process.migrated? %> -
-
-

- Feature unavailable because of pending database migration. -

-
-
- <% elsif @processes.present? %> + <% if @processes.present? %>
diff --git a/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb b/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb index 27e5d054a..9cb695c78 100644 --- a/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb +++ b/lib/generators/good_job/templates/update/migrations/01_create_good_jobs.rb.erb @@ -18,6 +18,12 @@ class CreateGoodJobs < ActiveRecord::Migration<%= migration_version %> t.text :concurrency_key t.text :cron_key t.uuid :retried_good_job_id + t.timestamp :cron_at + end + + create_table :good_job_processes, id: :uuid do |t| + t.timestamps + t.jsonb :state end add_index :good_jobs, :scheduled_at, where: "(finished_at IS NULL)", name: "index_good_jobs_on_scheduled_at" @@ -25,5 +31,8 @@ class CreateGoodJobs < ActiveRecord::Migration<%= migration_version %> add_index :good_jobs, [:active_job_id, :created_at], name: :index_good_jobs_on_active_job_id_and_created_at add_index :good_jobs, :concurrency_key, where: "(finished_at IS NULL)", name: :index_good_jobs_on_concurrency_key_when_unfinished add_index :good_jobs, [:cron_key, :created_at], name: :index_good_jobs_on_cron_key_and_created_at + add_index :good_jobs, [:cron_key, :cron_at], name: :index_good_jobs_on_cron_key_and_cron_at, unique: true + add_index :good_jobs, [:active_job_id], name: :index_good_jobs_on_active_job_id + add_index :good_jobs, [:finished_at], where: "retried_good_job_id IS NULL AND finished_at IS NOT NULL", name: :index_good_jobs_jobs_on_finished_at end end diff --git a/lib/generators/good_job/templates/update/migrations/02_add_cron_at_to_good_jobs.rb.erb b/lib/generators/good_job/templates/update/migrations/02_add_cron_at_to_good_jobs.rb.erb deleted file mode 100644 index 7303e2a34..000000000 --- a/lib/generators/good_job/templates/update/migrations/02_add_cron_at_to_good_jobs.rb.erb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true -class AddCronAtToGoodJobs < ActiveRecord::Migration<%= migration_version %> - 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, :cron_at) - end - end - - add_column :good_jobs, :cron_at, :timestamp - end -end diff --git a/lib/generators/good_job/templates/update/migrations/03_add_cron_key_cron_at_index_to_good_jobs.rb.erb b/lib/generators/good_job/templates/update/migrations/03_add_cron_key_cron_at_index_to_good_jobs.rb.erb deleted file mode 100644 index 83f88daa7..000000000 --- a/lib/generators/good_job/templates/update/migrations/03_add_cron_key_cron_at_index_to_good_jobs.rb.erb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true -class AddCronKeyCronAtIndexToGoodJobs < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at) - end - end - - add_index :good_jobs, - [:cron_key, :cron_at], - algorithm: :concurrently, - name: :index_good_jobs_on_cron_key_and_cron_at, - unique: true - end -end diff --git a/lib/generators/good_job/templates/update/migrations/04_create_good_job_processes.rb.erb b/lib/generators/good_job/templates/update/migrations/04_create_good_job_processes.rb.erb deleted file mode 100644 index 8d70ca856..000000000 --- a/lib/generators/good_job/templates/update/migrations/04_create_good_job_processes.rb.erb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true -class CreateGoodJobProcesses < ActiveRecord::Migration<%= migration_version %> - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.table_exists?(:good_job_processes) - end - end - - create_table :good_job_processes, id: :uuid do |t| - t.timestamps - t.jsonb :state - end - end -end diff --git a/lib/generators/good_job/templates/update/migrations/04_index_good_job_jobs_on_finished_at.rb.erb b/lib/generators/good_job/templates/update/migrations/04_index_good_job_jobs_on_finished_at.rb.erb deleted file mode 100644 index e1cf5de5b..000000000 --- a/lib/generators/good_job/templates/update/migrations/04_index_good_job_jobs_on_finished_at.rb.erb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true -class IndexGoodJobJobsOnFinishedAt < ActiveRecord::Migration<%= migration_version %> - disable_ddl_transaction! - - def change - reversible do |dir| - dir.up do - # Ensure this incremental update migration is idempotent - # with monolithic install migration. - return if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_active_job_id) - end - end - - add_index :good_jobs, - [:active_job_id], - name: :index_good_jobs_on_active_job_id, - algorithm: :concurrently - - add_index :good_jobs, - [:finished_at], - where: "retried_good_job_id IS NULL AND finished_at IS NOT NULL", - name: :index_good_jobs_jobs_on_finished_at, - algorithm: :concurrently - end -end diff --git a/lib/good_job/notifier/process_registration.rb b/lib/good_job/notifier/process_registration.rb index 21c507707..945dd928f 100644 --- a/lib/good_job/notifier/process_registration.rb +++ b/lib/good_job/notifier/process_registration.rb @@ -14,8 +14,6 @@ module ProcessRegistration # Registers the current process. def register_process GoodJob::Process.with_connection(connection) do - next unless Process.migrated? - GoodJob::Process.cleanup @process = GoodJob::Process.register end @@ -24,8 +22,6 @@ def register_process # Deregisters the current process. def deregister_process GoodJob::Process.with_connection(connection) do - next unless Process.migrated? - @process&.deregister end end diff --git a/lib/models/good_job/execution.rb b/lib/models/good_job/execution.rb index 191a47c66..e5936a11e 100644 --- a/lib/models/good_job/execution.rb +++ b/lib/models/good_job/execution.rb @@ -207,14 +207,7 @@ def self.enqueue(active_job, scheduled_at: nil, create_with_advisory_lock: false if CurrentThread.cron_key execution_args[:cron_key] = CurrentThread.cron_key - - @cron_at_index = column_names.include?('cron_at') && connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at) unless instance_variable_defined?(:@cron_at_index) - - if @cron_at_index - execution_args[:cron_at] = CurrentThread.cron_at - else - migration_pending_warning! - end + execution_args[:cron_at] = CurrentThread.cron_at elsif CurrentThread.active_job_id && CurrentThread.active_job_id == active_job.job_id execution_args[:cron_key] = CurrentThread.execution.cron_key end diff --git a/lib/models/good_job/process.rb b/lib/models/good_job/process.rb index 79ff82888..a9aa79bfe 100644 --- a/lib/models/good_job/process.rb +++ b/lib/models/good_job/process.rb @@ -25,15 +25,6 @@ class Process < BaseRecord # @return [ActiveRecord::Relation] scope :inactive, -> { advisory_unlocked } - # Whether the +good_job_processes+ table exsists. - # @return [Boolean] - def self.migrated? - return true if connection.table_exists?(table_name) - - migration_pending_warning! - false - end - # UUID that is unique to the current process and changes when forked. # @return [String] def self.current_id