18F · mitchellhenke · Dec 29, 2022 · Dec 13, 2022
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -292,15 +292,15 @@ GEM
       rake
     formatador (0.2.5)
     foundation_emails (2.2.1.0)
-    fugit (1.7.1)
+    fugit (1.8.0)
       et-orbi (~> 1, >= 1.2.7)
       raabro (~> 1.4)
     geocoder (1.7.0)
     get_process_mem (0.2.7)
       ffi (~> 1.0)
     globalid (1.0.0)
       activesupport (>= 5.0)
-    good_job (3.6.0)
+    good_job (3.7.2)
       activejob (>= 6.0.0)
       activerecord (>= 6.0.0)
       concurrent-ruby (>= 1.0.2)
@@ -714,7 +714,7 @@ GEM
     xpath (3.2.0)
       nokogiri (~> 1.8)
     yard (0.9.26)
-    zeitwerk (2.6.1)
+    zeitwerk (2.6.6)
     zonebie (0.6.1)
     zxcvbn (0.1.7)
 

diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb
@@ -10,14 +10,6 @@ class GetUspsProofingResultsJob < ApplicationJob
   ]
 
   queue_as :long_running
-  include GoodJob::ActiveJobExtensions::Concurrency
-
-  good_job_control_concurrency_with(
-    total_limit: 1,
-    key: 'get_usps_proofing_results',
-  )
-
-  discard_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError
 
   def email_analytics_attributes(enrollment)
     {

diff --git a/app/jobs/gpo_daily_job.rb b/app/jobs/gpo_daily_job.rb
@@ -1,15 +1,6 @@
 class GpoDailyJob < ApplicationJob
   queue_as :low
 
-  include GoodJob::ActiveJobExtensions::Concurrency
-
-  good_job_control_concurrency_with(
-    total_limit: 1,
-    key: -> { "gpo-daily-job-#{arguments.first}" },
-  )
-
-  discard_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError
-
   # Enqueue a test letter every day, but only upload letters on working weekdays
   def perform(date)
     GpoDailyTestSender.new.run

diff --git a/app/jobs/in_person/email_reminder_job.rb b/app/jobs/in_person/email_reminder_job.rb
@@ -7,15 +7,6 @@ class EmailReminderJob < ApplicationJob
 
     queue_as :low
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: 'in_person_email_reminder_job',
-    )
-
-    discard_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError
-
     def perform(_now)
       return true unless IdentityConfig.store.in_person_proofing_enabled
 

diff --git a/app/jobs/phone_number_opt_out_sync_job.rb b/app/jobs/phone_number_opt_out_sync_job.rb
@@ -1,14 +1,5 @@
 class PhoneNumberOptOutSyncJob < ApplicationJob
   queue_as :long_running
-  include GoodJob::ActiveJobExtensions::Concurrency
-
-  good_job_control_concurrency_with(
-    total_limit: 1,
-    key: -> do
-      rounded = TimeService.round_time(time: arguments.first, interval: 1.hour)
-      "phone-number-opt-out-sync-#{rounded.to_i}"
-    end,
-  )
 
   def perform(_now)
     all_phone_numbers = Set.new

diff --git a/app/jobs/psql_stats_job.rb b/app/jobs/psql_stats_job.rb
@@ -1,16 +1,5 @@
 class PsqlStatsJob < ApplicationJob
   queue_as :default
-  include GoodJob::ActiveJobExtensions::Concurrency
-
-  good_job_control_concurrency_with(
-    total_limit: 1,
-    key: -> do
-      rounded = TimeService.round_time(time: arguments.first, interval: 1.minute)
-      "psql_bloat_statistics-#{rounded.to_i}"
-    end,
-  )
-
-  discard_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError
 
   # gather data on bloat for each table
   # https://github.com/ioguix/pgsql-bloat-estimation/blob/master/table/table_bloat.sql

diff --git a/app/jobs/reports/agreement_summary_report.rb b/app/jobs/reports/agreement_summary_report.rb
@@ -4,13 +4,6 @@ module Reports
   class AgreementSummaryReport < BaseReport
     REPORT_NAME = 'agreement-summary-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(_date)
       csv = build_report
 

diff --git a/app/jobs/reports/base_report.rb b/app/jobs/reports/base_report.rb
@@ -4,9 +4,6 @@ module Reports
   class BaseReport < ApplicationJob
     queue_as :long_running
 
-    # We use good_job's concurrency features to cancel "extra" or duplicative runs of the same job
-    discard_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError
-
     def self.transaction_with_timeout(rails_env = Rails.env)
       # rspec-rails's use_transactional_tests does not seem to act as expected when switching
       # connections mid-test, so we just skip for now :[

