React on comments from review.

- Do not use string literals for job class names. Use `class.name` instead.
- Rename `ApplicationJob#good_job_scheduled_at` to `ApplicationJob#job_scheduled_at` to be backend agnostic.
- update queries in bin/check-worker-liveness to use good_jobs table
- Set good_job config options through appropriate OpenProject::Configuration
- Remove delayed_jobs table.
- Update health_check docs.

app/contracts/settings/working_days_params_contract.rb

@@ -45,7 +45,7 @@ def working_days_are_present
     def unique_job
       if GoodJob::Job
            .where(finished_at: nil)
-           .exists?(job_class: 'WorkPackages::ApplyWorkingDaysChangeJob')
+           .exists?(job_class: WorkPackages::ApplyWorkingDaysChangeJob.name)
         errors.add :base, :previous_working_day_changes_unprocessed
       end
     end

app/workers/application_job.rb

@@ -92,9 +92,8 @@ def reload_mailer_settings!
     Setting.reload_mailer_settings!
   end
 
-  def good_job_scheduled_at
-    GoodJob::Job.where(id: job_id)
-                .pick(:scheduled_at)
+  def job_scheduled_at
+    GoodJob::Job.where(id: job_id).pick(:scheduled_at)
   end
 
   private

app/workers/notifications/schedule_date_alerts_notifications_job.rb

@@ -53,7 +53,7 @@ def times_from_scheduled_to_execution
     end
 
     def scheduled_time
-      good_job_scheduled_at.then { |t| t.change(min: t.min / 15 * 15) }
+      job_scheduled_at.then { |t| t.change(min: t.min / 15 * 15) }
     end
   end
 end

app/workers/notifications/schedule_reminder_mails_job.rb

@@ -29,7 +29,7 @@
 module Notifications
   class ScheduleReminderMailsJob < ApplicationJob
     def perform
-      User.having_reminder_mail_to_send(good_job_scheduled_at)
+      User.having_reminder_mail_to_send(job_scheduled_at)
           .pluck(:id)
           .each do |user_id|
         Mails::ReminderJob.perform_later(user_id)

bin/check-worker-liveness

@@ -13,7 +13,7 @@ FAIL_COUNT_FILE=/tmp/.liveness-check-fail-count.op
 #
 # This checks across all workers and can't tell if it's only this worker in particular
 # that doesn't seem to be doing anything.
-UNPROCESSED_COUNT=`psql -d ${DATABASE_URL%%\?*} -c "select count(*) from delayed_jobs where locked_by is null and run_at < (now() - interval '15 minutes');" | tail -n +3 | head -n1 | tr -d ' '`
+UNPROCESSED_COUNT=`psql -d ${DATABASE_URL%%\?*} -c "select count(*) from good_jobs where finished_at is null and scheduled_at < (now() - interval '15 minutes');" | tail -n +3 | head -n1 | tr -d ' '`
 
 echo "unprocessed: $UNPROCESSED_COUNT"
 
@@ -25,7 +25,7 @@ if [ "$UNPROCESSED_COUNT" = "0" ]; then
 
   exit 0
 else
-  MIN_RUN_AT=`psql -d ${DATABASE_URL%%\?*} -c "select run_at from delayed_jobs where locked_by is null and cron is null and run_at is not null and run_at < (now() - interval '1 minutes') order by run_at asc limit 1;" | tail -n +3 | head -n1`
+  MIN_RUN_AT=`psql -d ${DATABASE_URL%%\?*} -c "select scheduled_at from good_jobs where finished_at is null and cron_key is null and scheduled_at is not null and scheduled_at < (now() - interval '1 minutes') order by scheduled_at asc limit 1;" | tail -n +3 | head -n1`
   LAST_MIN_RUN_AT=`cat $MIN_RUN_AT_FILE 2>/dev/null || echo 0`
   NUM_FAILS=`cat $FAIL_COUNT_FILE 2>/dev/null || echo 0`
 

config/application.rb

@@ -214,15 +214,17 @@ class Application < Rails::Application
 
     config.active_job.queue_adapter = :good_job
 
-    config.good_job.preserve_job_records = true
     config.good_job.retry_on_unhandled_error = false
-    # config.good_job.on_thread_error = -> (exception) { Rails.error.report(exception) }
+    # It has been commented out because AppSignal gem modifies ActiveJob::Base to report exceptions already.
+    # config.good_job.on_thread_error = -> (exception) { OpenProject.logger.error(exception) }
     config.good_job.execution_mode = :external
-    config.good_job.queues = '*'
-    config.good_job.max_threads = 20
-    config.good_job.poll_interval = 30
-    config.good_job.shutdown_timeout = 25
-    config.good_job.enable_cron = true
+    config.good_job.preserve_job_records = true
+    config.good_job.cleanup_preserved_jobs_before_seconds_ago = OpenProject::Configuration[:good_job_cleanup_preserved_jobs_before_seconds_ago]
+    config.good_job.queues = OpenProject::Configuration[:good_job_queues]
+    config.good_job.max_threads = OpenProject::Configuration[:good_job_max_threads]
+    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.action_controller.asset_host = OpenProject::Configuration::AssetHost.value

config/constants/settings/definition.rb

@@ -483,6 +483,36 @@ class Definition
         description: 'Forced page size for manually sorted work package views',
         default: 250
       },
+      good_job_queues: {
+        description: '',
+        format: :string,
+        writable: false,
+        default: '*'
+      },
+      good_job_max_threads: {
+        description: '',
+        format: :integer,
+        writable: false,
+        default: 20
+      },
+      good_job_max_cache: {
+        description: '',
+        format: :integer,
+        writable: false,
+        default: 10_000
+      },
+      good_job_enable_cron: {
+        description: '',
+        format: :boolean,
+        writable: false,
+        default: true
+      },
+      good_job_cleanup_preserved_jobs_before_seconds_ago: {
+        description: '',
+        format: :integer,
+        writable: false,
+        default: 7.days
+      },
       host_name: {
         default: "localhost:3000"
       },

config/initializers/cronjobs.rb

@@ -27,44 +27,44 @@
 #++
 
 # Register "Cron-like jobs"