diff --git a/app/jobs/reports/combined_invoice_supplement_report.rb b/app/jobs/reports/combined_invoice_supplement_report.rb
@@ -4,13 +4,6 @@ module Reports
   class CombinedInvoiceSupplementReport < BaseReport
     REPORT_NAME = 'combined-invoice-supplement-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(_date)
       iaas = IaaReportingHelper.iaas
 

diff --git a/app/jobs/reports/daily_auths_report.rb b/app/jobs/reports/daily_auths_report.rb
@@ -2,13 +2,6 @@ module Reports
   class DailyAuthsReport < BaseReport
     REPORT_NAME = 'daily-auths-report'
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     attr_reader :report_date
 
     def perform(report_date)

diff --git a/app/jobs/reports/daily_dropoffs_report.rb b/app/jobs/reports/daily_dropoffs_report.rb
@@ -18,13 +18,6 @@ class DailyDropoffsReport < BaseReport
       verified
     ].freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     attr_reader :report_date
 
     def perform(report_date)

diff --git a/app/jobs/reports/deleted_user_accounts_report.rb b/app/jobs/reports/deleted_user_accounts_report.rb
@@ -5,13 +5,6 @@ module Reports
   class DeletedUserAccountsReport < BaseReport
     REPORT_NAME = 'deleted-user-accounts-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(_date)
       configs = IdentityConfig.store.deleted_user_accounts_report_configs
       configs.each do |report_hash|

diff --git a/app/jobs/reports/irs_weekly_summary_report.rb b/app/jobs/reports/irs_weekly_summary_report.rb
@@ -5,13 +5,6 @@ class IrsWeeklySummaryReport < BaseReport
     attr_reader :report_date
     REPORT_NAME = 'irs-weekly-summary-report'
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(report_date)
       @name = REPORT_NAME
       @report_date = report_date

diff --git a/app/jobs/reports/monthly_gpo_letter_requests_report.rb b/app/jobs/reports/monthly_gpo_letter_requests_report.rb
@@ -4,13 +4,6 @@ module Reports
   class MonthlyGpoLetterRequestsReport < BaseReport
     REPORT_NAME = 'monthly-usps-letter-requests-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(_date, start_time: first_of_this_month, end_time: end_of_today)
       daily_results = transaction_with_timeout do
         ::LetterRequestsToGpoFtpLog.where(ftp_at: start_time..end_time)

diff --git a/app/jobs/reports/sp_active_users_report.rb b/app/jobs/reports/sp_active_users_report.rb
@@ -4,13 +4,6 @@ module Reports
   class SpActiveUsersReport < BaseReport
     REPORT_NAME = 'sp-active-users-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     # This daily job captures the total number of active users per SP from the beginning of the the
     # current fiscal year until now.
     #

diff --git a/app/jobs/reports/sp_user_counts_report.rb b/app/jobs/reports/sp_user_counts_report.rb
@@ -4,13 +4,6 @@ module Reports
   class SpUserCountsReport < BaseReport
     REPORT_NAME = 'sp-user-counts-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(_date)
       user_counts = transaction_with_timeout do
         Db::Identity::SpUserCounts.call

diff --git a/app/jobs/reports/total_ial2_costs_report.rb b/app/jobs/reports/total_ial2_costs_report.rb
@@ -5,13 +5,6 @@ class TotalIal2CostsReport < BaseReport
     REPORT_NAME = 'total-ial2-costs'.freeze
     NUM_LOOKBACK_DAYS = 45
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(date)
       results = transaction_with_timeout { query(date) }
 

diff --git a/app/jobs/reports/total_monthly_auths_report.rb b/app/jobs/reports/total_monthly_auths_report.rb
@@ -4,13 +4,6 @@ module Reports
   class TotalMonthlyAuthsReport < BaseReport
     REPORT_NAME = 'total-monthly-auths-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(_date)
       auth_counts = Db::MonthlySpAuthCount::TotalMonthlyAuthCounts.call
       save_report(REPORT_NAME, auth_counts.to_json, extension: 'json')

diff --git a/app/jobs/reports/verification_failures_report.rb b/app/jobs/reports/verification_failures_report.rb
@@ -6,13 +6,6 @@ module Reports
   class VerificationFailuresReport < BaseReport
     REPORT_NAME = 'verification-failures-report'.freeze
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> { "#{REPORT_NAME}-#{arguments.first}" },
-    )
-
     def perform(date)
       csv_reports = []
       configs = IdentityConfig.store.verification_errors_report_configs