-OpenProject::Application.configure do |application|
-  application.config.good_job.cron.merge!(
+Rails.application.config.after_initialize do
+  Rails.application.config.good_job.cron.merge!(
     {
       'Cron::ClearOldSessionsJob': {
         cron: '15 1 * * *', # runs at 1:15 nightly
-        class: 'Cron::ClearOldSessionsJob'
+        class: Cron::ClearOldSessionsJob.name
       },
       'Cron::ClearTmpCacheJob': {
         cron: '45 2 * * 7', # runs at 02:45 sundays
-        class: 'Cron::ClearTmpCacheJob'
+        class: Cron::ClearTmpCacheJob.name
       },
       'Cron::ClearUploadedFilesJob': {
         cron: '0 23 * * 5', # runs 23:00 fridays
-        class: 'Cron::ClearUploadedFilesJob'
+        class: Cron::ClearUploadedFilesJob.name
       },
       'OAuth::CleanupJob': {
         cron: '52 1 * * *',
-        class: 'OAuth::CleanupJob'
+        class: OAuth::CleanupJob.name
       },
       'PaperTrailAudits::CleanupJob': {
         cron: '3 4 * * 6',
-        class: 'PaperTrailAudits::CleanupJob'
+        class: PaperTrailAudits::CleanupJob.name
       },
       'Attachments::CleanupUncontaineredJob': {
         cron: '03 22 * * *',
-        class: 'Attachments::CleanupUncontaineredJob'
+        class: Attachments::CleanupUncontaineredJob.name
       },
       'Notifications::ScheduleDateAlertsNotificationsJob': {
         cron: '*/15 * * * *',
-        class: 'Notifications::ScheduleDateAlertsNotificationsJob'
+        class: Notifications::ScheduleDateAlertsNotificationsJob.name
       },
       'Notifications::ScheduleReminderMailsJob': {
         cron: '*/15 * * * *',
-        class: 'Notifications::ScheduleReminderMailsJob'
+        class: Notifications::ScheduleReminderMailsJob.name
       },
       'Ldap::SynchronizationJob': {
         cron: '30 23 * * *',
-        class: 'Ldap::SynchronizationJob'
+        class: Ldap::SynchronizationJob.name
       }
     }
   )

db/migrate/20240227154544_remove_delayed_jobs.rb

@@ -0,0 +1,47 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class RemoveDelayedJobs < ActiveRecord::Migration[7.1]
+  def change
+    drop_table :delayed_jobs do |t|
+      t.integer :priority, default: 0   # Allows some jobs to jump to the front of the queue
+      t.integer :attempts, default: 0   # Provides for retries, but still fail eventually.
+      t.text :handler                   # YAML-encoded string of the object that will do work
+      t.text :last_error                # reason for last failure (See Note below)
+      t.datetime :run_at                # When to run. Could be Time.zone.now for immediately, or sometime in the future.
+      t.datetime :locked_at             # Set when a client is working on this object
+      t.datetime :failed_at             # Set when all retries have failed (actually, by default, the record is deleted instead)
+      t.string :locked_by               # Who is working on this object (if locked)
+      t.timestamps null: true
+      t.string :queue
+      t.string :cron
+
+      t.index %i[priority run_at], name: 'delayed_jobs_priority'
+    end
+  end
+end

docs/installation-and-operations/installation-faq/README.md

@@ -124,7 +124,7 @@ Set a higher number of web workers to allow more processes to be handled at the
 
 There are two different types of emails in OpenProject: One sent directly within the request to the server (this includes the test mail) and one sent asynchronously, via a background job from the backend. The majority of mail sending jobs is run asynchronously to facilitate a faster response time for server request.
 
-Use a browser to call your domain name followed by "health_checks/all" (e.g. `https://myopenproject.com/health_checks/all`). There should be entries about "delayed_jobs_backed_up" and "delayed_jobs_never_ran". If PASSED is written behind it, everything is good.
+Use a browser to call your domain name followed by "health_checks/all" (e.g. `https://myopenproject.com/health_checks/all`). There should be entries about "worker" and "worker_backed_up". If PASSED is written behind it, everything is good.
 
 If the health check does not return satisfying results, have a look if the background worker is running by entering `ps aux | grep jobs` on the server. If it is not running, no entry is returned. If it is running an entry with "jobs:work" at the end is displayed.
 

docs/installation-and-operations/operation/monitoring/README.md

@@ -124,8 +124,10 @@ We provide the following health checks:
 
 - `https://your-hostname.example.tld/health_checks/default` - An application level check to ensure the web workers are running.
 - `https://your-hostname.example.tld/health_checks/database` - A database liveliness check.
-- `https://your-hostname.example.tld/health_checks/delayed_jobs_never_ran` - A check to ensure background jobs are being processed.
-- `https://your-hostname.example.tld/health_checks/delayed_jobs_backed_up` - A check to determine whether background workers are at capacity and might need to be scaled up to provide timely processing of mails and other background work.
+- `https://your-hostname.example.tld/health_checks/mail` - SMTP configuration check.
+- `https://your-hostname.example.tld/health_checks/puma` - A check on Puma web server.
+- `https://your-hostname.example.tld/health_checks/worker` - A check to ensure background jobs are being processed.
+- `https://your-hostname.example.tld/health_checks/worker_backed_up` - A check to determine whether background workers are at capacity and might need to be scaled up to provide timely processing of mails and other background work.
 - `https://your-hostname.example.tld/health_checks/all` - All of the above checks and additional checks combined as one. Not recommended as the liveliness check of a pod/container.
 
 ### Optional authentication

docs/system-admin-guide/integrations/nextcloud/README.md

@@ -350,7 +350,7 @@ You have setup the *Project folder* in both environments (Nextcloud and OpenProj
    `ps aux | grep job`
    The result should show lines containing `bundle exec bundle exec good_job start`
 
-   b. If you don't have root access to the OpenProject server then you can check the following URL in your browser: `https://<your-openproject-server>/health_checks/all` (please insert the domain name of your OpenProject server). If your background workers are running, you should see a line like that `delayed_jobs_never_ran: PASSED All previous jobs have completed within the past 5 minutes`
+   b. If you don't have root access to the OpenProject server then you can check the following URL in your browser: `https://<your-openproject-server>/health_checks/all` (please insert the domain name of your OpenProject server). If your background workers are running, you should see a line like that `worker_backed_up: PASSED No jobs are waiting to be picked up.`
 
 2. Ensure that your project is setup correctly:
    1. In your browser navigate to the project for which you want the **Project folders** feature to be working.

docs/user-guide/work-packages/work-packages-faq/README.md

@@ -236,7 +236,7 @@ The following factors can have an impact on the duration of the export:
 - Number of columns in the export (less of an impact)
 
 To identify how many background jobs have run or are delayed, enter "/health_checks/full" after the URL (e.g. myopenprojectinstance.com/health_checks/full).
-This provides an overview of "delayed_jobs_never_ran" which shows the number of background jobs that could not get ran within the last 10 minutes. If there are multiple entries, this can indicate that the number of web workers should be increased.
+This provides an overview of "worker_backed_up" which shows the number of background jobs that could not get ran within the last 5 minutes. If there are multiple entries, this can indicate that the number of web workers should be increased.
 
 For a documentation of how to do this, please refer to [these instructions](../../../installation-and-operations/operation/control) (see section "Scaling the number of web workers").
 

modules/github_integration/lib/open_project/github_integration/engine.rb

@@ -87,9 +87,9 @@ class Engine < ::Rails::Engine
 
     add_cron_jobs do
       {
-        ClearOldPullRequestsJob: {
+        'Cron::ClearOldPullRequestsJob': {
           cron: '25 1 * * *', # runs at 1:25 nightly
-          class: 'ClearOldPullRequestsJob'
+          class: ::Cron::ClearOldPullRequestsJob.name
         }
       }
     end

modules/job_status/lib/open_project/job_status/engine.rb

@@ -50,7 +50,7 @@ class Engine < ::Rails::Engine
       {
         'JobStatus::Cron::ClearOldJobStatusJob': {
           cron: '15 4 * * *', # runs at 4:15 nightly
-          class: '::JobStatus::Cron::ClearOldJobStatusJob'
+          class: ::JobStatus::Cron::ClearOldJobStatusJob.name
         }
       }
     end

modules/ldap_groups/lib/open_project/ldap_groups/engine.rb

@@ -23,7 +23,7 @@ class Engine < ::Rails::Engine
       {
         'Ldap::SynchronizationJob': {
           cron: '30 23 * * *', # Run once per night at 11:30pm
-          class: 'Ldap::SynchronizationJob'
+          class: Ldap::SynchronizationJob.name
         }
       }
     end

modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb

@@ -66,8 +66,8 @@ def debounce
       def disable_cron_job_if_needed
         if ::Storages::ProjectStorage.active_automatically_managed.exists?
           GoodJob::Setting.cron_key_enable(CRON_JOB_KEY) unless GoodJob::Setting.cron_key_enabled?(CRON_JOB_KEY)
-        else
-          GoodJob::Setting.cron_key_disable(CRON_JOB_KEY) if GoodJob::Setting.cron_key_enabled?(CRON_JOB_KEY)
+        elsif GoodJob::Setting.cron_key_enabled?(CRON_JOB_KEY)
+          GoodJob::Setting.cron_key_disable(CRON_JOB_KEY)
         end
       end
 
@@ -80,11 +80,8 @@ def debounce_happend_in_current_thread_recently?
     end
 
     def perform
-      ::Storages::Storage
-        .automatic_management_enabled
-        .includes(:oauth_client)
-        .find_each do |storage|
-          result = service_for(storage).call(storage)
+      find_storages do |storage|
+        result = service_for(storage).call(storage)
         result.match(
           on_success: ->(_) { storage.mark_as_healthy },
           on_failure: ->(errors) { storage.mark_as_unhealthy(reason: errors.to_s) }
@@ -94,6 +91,13 @@ def perform
 
     private
 
+    def find_storages(&)
+      ::Storages::Storage
+        .automatic_management_enabled
+        .includes(:oauth_client)
+        .find_each(&)
+    end
+
     def service_for(storage)
       return NextcloudGroupFolderPropertiesSyncService if storage.provider_type_nextcloud?
       return OneDriveManagedFolderSyncService if storage.provider_type_one_drive?

modules/storages/lib/open_project/storages/engine.rb

@@ -287,12 +287,12 @@ def self.permissions
       {
         'Storages::CleanupUncontaineredFileLinksJob': {
           cron: "06 22 * * *",
-          class: "Storages::CleanupUncontaineredFileLinksJob"
+          class: ::Storages::CleanupUncontaineredFileLinksJob.name
         },
 
         'Storages::ManageNextcloudIntegrationJob': {
           cron: "*/5 * * * *",
-          class: "Storages::ManageNextcloudIntegrationJob"
+          class: ::Storages::ManageNextcloudIntegrationJob.name
         }
       }
     end

modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb

@@ -32,7 +32,7 @@
 require_module_spec_helper
 
 RSpec.describe Storages::CleanupUncontaineredFileLinksJob, type: :job do
-  describe '#perfrom' do
+  describe '#perform' do
     it 'removes uncontainered file_links which are old enough' do
       grace_period = 10
       allow(OpenProject::Configuration)

modules/webhooks/lib/open_project/webhooks/engine.rb

@@ -55,7 +55,7 @@ class Engine < ::Rails::Engine
       {
         CleanupWebhookLogsJob: {
           cron: '28 5 * * 7', # runs at 5:28 on Sunday
-          class: 'CleanupWebhookLogsJob'
+          class: ::CleanupWebhookLogsJob.name
         }
       }
     end

spec/features/notifications/reminder_mail_spec.rb

@@ -9,7 +9,7 @@
   let(:work_package) { create(:work_package, project:) }
   let(:watched_work_package) { create(:work_package, project:, watcher_users: [current_user]) }
   let(:involved_work_package) { create(:work_package, project:, assigned_to: current_user) }
-  # GoodJob::Job#scheduled_at is used for scheduling the reminder mails
+  # ApplicationJob#job_scheduled_at is used for scheduling the reminder mails
   # needs to be within a time frame eligible for sending out mails for the chose
   # time zone. For the time zone Hawaii (UTC-10) this means between 8:00:00 and 8:14:59 UTC.
   # The job is scheduled to run every 15 min so the scheduled_at will in production always move between the quarters of an hour.

spec/workers/notifications/schedule_reminder_mails_job_spec.rb

@@ -57,7 +57,7 @@
       it 'queries with the intended job execution time (which might have been missed due to high load)' do
         GoodJob.perform_inline
 
-        expect(User).to have_received(:having_reminder_mail_to_send).with(scheduled_job.good_job_scheduled_at)
+        expect(User).to have_received(:having_reminder_mail_to_send).with(scheduled_job.job_scheduled_at)
       end
     end
 
@@ -67,7 +67,7 @@
       before do
         GoodJob::Job
           .where(id: scheduled_job.job_id)
-          .update_all(scheduled_at: scheduled_job.good_job_scheduled_at - 3.hours)
+          .update_all(scheduled_at: scheduled_job.job_scheduled_at - 3.hours)
       end
 
       it_behaves_like 'schedules reminder mails'