diff --git a/app/services/account_reset/grant_requests_and_send_emails.rb b/app/services/account_reset/grant_requests_and_send_emails.rb
@@ -2,18 +2,6 @@ module AccountReset
   class GrantRequestsAndSendEmails < ApplicationJob
     queue_as :low
 
-    include GoodJob::ActiveJobExtensions::Concurrency
-
-    good_job_control_concurrency_with(
-      total_limit: 1,
-      key: -> do
-        rounded = TimeService.round_time(time: arguments.first, interval: 5.minutes)
-        "grant-requests-and-send-emails-#{rounded.to_i}"
-      end,
-    )
-
-    discard_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError
-
     def perform(now)
       notifications_sent = 0
       AccountResetRequest.where(

diff --git a/config/application.rb b/config/application.rb
@@ -69,7 +69,7 @@ class Application < Rails::Application
     config.good_job.enable_cron = true
     config.good_job.max_threads = IdentityConfig.store.good_job_max_threads
     config.good_job.queues = IdentityConfig.store.good_job_queues
-    config.good_job.preserve_job_records = true
+    config.good_job.preserve_job_records = false
     config.good_job.queue_select_limit = IdentityConfig.store.good_job_queue_select_limit
     # see config/initializers/job_configurations.rb for cron schedule
 

diff --git a/spec/jobs/gpo_daily_job_spec.rb b/spec/jobs/gpo_daily_job_spec.rb
@@ -48,13 +48,4 @@
       end
     end
   end
-
-  describe '#good_job_concurrency_key' do
-    let(:date) { Time.zone.today }
-
-    it 'is the job name and the date' do
-      job = described_class.new(date)
-      expect(job.good_job_concurrency_key).to eq("gpo-daily-job-#{date}")
-    end
-  end
 end
diff --git a/spec/jobs/phone_number_opt_out_sync_job_spec.rb b/spec/jobs/phone_number_opt_out_sync_job_spec.rb
@@ -32,19 +32,4 @@
       end
     end
   end
-
-  describe '#good_job_concurrency_key' do
-    it 'is the job name and the current time, rounded to the nearest hour' do
-      now = Time.zone.at(1629817200)
-
-      job_now = PhoneNumberOptOutSyncJob.new(now)
-      expect(job_now.good_job_concurrency_key).to eq("phone-number-opt-out-sync-#{now.to_i}")
-
-      job_plus_30m = PhoneNumberOptOutSyncJob.new(now + 30.minutes)
-      expect(job_plus_30m.good_job_concurrency_key).to eq(job_now.good_job_concurrency_key)
-
-      job_plus_1h = PhoneNumberOptOutSyncJob.new(now + 1.hour)
-      expect(job_plus_1h.good_job_concurrency_key).to_not eq(job_now.good_job_concurrency_key)
-    end
-  end
 end
diff --git a/spec/jobs/reports/agreement_summary_report_spec.rb b/spec/jobs/reports/agreement_summary_report_spec.rb
@@ -78,14 +78,4 @@
       end
     end
   end
-
-  describe '#good_job_concurrency_key' do
-    let(:date) { Time.zone.today }
-
-    it 'is the job name and the date' do
-      job = described_class.new(date)
-      expect(job.good_job_concurrency_key).
-        to eq("#{described_class::REPORT_NAME}-#{date}")
-    end
-  end
 end
diff --git a/spec/jobs/reports/combined_invoice_supplement_report_spec.rb b/spec/jobs/reports/combined_invoice_supplement_report_spec.rb
@@ -215,14 +215,4 @@ def build_integration(issuer:, partner_account:)
       partner_account: partner_account,
     )
   end
-
-  describe '#good_job_concurrency_key' do
-    let(:date) { Time.zone.today }
-
-    it 'is the job name and the date' do
-      job = described_class.new(date)
-      expect(job.good_job_concurrency_key).
-        to eq("#{described_class::REPORT_NAME}-#{date}")
-    end
-  end
 end
diff --git a/spec/jobs/reports/daily_auths_report_spec.rb b/spec/jobs/reports/daily_auths_report_spec.rb
@@ -108,14 +108,4 @@
       end
     end
   end
-
-  describe '#good_job_concurrency_key' do
-    let(:date) { Time.zone.today }
-
-    it 'is the job name and the date' do
-      job = described_class.new(date)
-      expect(job.good_job_concurrency_key).
-        to eq("#{described_class::REPORT_NAME}-#{date}")
-    end
-  end
 end