From fd6a899b297ac81a32a2e4a57fa2be4d7073e9d7 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Fri, 30 Jun 2023 14:10:04 +0200 Subject: [PATCH 01/19] [#48717] Replace DelayedJob with GoodJob. https://community.openproject.org/work_packages/48717 --- Gemfile | 3 +- Gemfile.lock | 17 +- Procfile.dev | 2 +- .../settings/working_days_params_contract.rb | 5 +- app/seeders/root_seeder.rb | 6 +- app/workers/application_job.rb | 5 + .../attachments/cleanup_uncontainered_job.rb | 7 +- app/workers/concerns/scheduled_job.rb | 41 ----- app/workers/cron/clear_old_sessions_job.rb | 5 +- app/workers/cron/clear_tmp_cache_job.rb | 5 +- app/workers/cron/clear_uploaded_files_job.rb | 5 +- app/workers/cron/cron_job.rb | 76 --------- app/workers/ldap/synchronization_job.rb | 5 +- .../schedule_date_alerts_notifications_job.rb | 10 +- .../schedule_reminder_mails_job.rb | 13 +- app/workers/oauth/cleanup_job.rb | 5 +- app/workers/paper_trail_audits/cleanup_job.rb | 5 +- .../apply_working_days_change_job.rb | 1 - bin/check-worker-readiness | 2 +- bin/delayed_job | 5 - bin/setup_dev | 2 +- config/application.rb | 13 +- config/initializers/cronjobs.rb | 82 +++++++-- config/initializers/database_pool_size.rb | 28 +++ config/initializers/delayed_job_config.rb | 55 ------ config/initializers/time_with_zone_as_json.rb | 33 ---- config/routes.rb | 4 + .../20201125121949_remove_renamed_cron_job.rb | 38 ----- ...20220202140507_reorder_project_children.rb | 33 ---- ...4112533_remove_notification_cleanup_job.rb | 41 ----- ...6132134_bigint_primary_and_foreign_keys.rb | 1 - ...105073117_remove_renamed_date_alert_job.rb | 37 ---- db/migrate/20240123151246_create_good_jobs.rb | 40 +++++ ...20240123151247_create_good_job_settings.rb | 20 +++ ..._on_priority_created_at_when_unfinished.rb | 19 +++ .../20240123151249_create_good_job_batches.rb | 35 ++++ ...240123151250_create_good_job_executions.rb | 33 ++++ ...0123151251_create_good_jobs_error_event.rb | 16 ++ ..._good_job_cron_indexes_with_conditional.rb | 45 +++++ docker-compose.yml | 2 +- docker/prod/bimworker | 2 +- docker/prod/worker | 2 +- .../development-environment-docker/README.md | 7 - .../development-environment-osx/README.md | 8 +- .../development-environment-ubuntu/README.md | 8 +- .../integrations/nextcloud/README.md | 2 +- .../patches/delayed_job_adapter.rb | 48 ------ lib/open_project/patches/delivery_job.rb | 40 ----- lib/open_project/plugins/acts_as_op_engine.rb | 8 +- lib/tasks/cron.rake | 11 +- lib/tasks/delayed_job.rake | 43 ----- .../cron/clear_old_pull_requests_job.rb | 5 +- .../open_project/github_integration/engine.rb | 10 +- .../cron/clear_old_job_status_job.rb | 7 +- .../lib/open_project/job_status/engine.rb | 12 +- .../ldap_groups/synchronization_job.rb | 5 +- .../lib/open_project/ldap_groups/engine.rb | 9 +- .../cleanup_uncontainered_file_links_job.rb | 6 +- .../manage_nextcloud_integration_cron_job.rb | 47 ------ ...manage_nextcloud_integration_events_job.rb | 63 ------- .../manage_nextcloud_integration_job.rb | 104 ++++++++++++ .../manage_nextcloud_integration_job_mixin.rb | 66 -------- .../20231009135807_remove_renamed_cronjobs.rb | 36 ---- .../lib/open_project/storages/engine.rb | 47 +++--- ...eanup_uncontainered_file_links_job_spec.rb | 54 +++--- ...age_nextcloud_integration_cron_job_spec.rb | 154 ----------------- ...e_nextcloud_integration_events_job_spec.rb | 88 ---------- .../manage_nextcloud_integration_job_spec.rb | 159 ++++++++++++++++++ .../app/workers/cleanup_webhook_logs_job.rb | 5 +- .../lib/open_project/webhooks/engine.rb | 9 +- packaging/scripts/check | 2 +- packaging/scripts/worker | 2 +- .../working_days_params_contract_spec.rb | 12 +- spec/controllers/roles_controller_spec.rb | 2 +- spec/features/admin/working_days_spec.rb | 7 +- .../notifications/reminder_mail_spec.rb | 21 +-- spec/features/projects/destroy_spec.rb | 16 +- spec/lib/open_project/events_spec.rb | 27 ++- spec/seeders/setting_seeder_spec.rb | 5 - spec/workers/non_existing_job_class_spec.rb | 65 ------- ...dule_date_alerts_notifications_job_spec.rb | 70 ++++---- .../schedule_reminder_mails_job_spec.rb | 61 +++---- .../apply_working_days_change_job_spec.rb | 22 --- 83 files changed, 805 insertions(+), 1372 deletions(-) delete mode 100644 app/workers/concerns/scheduled_job.rb delete mode 100644 app/workers/cron/cron_job.rb delete mode 100755 bin/delayed_job delete mode 100644 config/initializers/delayed_job_config.rb delete mode 100644 config/initializers/time_with_zone_as_json.rb delete mode 100644 db/migrate/20201125121949_remove_renamed_cron_job.rb delete mode 100644 db/migrate/20220202140507_reorder_project_children.rb delete mode 100644 db/migrate/20220804112533_remove_notification_cleanup_job.rb delete mode 100644 db/migrate/20230105073117_remove_renamed_date_alert_job.rb create mode 100644 db/migrate/20240123151246_create_good_jobs.rb create mode 100644 db/migrate/20240123151247_create_good_job_settings.rb create mode 100644 db/migrate/20240123151248_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb create mode 100644 db/migrate/20240123151249_create_good_job_batches.rb create mode 100644 db/migrate/20240123151250_create_good_job_executions.rb create mode 100644 db/migrate/20240123151251_create_good_jobs_error_event.rb create mode 100644 db/migrate/20240123151252_recreate_good_job_cron_indexes_with_conditional.rb delete mode 100644 lib/open_project/patches/delayed_job_adapter.rb delete mode 100644 lib/open_project/patches/delivery_job.rb delete mode 100644 lib/tasks/delayed_job.rake delete mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb delete mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb create mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb delete mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_job_mixin.rb delete mode 100644 modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb delete mode 100644 modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb delete mode 100644 modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb create mode 100644 modules/storages/spec/workers/storages/manage_nextcloud_integration_job_spec.rb delete mode 100644 spec/workers/non_existing_job_class_spec.rb diff --git a/Gemfile b/Gemfile index 7c1e9678975..d1f0ac5216e 100644 --- a/Gemfile +++ b/Gemfile @@ -124,8 +124,7 @@ gem 'multi_json', '~> 1.15.0' gem 'oj', '~> 3.16.0' gem 'daemons' -gem 'delayed_cron_job', '~> 0.9.0' -gem 'delayed_job_active_record', '~> 4.1.5' +gem 'good_job' gem 'rack-protection', '~> 3.2.0' diff --git a/Gemfile.lock b/Gemfile.lock index 6b2dc23041d..29279b35a89 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -457,13 +457,6 @@ GEM deckar01-task_list (2.3.3) html-pipeline declarative (0.0.20) - delayed_cron_job (0.9.0) - fugit (>= 1.5) - delayed_job (4.1.11) - activesupport (>= 3.0, < 8.0) - delayed_job_active_record (4.1.8) - activerecord (>= 3.0, < 8.0) - delayed_job (>= 3.0, < 5) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.5.1) @@ -577,6 +570,13 @@ GEM i18n (>= 0.7) multi_json request_store (>= 1.0) + good_job (3.21.5) + activejob (>= 6.0.0) + activerecord (>= 6.0.0) + concurrent-ruby (>= 1.0.2) + fugit (>= 1.1) + railties (>= 6.0.0) + thor (>= 0.14.1) google-apis-core (0.13.0) addressable (~> 2.5, >= 2.5.1) googleauth (~> 1.9) @@ -1159,8 +1159,6 @@ DEPENDENCIES date_validator (~> 0.12.0) debug deckar01-task_list (~> 2.3.1) - delayed_cron_job (~> 0.9.0) - delayed_job_active_record (~> 4.1.5) disposable (~> 0.6.2) doorkeeper (~> 5.6.6) dotenv-rails @@ -1178,6 +1176,7 @@ DEPENDENCIES friendly_id (~> 5.5.0) fuubar (~> 2.5.0) gon (~> 6.4.0) + good_job google-apis-gmail_v1 googleauth grape (~> 2.0.0) diff --git a/Procfile.dev b/Procfile.dev index 60c25648b2d..26ad472210c 100644 --- a/Procfile.dev +++ b/Procfile.dev @@ -1,3 +1,3 @@ web: bundle exec rails server -p 3000 -b ${HOST:="127.0.0.1"} --environment ${RAILS_ENV:="development"} angular: npm run serve -worker: bundle exec rake jobs:work +worker: bundle exec good_job start diff --git a/app/contracts/settings/working_days_params_contract.rb b/app/contracts/settings/working_days_params_contract.rb index c19f63062f8..1c8f16c28c5 100644 --- a/app/contracts/settings/working_days_params_contract.rb +++ b/app/contracts/settings/working_days_params_contract.rb @@ -41,8 +41,11 @@ module Settings end end + # TODO: consider implementing using GoodJob concurrency control mechanisms def unique_job - if WorkPackages::ApplyWorkingDaysChangeJob.scheduled? + if GoodJob::Job + .where(finished_at: nil) + .exists?(job_class: 'WorkPackages::ApplyWorkingDaysChangeJob') errors.add :base, :previous_working_day_changes_unprocessed end end diff --git a/app/seeders/root_seeder.rb b/app/seeders/root_seeder.rb index 0712ee73008..0537e57076e 100644 --- a/app/seeders/root_seeder.rb +++ b/app/seeders/root_seeder.rb @@ -123,13 +123,13 @@ class RootSeeder < Seeder ActionMailer::Base.perform_deliveries = false # Avoid asynchronous DeliverWorkPackageCreatedJob - previous_delay_jobs = Delayed::Worker.delay_jobs - Delayed::Worker.delay_jobs = false + previous_execution_mode = Rails.configuration.good_job.execution_mode + Rails.configuration.good_job.execution_mode = :inline yield ensure ActionMailer::Base.perform_deliveries = previous_perform_deliveries - Delayed::Worker.delay_jobs = previous_delay_jobs + Rails.configuration.good_job.execution_mode = previous_execution_mode end def seed_basic_data diff --git a/app/workers/application_job.rb b/app/workers/application_job.rb index de6bccc0067..703cedbfe11 100644 --- a/app/workers/application_job.rb +++ b/app/workers/application_job.rb @@ -92,6 +92,11 @@ class ApplicationJob < ActiveJob::Base Setting.reload_mailer_settings! end + def good_job_scheduled_at + GoodJob::Job.where(id: job_id) + .pick(:scheduled_at) + end + private def prepare_job_context diff --git a/app/workers/attachments/cleanup_uncontainered_job.rb b/app/workers/attachments/cleanup_uncontainered_job.rb index 729fd37231b..5b0dc599ef5 100644 --- a/app/workers/attachments/cleanup_uncontainered_job.rb +++ b/app/workers/attachments/cleanup_uncontainered_job.rb @@ -26,12 +26,9 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Attachments::CleanupUncontaineredJob < Cron::CronJob +class Attachments::CleanupUncontaineredJob < ApplicationJob queue_with_priority :low - # runs at 10:03 pm - self.cron_expression = '03 22 * * *' - def perform Attachment .where(container: nil) @@ -50,7 +47,7 @@ class Attachments::CleanupUncontaineredJob < Cron::CronJob attachment_table = Attachment.arel_table attachment_table[:created_at] - .lteq(Time.now - OpenProject::Configuration.attachments_grace_period.minutes) + .lteq(Time.zone.now - OpenProject::Configuration.attachments_grace_period.minutes) .to_sql end end diff --git a/app/workers/concerns/scheduled_job.rb b/app/workers/concerns/scheduled_job.rb deleted file mode 100644 index 8c7516a31ec..00000000000 --- a/app/workers/concerns/scheduled_job.rb +++ /dev/null @@ -1,41 +0,0 @@ -# OpenProject is an open source project management software. -# Copyright (C) 2010-2022 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. - -module ScheduledJob - extend ActiveSupport::Concern - - class_methods do - ## - # Is there a job scheduled? - def scheduled? - delayed_job_query.exists? - end - - def delayed_job_query - Delayed::Job.where('handler LIKE ?', "%job_class: #{name}%") - end - end -end diff --git a/app/workers/cron/clear_old_sessions_job.rb b/app/workers/cron/clear_old_sessions_job.rb index e53494d626d..252b506d2bb 100644 --- a/app/workers/cron/clear_old_sessions_job.rb +++ b/app/workers/cron/clear_old_sessions_job.rb @@ -27,12 +27,9 @@ #++ module Cron - class ClearOldSessionsJob < CronJob + class ClearOldSessionsJob < ApplicationJob include ::RakeJob - # runs at 1:15 nightly - self.cron_expression = '15 1 * * *' - def perform super('db:sessions:expire', 7) end diff --git a/app/workers/cron/clear_tmp_cache_job.rb b/app/workers/cron/clear_tmp_cache_job.rb index 8ac9a6c2101..74cb6ce1bd5 100644 --- a/app/workers/cron/clear_tmp_cache_job.rb +++ b/app/workers/cron/clear_tmp_cache_job.rb @@ -27,12 +27,9 @@ #++ module Cron - class ClearTmpCacheJob < CronJob + class ClearTmpCacheJob < ApplicationJob include ::RakeJob - # runs at 02:45 sundays - self.cron_expression = '45 2 * * 7' - def perform super('tmp:cache:clear') end diff --git a/app/workers/cron/clear_uploaded_files_job.rb b/app/workers/cron/clear_uploaded_files_job.rb index 458615f98b0..e0733843e8f 100644 --- a/app/workers/cron/clear_uploaded_files_job.rb +++ b/app/workers/cron/clear_uploaded_files_job.rb @@ -27,12 +27,9 @@ #++ module Cron - class ClearUploadedFilesJob < CronJob + class ClearUploadedFilesJob < ApplicationJob include ::RakeJob - # Runs 23pm fridays - self.cron_expression = '0 23 * * 5' - def perform super('attachments:clear') end diff --git a/app/workers/cron/cron_job.rb b/app/workers/cron/cron_job.rb deleted file mode 100644 index a2afe9f260d..00000000000 --- a/app/workers/cron/cron_job.rb +++ /dev/null @@ -1,76 +0,0 @@ -#-- 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. -#++ - -module Cron - class CronJob < ApplicationJob - class_attribute :cron_expression - - # List of registered jobs, requires eager load in dev(!) - class_attribute :registered_jobs, default: [] - - include ScheduledJob - - class << self - ## - # Register new job class(es) - def register!(*job_classes) - Array(job_classes).each do |clz| - raise ArgumentError, "Needs to be subclass of ::Cron::CronJob" unless clz.ancestors.include?(self) - - registered_jobs << clz - end - end - - def schedule_registered_jobs! - registered_jobs.each do |job_class| - job_class.ensure_scheduled! - end - end - - ## - # Ensure the job is scheduled unless it is already - def ensure_scheduled! - # Ensure scheduled only once - return if scheduled? - - Rails.logger.info { "Scheduling #{name} recurrent background job." } - set(cron: cron_expression).perform_later - end - - ## - # Remove the scheduled job, if any - def remove - delayed_job&.destroy - end - - def delayed_job - delayed_job_query.first - end - end - end -end diff --git a/app/workers/ldap/synchronization_job.rb b/app/workers/ldap/synchronization_job.rb index e584179d931..a100d5b8789 100644 --- a/app/workers/ldap/synchronization_job.rb +++ b/app/workers/ldap/synchronization_job.rb @@ -27,10 +27,7 @@ #++ module Ldap - class SynchronizationJob < ::Cron::CronJob - # Run once per night at 11:30pm - self.cron_expression = '30 23 * * *' - + class SynchronizationJob < ApplicationJob def perform run_user_sync end diff --git a/app/workers/notifications/schedule_date_alerts_notifications_job.rb b/app/workers/notifications/schedule_date_alerts_notifications_job.rb index 43c5bd810bd..a351ec983fb 100644 --- a/app/workers/notifications/schedule_date_alerts_notifications_job.rb +++ b/app/workers/notifications/schedule_date_alerts_notifications_job.rb @@ -28,15 +28,11 @@ module Notifications # Creates date alert jobs for users whose local time is 1:00 am. - class ScheduleDateAlertsNotificationsJob < Cron::CronJob - # runs every quarter of an hour, so 00:00, 00:15,..., 15:30, 15:45, 16:00, ... - self.cron_expression = '*/15 * * * *' - + class ScheduleDateAlertsNotificationsJob < ApplicationJob def perform return unless EnterpriseToken.allows_to?(:date_alerts) - service = Service.new(times_from_scheduled_to_execution) - service.call + Service.new(times_from_scheduled_to_execution).call end # Returns times from scheduled execution time to current time in 15 minutes @@ -57,7 +53,7 @@ module Notifications end def scheduled_time - self.class.delayed_job.run_at.then { |t| t.change(min: t.min / 15 * 15) } + good_job_scheduled_at.then { |t| t.change(min: t.min / 15 * 15) } end end end diff --git a/app/workers/notifications/schedule_reminder_mails_job.rb b/app/workers/notifications/schedule_reminder_mails_job.rb index 0932594fbda..6c985161b75 100644 --- a/app/workers/notifications/schedule_reminder_mails_job.rb +++ b/app/workers/notifications/schedule_reminder_mails_job.rb @@ -27,18 +27,13 @@ #++ module Notifications - class ScheduleReminderMailsJob < Cron::CronJob - # runs every quarter of an hour, so 00:00, 00:15... - self.cron_expression = '*/15 * * * *' - + class ScheduleReminderMailsJob < ApplicationJob def perform - User.having_reminder_mail_to_send(run_at).pluck(:id).each do |user_id| + User.having_reminder_mail_to_send(good_job_scheduled_at) + .pluck(:id) + .each do |user_id| Mails::ReminderJob.perform_later(user_id) end end - - def run_at - self.class.delayed_job.run_at - end end end diff --git a/app/workers/oauth/cleanup_job.rb b/app/workers/oauth/cleanup_job.rb index 3295cb0e82b..8ada62fb0b1 100644 --- a/app/workers/oauth/cleanup_job.rb +++ b/app/workers/oauth/cleanup_job.rb @@ -27,12 +27,9 @@ #++ module OAuth - class CleanupJob < ::Cron::CronJob + class CleanupJob < ApplicationJob include ::RakeJob - # runs at 1:52 nightly - self.cron_expression = '52 1 * * *' - queue_with_priority :low def perform diff --git a/app/workers/paper_trail_audits/cleanup_job.rb b/app/workers/paper_trail_audits/cleanup_job.rb index 634f240c764..5d33de1a0be 100644 --- a/app/workers/paper_trail_audits/cleanup_job.rb +++ b/app/workers/paper_trail_audits/cleanup_job.rb @@ -27,10 +27,7 @@ #++ module PaperTrailAudits - class CleanupJob < ::Cron::CronJob - # runs at 4:03 on Saturday - self.cron_expression = '3 4 * * 6' - + class CleanupJob < ApplicationJob # Clean any paper trails older than 60 days def perform ::PaperTrailAudit diff --git a/app/workers/work_packages/apply_working_days_change_job.rb b/app/workers/work_packages/apply_working_days_change_job.rb index db79c3c4301..619acf4e907 100644 --- a/app/workers/work_packages/apply_working_days_change_job.rb +++ b/app/workers/work_packages/apply_working_days_change_job.rb @@ -28,7 +28,6 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob queue_with_priority :above_normal - include ::ScheduledJob def perform(user_id:, previous_working_days:, previous_non_working_days:) user = User.find(user_id) diff --git a/bin/check-worker-readiness b/bin/check-worker-readiness index 84fbd973fe4..1f9beb41ac3 100755 --- a/bin/check-worker-readiness +++ b/bin/check-worker-readiness @@ -1,6 +1,6 @@ #!/bin/bash -if ps aux | grep 'rake jobs:work' | grep -v grep; then +if ps aux | grep 'good_job start' | grep -v grep; then echo "background worker running" exit 0 fi diff --git a/bin/delayed_job b/bin/delayed_job deleted file mode 100755 index edf195985f6..00000000000 --- a/bin/delayed_job +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env ruby - -require File.expand_path(File.join(File.dirname(__FILE__), '..', 'config', 'environment')) -require 'delayed/command' -Delayed::Command.new(ARGV).daemonize diff --git a/bin/setup_dev b/bin/setup_dev index b69721264a9..7236581a7fc 100755 --- a/bin/setup_dev +++ b/bin/setup_dev @@ -30,7 +30,7 @@ echo "---------------------------------------" echo "Done. Now start the following services" echo '- Rails server `RAILS_ENV=development bin/rails server`' echo '- Angular CLI: `npm run serve`' -echo '- Delayed Job worker: `RAILS_ENV=development bin/rails jobs:work`' +echo '- Good Job worker: `RAILS_ENV=development bundle exec good_job start`' echo "" echo 'You can also run `bin/dev` to run all the above on a single terminal.' echo "" diff --git a/config/application.rb b/config/application.rb index 3c92d6857a3..2290c287a96 100644 --- a/config/application.rb +++ b/config/application.rb @@ -212,7 +212,18 @@ module OpenProject # This allows for setting the root either via config file or via environment variable. config.action_controller.relative_url_root = OpenProject::Configuration['rails_relative_url_root'] - config.active_job.queue_adapter = :delayed_job + 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) } + 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.smaller_number_is_higher_priority = false config.action_controller.asset_host = OpenProject::Configuration::AssetHost.value diff --git a/config/initializers/cronjobs.rb b/config/initializers/cronjobs.rb index 6dda0beb89e..ba22d94574e 100644 --- a/config/initializers/cronjobs.rb +++ b/config/initializers/cronjobs.rb @@ -1,15 +1,71 @@ -# Register "Cron-like jobs" +#-- 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. +#++ -Rails.application.configure do |application| - application.config.to_prepare do - Cron::CronJob.register! Cron::ClearOldSessionsJob, - Cron::ClearTmpCacheJob, - Cron::ClearUploadedFilesJob, - OAuth::CleanupJob, - PaperTrailAudits::CleanupJob, - Attachments::CleanupUncontaineredJob, - Notifications::ScheduleDateAlertsNotificationsJob, - Notifications::ScheduleReminderMailsJob, - Ldap::SynchronizationJob - end +# Register "Cron-like jobs" +OpenProject::Application.configure do |application| + application.config.good_job.cron.merge!( + { + 'Cron::ClearOldSessionsJob': { + cron: '15 1 * * *', # runs at 1:15 nightly + class: 'Cron::ClearOldSessionsJob' + }, + 'Cron::ClearTmpCacheJob': { + cron: '45 2 * * 7', # runs at 02:45 sundays + class: 'Cron::ClearTmpCacheJob' + }, + 'Cron::ClearUploadedFilesJob': { + cron: '0 23 * * 5', # runs 23:00 fridays + class: 'Cron::ClearUploadedFilesJob' + }, + 'OAuth::CleanupJob': { + cron: '52 1 * * *', + class: 'OAuth::CleanupJob' + }, + 'PaperTrailAudits::CleanupJob': { + cron: '3 4 * * 6', + class: 'PaperTrailAudits::CleanupJob' + }, + 'Attachments::CleanupUncontaineredJob': { + cron: '03 22 * * *', + class: 'Attachments::CleanupUncontaineredJob' + }, + 'Notifications::ScheduleDateAlertsNotificationsJob': { + cron: '*/15 * * * *', + class: 'Notifications::ScheduleDateAlertsNotificationsJob' + }, + 'Notifications::ScheduleReminderMailsJob': { + cron: '*/15 * * * *', + class: 'Notifications::ScheduleReminderMailsJob' + }, + 'Ldap::SynchronizationJob': { + cron: '30 23 * * *', + class: 'Ldap::SynchronizationJob' + } + } + ) end diff --git a/config/initializers/database_pool_size.rb b/config/initializers/database_pool_size.rb index 8809f2439de..ac707aeef26 100644 --- a/config/initializers/database_pool_size.rb +++ b/config/initializers/database_pool_size.rb @@ -1,3 +1,31 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 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. +#++ + config = Rails.env.production? && Rails.application.config.database_configuration[Rails.env] pool_size = config && [OpenProject::Configuration.web_max_threads + 1, config['pool'].to_i].max diff --git a/config/initializers/delayed_job_config.rb b/config/initializers/delayed_job_config.rb deleted file mode 100644 index 89c3af7505c..00000000000 --- a/config/initializers/delayed_job_config.rb +++ /dev/null @@ -1,55 +0,0 @@ -#-- 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. -#++ - -# Disable delayed_job's own logging as we have activejob -Delayed::Worker.logger = nil - -# By default bypass worker queue and execute asynchronous tasks at once -Delayed::Worker.delay_jobs = true - -# Prevent loading ApplicationJob during initialization -Rails.application.reloader.to_prepare do - # Set default priority (lower = higher priority) - # Example ordering, see ApplicationJob.priority_number - Delayed::Worker.default_priority = ApplicationJob.priority_number(:default) -end - -# Do not retry jobs from delayed_job -# instead use 'retry_on' activejob functionality -Delayed::Worker.max_attempts = 1 - -# Remember DJ id in the payload object -class Delayed::ProviderJobIdPlugin < Delayed::Plugin - callbacks do |lifecycle| - lifecycle.before(:invoke_job) do |job| - job.payload_object.job_data['provider_job_id'] = job.id if job.payload_object.respond_to?(:job_data) - end - end -end - -Delayed::Worker.plugins << Delayed::ProviderJobIdPlugin diff --git a/config/initializers/time_with_zone_as_json.rb b/config/initializers/time_with_zone_as_json.rb deleted file mode 100644 index 117f21ffaa4..00000000000 --- a/config/initializers/time_with_zone_as_json.rb +++ /dev/null @@ -1,33 +0,0 @@ -#-- 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 ActiveSupport::TimeWithZone - def as_json(_options = {}) - time.strftime('%m/%d/%Y/ %H:%M %p').to_s - end -end diff --git a/config/routes.rb b/config/routes.rb index 11de0c2a584..f90494edc42 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -639,4 +639,8 @@ Rails.application.routes.draw do if OpenProject::Configuration.lookbook_enabled? mount Lookbook::Engine, at: "/lookbook" end + + constraints(->(req) { User.exists?(id: req.session[:user_id], admin: true) }) do + mount GoodJob::Engine => 'good_job' + end end diff --git a/db/migrate/20201125121949_remove_renamed_cron_job.rb b/db/migrate/20201125121949_remove_renamed_cron_job.rb deleted file mode 100644 index c9d08ea8a7e..00000000000 --- a/db/migrate/20201125121949_remove_renamed_cron_job.rb +++ /dev/null @@ -1,38 +0,0 @@ -#-- 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 RemoveRenamedCronJob < ActiveRecord::Migration[6.0] - def up - # The job has been renamed to JobStatus::Cron::ClearOldJobStatusJob - # the new job will be added on restarting the application but the old will still be in the database - # and will cause 'uninitialized constant' errors. - Delayed::Job - .where('handler LIKE ?', "%job_class: Cron::ClearOldJobStatusJob%") - .delete_all - end -end diff --git a/db/migrate/20220202140507_reorder_project_children.rb b/db/migrate/20220202140507_reorder_project_children.rb deleted file mode 100644 index d964d0d077e..00000000000 --- a/db/migrate/20220202140507_reorder_project_children.rb +++ /dev/null @@ -1,33 +0,0 @@ -#-- 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 ReorderProjectChildren < ActiveRecord::Migration[6.1] - def up - ::Projects::ReorderHierarchyJob.perform_later - end -end diff --git a/db/migrate/20220804112533_remove_notification_cleanup_job.rb b/db/migrate/20220804112533_remove_notification_cleanup_job.rb deleted file mode 100644 index 956032ddad7..00000000000 --- a/db/migrate/20220804112533_remove_notification_cleanup_job.rb +++ /dev/null @@ -1,41 +0,0 @@ -#-- 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 RemoveNotificationCleanupJob < ActiveRecord::Migration[7.0] - def up - # Remove the cron job no longer desired. - # The code itself is removed but keeping it in the database would lead to UninitializedConstant errors. - Delayed::Job - .where('handler LIKE ?', "%job_class: Notifications::CleanupJob%") - .delete_all - - Setting - .where(name: 'notification_retention_period_days') - .delete_all - end -end diff --git a/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb b/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb index c641d892044..9a60233b59a 100644 --- a/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb +++ b/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb @@ -60,7 +60,6 @@ class BigintPrimaryAndForeignKeys < ActiveRecord::Migration[7.0] CustomStyle => [:id], CustomValue => %i[id customized_id custom_field_id], Journal::CustomizableJournal => %i[id journal_id custom_field_id], - Delayed::Job => [:id], DesignColor => [:id], Journal::DocumentJournal => %i[id project_id category_id], Document => %i[id project_id category_id], diff --git a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb deleted file mode 100644 index 9ae61bc1923..00000000000 --- a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb +++ /dev/null @@ -1,37 +0,0 @@ -#-- 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 RemoveRenamedDateAlertJob < ActiveRecord::Migration[6.0] - def up - # The job has been renamed to Notifications::ScheduleDateAlertsNotificationsJob. - # The new job will be added on restarting the application. - Delayed::Job - .where('handler LIKE ?', "%job_class: Notifications::CreateDateAlertsNotificationsJob%") - .delete_all - end -end diff --git a/db/migrate/20240123151246_create_good_jobs.rb b/db/migrate/20240123151246_create_good_jobs.rb new file mode 100644 index 00000000000..83cb7893fae --- /dev/null +++ b/db/migrate/20240123151246_create_good_jobs.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class CreateGoodJobs < ActiveRecord::Migration[7.0] + def change + # Uncomment for Postgres v12 or earlier to enable gen_random_uuid() support + # enable_extension 'pgcrypto' + + create_table :good_jobs, id: :uuid do |t| + t.text :queue_name + t.integer :priority + t.jsonb :serialized_params + t.datetime :scheduled_at + t.datetime :performed_at + t.datetime :finished_at + t.text :error + + t.timestamps + + t.uuid :active_job_id + t.text :concurrency_key + t.text :cron_key + t.uuid :retried_good_job_id + t.datetime :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 + add_index :good_jobs, [:queue_name, :scheduled_at], where: "(finished_at IS NULL)", name: :index_good_jobs_on_queue_name_and_scheduled_at + 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/db/migrate/20240123151247_create_good_job_settings.rb b/db/migrate/20240123151247_create_good_job_settings.rb new file mode 100644 index 00000000000..fa3540174a6 --- /dev/null +++ b/db/migrate/20240123151247_create_good_job_settings.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateGoodJobSettings < ActiveRecord::Migration[7.0] + 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_settings) + end + end + + create_table :good_job_settings, id: :uuid do |t| + t.timestamps + t.text :key + t.jsonb :value + t.index :key, unique: true + end + end +end diff --git a/db/migrate/20240123151248_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb b/db/migrate/20240123151248_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb new file mode 100644 index 00000000000..7b9dbab3816 --- /dev/null +++ b/db/migrate/20240123151248_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class CreateIndexGoodJobsJobsOnPriorityCreatedAtWhenUnfinished < ActiveRecord::Migration[7.0] + 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_jobs_on_priority_created_at_when_unfinished) + end + end + + add_index :good_jobs, [:priority, :created_at], order: { priority: "DESC NULLS LAST", created_at: :asc }, + where: "finished_at IS NULL", name: :index_good_jobs_jobs_on_priority_created_at_when_unfinished, + algorithm: :concurrently + end +end diff --git a/db/migrate/20240123151249_create_good_job_batches.rb b/db/migrate/20240123151249_create_good_job_batches.rb new file mode 100644 index 00000000000..ac6c5b37e98 --- /dev/null +++ b/db/migrate/20240123151249_create_good_job_batches.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class CreateGoodJobBatches < ActiveRecord::Migration[7.0] + 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_batches) + end + end + + create_table :good_job_batches, id: :uuid do |t| + t.timestamps + t.text :description + t.jsonb :serialized_properties + t.text :on_finish + t.text :on_success + t.text :on_discard + t.text :callback_queue_name + t.integer :callback_priority + t.datetime :enqueued_at + t.datetime :discarded_at + t.datetime :finished_at + end + + change_table :good_jobs do |t| + t.uuid :batch_id + t.uuid :batch_callback_id + + t.index :batch_id, where: "batch_id IS NOT NULL" + t.index :batch_callback_id, where: "batch_callback_id IS NOT NULL" + end + end +end diff --git a/db/migrate/20240123151250_create_good_job_executions.rb b/db/migrate/20240123151250_create_good_job_executions.rb new file mode 100644 index 00000000000..32723220cce --- /dev/null +++ b/db/migrate/20240123151250_create_good_job_executions.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class CreateGoodJobExecutions < ActiveRecord::Migration[7.0] + 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_executions) + end + end + + create_table :good_job_executions, id: :uuid do |t| + t.timestamps + + t.uuid :active_job_id, null: false + t.text :job_class + t.text :queue_name + t.jsonb :serialized_params + t.datetime :scheduled_at + t.datetime :finished_at + t.text :error + + t.index [:active_job_id, :created_at], name: :index_good_job_executions_on_active_job_id_and_created_at + end + + change_table :good_jobs do |t| + t.boolean :is_discrete + t.integer :executions_count + t.text :job_class + end + end +end diff --git a/db/migrate/20240123151251_create_good_jobs_error_event.rb b/db/migrate/20240123151251_create_good_jobs_error_event.rb new file mode 100644 index 00000000000..b07e0f14e7f --- /dev/null +++ b/db/migrate/20240123151251_create_good_jobs_error_event.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class CreateGoodJobsErrorEvent < ActiveRecord::Migration[7.0] + 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, :error_event) + end + end + + add_column :good_jobs, :error_event, :integer, limit: 2 + add_column :good_job_executions, :error_event, :integer, limit: 2 + end +end diff --git a/db/migrate/20240123151252_recreate_good_job_cron_indexes_with_conditional.rb b/db/migrate/20240123151252_recreate_good_job_cron_indexes_with_conditional.rb new file mode 100644 index 00000000000..aff2d4eae9a --- /dev/null +++ b/db/migrate/20240123151252_recreate_good_job_cron_indexes_with_conditional.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +class RecreateGoodJobCronIndexesWithConditional < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def change + reversible do |dir| + dir.up do + unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at_cond) + add_index :good_jobs, [:cron_key, :created_at], where: "(cron_key IS NOT NULL)", + name: :index_good_jobs_on_cron_key_and_created_at_cond, algorithm: :concurrently + end + unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at_cond) + add_index :good_jobs, [:cron_key, :cron_at], where: "(cron_key IS NOT NULL)", unique: true, + name: :index_good_jobs_on_cron_key_and_cron_at_cond, algorithm: :concurrently + end + + if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at) + remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_created_at + end + if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at) + remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_cron_at + end + end + + dir.down do + unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at) + add_index :good_jobs, [:cron_key, :created_at], + name: :index_good_jobs_on_cron_key_and_created_at, algorithm: :concurrently + end + unless connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at) + add_index :good_jobs, [:cron_key, :cron_at], unique: true, + name: :index_good_jobs_on_cron_key_and_cron_at, algorithm: :concurrently + end + + if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_created_at_cond) + remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_created_at_cond + end + if connection.index_name_exists?(:good_jobs, :index_good_jobs_on_cron_key_and_cron_at_cond) + remove_index :good_jobs, name: :index_good_jobs_on_cron_key_and_cron_at_cond + end + end + end + end +end diff --git a/docker-compose.yml b/docker-compose.yml index 5c77c0aa4a3..9e6ccbfdd84 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -68,7 +68,7 @@ services: worker: <<: *backend - command: bundle exec rake jobs:work + command: bundle exec good_job start depends_on: - db - cache diff --git a/docker/prod/bimworker b/docker/prod/bimworker index a63e2777dbc..01b7766a1b5 100755 --- a/docker/prod/bimworker +++ b/docker/prod/bimworker @@ -1,3 +1,3 @@ #!/bin/bash -e export QUEUE=bim,ifc_conversion -exec bundle exec rake jobs:work +exec bundle exec good_job start diff --git a/docker/prod/worker b/docker/prod/worker index bc4d194cbaa..87130302b19 100755 --- a/docker/prod/worker +++ b/docker/prod/worker @@ -5,4 +5,4 @@ if [ "$1" = "--seed" ]; then $APP_PATH/docker/prod/seeder "$@" fi -QUIET=true bundle exec rake jobs:work +QUIET=true bundle exec good_job start diff --git a/docs/development/development-environment-docker/README.md b/docs/development/development-environment-docker/README.md index 29e67061012..960da09ae60 100644 --- a/docs/development/development-environment-docker/README.md +++ b/docs/development/development-environment-docker/README.md @@ -141,15 +141,8 @@ system's resources. ```shell # Start the worker service and let it run continuously docker compose up -d worker - -# Start the worker service to work off all delayed jobs and shut it down afterwards -docker compose run --rm worker rake jobs:workoff ``` -The testing containers are excluded as well, while they are harmless to start, but take up system resources again and -clog your logs while running. The delayed_job background worker reloads the application for every job in development -mode. This is a know issue and documented here: https://github.com/collectiveidea/delayed_job/issues/823 - This process can take quite a long time on the first run where all gems are installed for the first time. However, these are cached in a docker volume. Meaning that from the 2nd run onwards it will start a lot quicker. diff --git a/docs/development/development-environment-osx/README.md b/docs/development/development-environment-osx/README.md index c484e6ea84f..8a6e2a77c9b 100644 --- a/docs/development/development-environment-osx/README.md +++ b/docs/development/development-environment-osx/README.md @@ -250,7 +250,7 @@ You can then access the application either through `localhost:3000` (Rails serve ### Delayed Job background worker ```shell -RAILS_ENV=development bin/rails jobs:work +RAILS_ENV=development bundle exec good_job start ``` This will start a Delayed::Job worker to perform asynchronous jobs like sending emails. @@ -298,12 +298,6 @@ brew install git ## Known issues -### Memory management - -The delayed_job background worker reloads the application for every job in development mode. This is a know issue and documented here: https://github.com/collectiveidea/delayed_job/issues/823 - - - ### Spawning a lot of browser tabs If you haven't run this command for a while, chances are that a lot of background jobs have queued up and might cause a significant amount of open tabs (due to the way we deliver mails with the letter_opener gem). To get rid of the jobs before starting the worker, use the following command. **This will remove all currently scheduled jobs, never use this in a production setting.** diff --git a/docs/development/development-environment-ubuntu/README.md b/docs/development/development-environment-ubuntu/README.md index 25c551a6adb..52ed199ef6e 100644 --- a/docs/development/development-environment-ubuntu/README.md +++ b/docs/development/development-environment-ubuntu/README.md @@ -301,7 +301,7 @@ You can then access the application either through `localhost:3000` (Rails serve ### Background job worker ```shell -RAILS_ENV=development bin/rails jobs:work +RAILS_ENV=development bundle exec good_job start ``` This will start a Delayed::Job worker to perform asynchronous jobs like sending emails. @@ -310,12 +310,6 @@ This will start a Delayed::Job worker to perform asynchronous jobs like sending ## Known issues -### Memory management - -The delayed_job background worker reloads the application for every job in development mode. This is a know issue and documented here: https://github.com/collectiveidea/delayed_job/issues/823 - - - ### Spawning a lot of browser tabs If you haven't run this command for a while, chances are that a lot of background jobs have queued up and might cause a significant amount of open tabs (due to the way we deliver mails with the letter_opener gem). To get rid of the jobs before starting the worker, use the following command. **This will remove all currently scheduled jobs, never use this in a production setting.** diff --git a/docs/system-admin-guide/integrations/nextcloud/README.md b/docs/system-admin-guide/integrations/nextcloud/README.md index 1fc9d5a245a..f4787b9eac9 100644 --- a/docs/system-admin-guide/integrations/nextcloud/README.md +++ b/docs/system-admin-guide/integrations/nextcloud/README.md @@ -348,7 +348,7 @@ You have setup the *Project folder* in both environments (Nextcloud and OpenProj a. If you have root access to the OpenProject server where your worker should be running, check if the worker processes are in fact present: `ps aux | grep job` - The result should show lines containing `bundle exec rake jobs:work` + 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:///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` diff --git a/lib/open_project/patches/delayed_job_adapter.rb b/lib/open_project/patches/delayed_job_adapter.rb deleted file mode 100644 index 1a3af2f472b..00000000000 --- a/lib/open_project/patches/delayed_job_adapter.rb +++ /dev/null @@ -1,48 +0,0 @@ -#-- 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. -#++ - -# This patch adds our job status extension to background jobs carried out when mailing with -# perform_later. - -module OpenProject - module Patches - module DelayedJobAdapter - module AllowNonExistingJobClass - def log_arguments? - super - rescue NameError - false - end - end - end - end -end - -ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper.prepend( - OpenProject::Patches::DelayedJobAdapter::AllowNonExistingJobClass -) diff --git a/lib/open_project/patches/delivery_job.rb b/lib/open_project/patches/delivery_job.rb deleted file mode 100644 index 99f29caa047..00000000000 --- a/lib/open_project/patches/delivery_job.rb +++ /dev/null @@ -1,40 +0,0 @@ -#-- 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. -#++ - -# This patch adds our job status extension to background jobs carried out when mailing with -# perform_later. - -module OpenProject - module Patches - module DeliveryJob - # include ::JobStatus::ApplicationJobWithStatus - end - end -end - -ActionMailer::MailDeliveryJob.include JobStatus::ApplicationJobWithStatus diff --git a/lib/open_project/plugins/acts_as_op_engine.rb b/lib/open_project/plugins/acts_as_op_engine.rb index b7549c164d3..473d88bac8c 100644 --- a/lib/open_project/plugins/acts_as_op_engine.rb +++ b/lib/open_project/plugins/acts_as_op_engine.rb @@ -293,13 +293,9 @@ module OpenProject::Plugins OpenProject::Activity.register(event_type, options) end - ## - # Register a "cron"-like background job - def add_cron_jobs + def add_cron_jobs(&block) config.to_prepare do - Array(yield).each do |clz| - ::Cron::CronJob.register!(clz.is_a?(Class) ? clz : clz.to_s.constantize) - end + Rails.application.config.good_job.cron.merge!(block.call) end end diff --git a/lib/tasks/cron.rake b/lib/tasks/cron.rake index 8f76dff9880..2d153f1df01 100644 --- a/lib/tasks/cron.rake +++ b/lib/tasks/cron.rake @@ -27,15 +27,8 @@ #++ namespace 'openproject:cron' do - desc 'An hourly cron job hook for plugin functionality' - task :hourly do - # Does nothing by default - end - - # This task will be automatically called when running jobs:work or jobs:workoff - # making sure cron jobs are scheduled. See lib/tasks/delayed_job.rake. - desc 'Ensure the cron-like background jobs are actively scheduled' + desc 'Ensure the cron-like background jobs are properly unscheduled if needed' task schedule: [:environment] do - Cron::CronJob.schedule_registered_jobs! + Storages::ManageNextcloudIntegrationJob.disable_cron_job_if_needed end end diff --git a/lib/tasks/delayed_job.rake b/lib/tasks/delayed_job.rake deleted file mode 100644 index 8ea28800017..00000000000 --- a/lib/tasks/delayed_job.rake +++ /dev/null @@ -1,43 +0,0 @@ -#-- 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. -#++ - -## -# Enhance the delayed_job prerequisites rake task to load the environment -unless Rake::Task.task_defined?('jobs:environment_options') && - Rake::Task['jobs:work'].prerequisites == %w(environment_options) - raise "Trying to load the full environment for delayed_job, but jobs:work seems to have changed." -end - -Rake::Task['jobs:environment_options'] - .clear_prerequisites - .enhance(['environment:full']) - -# Enhance delayed job workers to use cron -load 'lib/tasks/cron.rake' -Rake::Task["jobs:work"].enhance [:'openproject:cron:schedule'] -Rake::Task["jobs:workoff"].enhance [:'openproject:cron:schedule'] diff --git a/modules/github_integration/app/workers/cron/clear_old_pull_requests_job.rb b/modules/github_integration/app/workers/cron/clear_old_pull_requests_job.rb index f43df184e8e..242892c5e32 100644 --- a/modules/github_integration/app/workers/cron/clear_old_pull_requests_job.rb +++ b/modules/github_integration/app/workers/cron/clear_old_pull_requests_job.rb @@ -27,12 +27,9 @@ #++ module Cron - class ClearOldPullRequestsJob < CronJob + class ClearOldPullRequestsJob < ApplicationJob priority_number :low - # runs at 1:25 nightly - self.cron_expression = '25 1 * * *' - def perform GithubPullRequest.without_work_package .find_each(&:destroy!) diff --git a/modules/github_integration/lib/open_project/github_integration/engine.rb b/modules/github_integration/lib/open_project/github_integration/engine.rb index baede67326c..14cd9a03d6d 100644 --- a/modules/github_integration/lib/open_project/github_integration/engine.rb +++ b/modules/github_integration/lib/open_project/github_integration/engine.rb @@ -85,9 +85,13 @@ module OpenProject::GithubIntegration mount ::API::V3::GithubPullRequests::GithubPullRequestsAPI end - config.to_prepare do - # Register the cron job to clean up old github pull requests - ::Cron::CronJob.register! ::Cron::ClearOldPullRequestsJob + add_cron_jobs do + { + ClearOldPullRequestsJob: { + cron: '25 1 * * *', # runs at 1:25 nightly + class: 'ClearOldPullRequestsJob' + } + } end end end diff --git a/modules/job_status/app/workers/job_status/cron/clear_old_job_status_job.rb b/modules/job_status/app/workers/job_status/cron/clear_old_job_status_job.rb index 2b5e87b3d18..5069d9b39ea 100644 --- a/modules/job_status/app/workers/job_status/cron/clear_old_job_status_job.rb +++ b/modules/job_status/app/workers/job_status/cron/clear_old_job_status_job.rb @@ -28,15 +28,12 @@ module JobStatus module Cron - class ClearOldJobStatusJob < ::Cron::CronJob - # runs at 4:15 nightly - self.cron_expression = '15 4 * * *' - + class ClearOldJobStatusJob < ApplicationJob RETENTION_PERIOD = 2.days.freeze def perform ::JobStatus::Status - .where(::JobStatus::Status.arel_table[:updated_at].lteq(Time.now - RETENTION_PERIOD)) + .where(::JobStatus::Status.arel_table[:updated_at].lteq(Time.zone.now - RETENTION_PERIOD)) .destroy_all end end diff --git a/modules/job_status/lib/open_project/job_status/engine.rb b/modules/job_status/lib/open_project/job_status/engine.rb index 66b9fe7afb8..e5147bde388 100644 --- a/modules/job_status/lib/open_project/job_status/engine.rb +++ b/modules/job_status/lib/open_project/job_status/engine.rb @@ -46,10 +46,16 @@ module OpenProject::JobStatus "#{root}/job_statuses/#{uuid}" end - config.to_prepare do - # Register the cron job to clear statuses periodically - ::Cron::CronJob.register! ::JobStatus::Cron::ClearOldJobStatusJob + add_cron_jobs do + { + 'JobStatus::Cron::ClearOldJobStatusJob': { + cron: '15 4 * * *', # runs at 4:15 nightly + class: '::JobStatus::Cron::ClearOldJobStatusJob' + } + } + end + config.to_prepare do # Extends the ActiveJob adapter in use (DelayedJob) by a Status which lives # indenpendently from the job itself (which is deleted once successful or after max attempts). # That way, the result of a background job is available even after the original job is gone. diff --git a/modules/ldap_groups/app/workers/ldap_groups/synchronization_job.rb b/modules/ldap_groups/app/workers/ldap_groups/synchronization_job.rb index e2fdf252592..afde20b08a3 100644 --- a/modules/ldap_groups/app/workers/ldap_groups/synchronization_job.rb +++ b/modules/ldap_groups/app/workers/ldap_groups/synchronization_job.rb @@ -27,10 +27,7 @@ #++ module LdapGroups - class SynchronizationJob < ::Cron::CronJob - # Run every 30 minutes - self.cron_expression = '*/30 * * * *' - + class SynchronizationJob < ApplicationJob def perform return unless EnterpriseToken.allows_to?(:ldap_groups) return if skipped? diff --git a/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb b/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb index e462c7464ea..6bb2faff47d 100644 --- a/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb +++ b/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb @@ -19,7 +19,14 @@ module OpenProject::LdapGroups enterprise_feature: 'ldap_groups' end - add_cron_jobs { LdapGroups::SynchronizationJob } + add_cron_jobs do + { + 'Ldap::SynchronizationJob': { + cron: '30 23 * * *', # Run once per night at 11:30pm + class: 'Ldap::SynchronizationJob' + } + } + end patches %i[LdapAuthSource Group User] end diff --git a/modules/storages/app/workers/storages/cleanup_uncontainered_file_links_job.rb b/modules/storages/app/workers/storages/cleanup_uncontainered_file_links_job.rb index 0f47e0d6ef7..cc5dbf1048c 100644 --- a/modules/storages/app/workers/storages/cleanup_uncontainered_file_links_job.rb +++ b/modules/storages/app/workers/storages/cleanup_uncontainered_file_links_job.rb @@ -26,15 +26,13 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Storages::CleanupUncontaineredFileLinksJob < Cron::CronJob +class Storages::CleanupUncontaineredFileLinksJob < ApplicationJob queue_with_priority :low - self.cron_expression = '06 22 * * *' - def perform Storages::FileLink .where(container: nil) - .where('created_at <= ?', Time.current - OpenProject::Configuration.attachments_grace_period.minutes) + .where("created_at <= ?", Time.current - OpenProject::Configuration.attachments_grace_period.minutes) .delete_all end end diff --git a/modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb deleted file mode 100644 index 0c9fdbdb87a..00000000000 --- a/modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -#-- 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. -#++ - -module Storages - class ManageNextcloudIntegrationCronJob < Cron::CronJob - include ManageNextcloudIntegrationJobMixin - - queue_with_priority :low - - self.cron_expression = '*/5 * * * *' - - def self.ensure_scheduled! - if ::Storages::ProjectStorage.active_automatically_managed.exists? - super - else - remove - end - end - end -end diff --git a/modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb deleted file mode 100644 index 89485d35f3c..00000000000 --- a/modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb +++ /dev/null @@ -1,63 +0,0 @@ -#-- 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. -#++ - -module Storages - class ManageNextcloudIntegrationEventsJob < ApplicationJob - include ManageNextcloudIntegrationJobMixin - - SINGLE_THREAD_DEBOUNCE_TIME = 4.seconds.freeze - MULTI_THREAD_DEBOUNCE_TIME = 5.seconds.freeze - KEY = :manage_nextcloud_integration_events_job_debounce_happend_at - - queue_with_priority :above_normal - - class << self - def debounce - unless debounce_happend_in_current_thread_recently? - Rails.cache.fetch(KEY, expires_in: MULTI_THREAD_DEBOUNCE_TIME) do - set(wait: MULTI_THREAD_DEBOUNCE_TIME).perform_later - RequestStore.store[KEY] = Time.current - end - end - end - - private - - def debounce_happend_in_current_thread_recently? - timestamp = RequestStore.store[KEY] - timestamp.present? && (timestamp + SINGLE_THREAD_DEBOUNCE_TIME) > Time.current - end - end - - def perform - lock_obtained = super - self.class.debounce unless lock_obtained - lock_obtained - end - end -end diff --git a/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb new file mode 100644 index 00000000000..f476544dbe4 --- /dev/null +++ b/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb @@ -0,0 +1,104 @@ +#-- 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. +#++ + +module Storages + class ManageNextcloudIntegrationJob < ApplicationJob + include GoodJob::ActiveJobExtensions::Concurrency + using ::Storages::Peripherals::ServiceResultRefinements + + good_job_control_concurrency_with( + total_limit: 2, + enqueue_limit: 1, + perform_limit: 1, + key: "ManageNextcloudIntegrationJob" + ) + SINGLE_THREAD_DEBOUNCE_TIME = 4.seconds.freeze + KEY = :manage_nextcloud_integration_job_debounce_happend_at + CRON_JOB_KEY = :'Storages::ManageNextcloudIntegrationJob' + + queue_with_priority :above_normal + + class << self + def debounce + if debounce_happend_in_current_thread_recently? + false + else + # TODO: + # Why there is 5 seconds delay? + # it is like that because for 1 thread and if there is no delay more than + # SINGLE_THREAD_DEBOUNCE_TIME(4.seconds) + # then some events can be lost + # + # Possibly "true" solutions are: + # 1. have after_request middleware to schedule one job after a request cycle + # 2. use concurrent ruby to have 'true' debounce. + result = set(wait: 5.seconds).perform_later + RequestStore.store[KEY] = Time.current + result + end + end + + def disable_cron_job_if_needed + if ::Storages::ProjectStorage.active_automatically_managed.exists? + GoodJob::Setting.cron_key_enable(CRON_JOB_KEY) + else + GoodJob::Setting.cron_key_disable(CRON_JOB_KEY) + end + end + + private + + def debounce_happend_in_current_thread_recently? + timestamp = RequestStore.store[KEY] + timestamp.present? && (timestamp + SINGLE_THREAD_DEBOUNCE_TIME) > Time.current + end + end + + def perform + ::Storages::Storage + .automatic_management_enabled + .includes(:oauth_client) + .find_each 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) } + ) + end + end + + private + + def service_for(storage) + return NextcloudGroupFolderPropertiesSyncService if storage.provider_type_nextcloud? + return OneDriveManagedFolderSyncService if storage.provider_type_one_drive? + + raise 'Unknown Storage' + end + end +end diff --git a/modules/storages/app/workers/storages/manage_nextcloud_integration_job_mixin.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_job_mixin.rb deleted file mode 100644 index 661930c1e6b..00000000000 --- a/modules/storages/app/workers/storages/manage_nextcloud_integration_job_mixin.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -#-- 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. -#++ - -module Storages - module ManageNextcloudIntegrationJobMixin - using Peripherals::ServiceResultRefinements - - def perform - OpenProject::Mutex.with_advisory_lock( - ::Storages::NextcloudStorage, - 'sync_all_group_folders', - timeout_seconds: 0, - transaction: false - ) do - ::Storages::Storage.automatic_management_enabled.includes(:oauth_client).find_each do |storage| - result = service_for(storage).call(storage) - result.match( - on_success: ->(_) do - storage.mark_as_healthy - end, - on_failure: ->(errors) do - storage.mark_as_unhealthy(reason: errors.to_s) - end - ) - end - true - end - end - - private - - def service_for(storage) - return NextcloudGroupFolderPropertiesSyncService if storage.provider_type_nextcloud? - return OneDriveManagedFolderSyncService if storage.provider_type_one_drive? - - raise 'Unknown Storage' - end - end -end diff --git a/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb b/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb deleted file mode 100644 index b4042f391e2..00000000000 --- a/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb +++ /dev/null @@ -1,36 +0,0 @@ -#-- 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 RemoveRenamedCronjobs < ActiveRecord::Migration[7.0] - def up - Delayed::Job.where("handler LIKE '%job_class: CleanupUncontaineredFileLinksJob%'").delete_all - Delayed::Job.where("handler LIKE '%job_class: ManageNextcloudIntegrationJob%'").delete_all - end - - def down; end -end diff --git a/modules/storages/lib/open_project/storages/engine.rb b/modules/storages/lib/open_project/storages/engine.rb index de36e6e7b23..6dc9488260d 100644 --- a/modules/storages/lib/open_project/storages/engine.rb +++ b/modules/storages/lib/open_project/storages/engine.rb @@ -46,11 +46,11 @@ module OpenProject::Storages # please see comments inside ActsAsOpEngine class include OpenProject::Plugins::ActsAsOpEngine - initializer 'openproject_storages.feature_decisions' do + initializer "openproject_storages.feature_decisions" do OpenProject::FeatureDecisions.add :storage_file_picking_select_all end - initializer 'openproject_storages.event_subscriptions' do + initializer "openproject_storages.event_subscriptions" do Rails.application.config.after_initialize do [ OpenProject::Events::MEMBER_CREATED, @@ -62,29 +62,29 @@ module OpenProject::Storages OpenProject::Events::PROJECT_UNARCHIVED ].each do |event| OpenProject::Notifications.subscribe(event) do |_payload| - ::Storages::ManageNextcloudIntegrationEventsJob.debounce + ::Storages::ManageNextcloudIntegrationJob.debounce end end OpenProject::Notifications.subscribe( OpenProject::Events::OAUTH_CLIENT_TOKEN_CREATED ) do |payload| - if payload[:integration_type] == 'Storages::Storage' - ::Storages::ManageNextcloudIntegrationEventsJob.debounce + if payload[:integration_type] == "Storages::Storage" + ::Storages::ManageNextcloudIntegrationJob.debounce end end OpenProject::Notifications.subscribe( OpenProject::Events::ROLE_UPDATED ) do |payload| if payload[:permissions_diff]&.intersect?(OpenProject::Storages::Engine.permissions) - ::Storages::ManageNextcloudIntegrationEventsJob.debounce + ::Storages::ManageNextcloudIntegrationJob.debounce end end OpenProject::Notifications.subscribe( OpenProject::Events::ROLE_DESTROYED ) do |payload| if payload[:permissions]&.intersect?(OpenProject::Storages::Engine.permissions) - ::Storages::ManageNextcloudIntegrationEventsJob.debounce + ::Storages::ManageNextcloudIntegrationJob.debounce end end @@ -95,8 +95,8 @@ module OpenProject::Storages ].each do |event| OpenProject::Notifications.subscribe(event) do |payload| if payload[:project_folder_mode] == :automatic - ::Storages::ManageNextcloudIntegrationEventsJob.debounce - ::Storages::ManageNextcloudIntegrationCronJob.ensure_scheduled! + ::Storages::ManageNextcloudIntegrationJob.debounce + ::Storages::ManageNextcloudIntegrationJob.disable_cron_job_if_needed end end end @@ -106,8 +106,8 @@ module OpenProject::Storages # For documentation see the definition of register in "ActsAsOpEngine" # This corresponds to the openproject-storage.gemspec # Pass a block to the plugin (for defining permissions, menu items and the like) - register 'openproject-storages', - author_url: 'https://www.openproject.org', + register "openproject-storages", + author_url: "https://www.openproject.org", bundled: true, settings: {} do # Defines permission constraints used in the module (controller, etc.) @@ -142,14 +142,14 @@ module OpenProject::Storages # condition ("if:"), caption and icon. menu :admin_menu, :storages_admin_settings, - { controller: '/storages/admin/storages', action: :index }, + { controller: "/storages/admin/storages", action: :index }, if: Proc.new { User.current.admin? }, caption: :project_module_storages, - icon: 'hosting' + icon: "hosting" menu :project_menu, :settings_project_storages, - { controller: '/storages/admin/project_storages', action: 'index' }, + { controller: "/storages/admin/project_storages", action: "index" }, caption: :project_module_storages, parent: :settings @@ -273,21 +273,28 @@ module OpenProject::Storages end # Add api endpoints specific to this module - add_api_endpoint 'API::V3::Root' do + add_api_endpoint "API::V3::Root" do mount ::API::V3::Storages::StoragesAPI mount ::API::V3::ProjectStorages::ProjectStoragesAPI mount ::API::V3::FileLinks::FileLinksAPI end - add_api_endpoint 'API::V3::WorkPackages::WorkPackagesAPI', :id do + add_api_endpoint "API::V3::WorkPackages::WorkPackagesAPI", :id do mount ::API::V3::FileLinks::WorkPackagesFileLinksAPI end add_cron_jobs do - [ - Storages::CleanupUncontaineredFileLinksJob, - Storages::ManageNextcloudIntegrationCronJob - ] + { + 'Storages::CleanupUncontaineredFileLinksJob': { + cron: "06 22 * * *", + class: "Storages::CleanupUncontaineredFileLinksJob" + }, + + 'Storages::ManageNextcloudIntegrationJob': { + cron: "*/5 * * * *", + class: "Storages::ManageNextcloudIntegrationJob" + } + } end end end diff --git a/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb b/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb index 7f6ee5ad25d..a5924b5a746 100644 --- a/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb +++ b/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb @@ -32,39 +32,37 @@ require 'spec_helper' require_module_spec_helper RSpec.describe Storages::CleanupUncontaineredFileLinksJob, type: :job do - it 'has a schedule set' do - expect(described_class.cron_expression).to eq('06 22 * * *') - end + describe '#perfrom' do + it 'removes uncontainered file_links which are old enough' do + grace_period = 10 + allow(OpenProject::Configuration) + .to receive(:attachments_grace_period) + .and_return(grace_period) - it 'removes uncontainered file_links which are old enough' do - grace_period = 10 - allow(OpenProject::Configuration) - .to receive(:attachments_grace_period) - .and_return(grace_period) + expect(Storages::FileLink.count).to eq(0) - expect(Storages::FileLink.count).to eq(0) - - uncontainered_old = create(:file_link, - container_id: nil, - container_type: nil, - created_at: Time.current - grace_period.minutes - 1.second) - uncontainered_young = create(:file_link, + uncontainered_old = create(:file_link, container_id: nil, - container_type: nil) - containered_old = create(:file_link, - container_id: 1, - created_at: Time.current - grace_period.minutes - 1.second) - containered_young = create(:file_link, - container_id: 1) + container_type: nil, + created_at: Time.current - grace_period.minutes - 1.second) + uncontainered_young = create(:file_link, + container_id: nil, + container_type: nil) + containered_old = create(:file_link, + container_id: 1, + created_at: Time.current - grace_period.minutes - 1.second) + containered_young = create(:file_link, + container_id: 1) - expect(Storages::FileLink.count).to eq(4) + expect(Storages::FileLink.count).to eq(4) - described_class.new.perform + described_class.new.perform - expect(Storages::FileLink.count).to eq(3) - file_link_ids = Storages::FileLink.pluck(:id).sort - expected_file_link_ids = [uncontainered_young.id, containered_old.id, containered_young.id].sort - expect(file_link_ids).to eq(expected_file_link_ids) - expect(file_link_ids).not_to include(uncontainered_old.id) + expect(Storages::FileLink.count).to eq(3) + file_link_ids = Storages::FileLink.pluck(:id).sort + expected_file_link_ids = [uncontainered_young.id, containered_old.id, containered_young.id].sort + expect(file_link_ids).to eq(expected_file_link_ids) + expect(file_link_ids).not_to include(uncontainered_old.id) + end end end diff --git a/modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb b/modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb deleted file mode 100644 index 0548e9f4e2a..00000000000 --- a/modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb +++ /dev/null @@ -1,154 +0,0 @@ -# frozen_string_literal: true - -#-- 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. -#++ - -require 'spec_helper' -require_module_spec_helper - -RSpec.describe Storages::ManageNextcloudIntegrationCronJob, :webmock, type: :job do - it 'has a schedule set' do - expect(described_class.cron_expression).to eq('*/5 * * * *') - end - - describe '.ensure_scheduled!' do - before { ActiveJob::Base.disable_test_adapter } - - subject { described_class.ensure_scheduled! } - - context 'when there is active nextcloud project storage' do - shared_let(:storage1) { create(:nextcloud_storage, :as_automatically_managed) } - shared_let(:project_storage) { create(:project_storage, :as_automatically_managed, storage: storage1) } - - it 'schedules cron_job if not scheduled' do - expect(described_class.scheduled?).to be(false) - expect(described_class.delayed_job_query.count).to eq(0) - - subject - - expect(described_class.scheduled?).to be(true) - expect(described_class.delayed_job_query.count).to eq(1) - end - - it 'does not schedules cron_job if already scheduled' do - described_class.ensure_scheduled! - expect(described_class.scheduled?).to be(true) - expect(described_class.delayed_job_query.count).to eq(1) - - subject - - expect(described_class.scheduled?).to be(true) - expect(described_class.delayed_job_query.count).to eq(1) - end - end - - context 'when there is no active nextcloud project storage' do - it 'does nothing but removes cron_job' do - described_class.set(cron: described_class.cron_expression).perform_later - expect(described_class.scheduled?).to be(true) - expect(described_class.delayed_job_query.count).to eq(1) - - subject - - expect(described_class.scheduled?).to be(false) - expect(described_class.delayed_job_query.count).to eq(0) - end - end - end - - describe '.perform' do - subject { described_class.new.perform } - - context 'when lock is free' do - it 'responds with true' do - expect(subject).to be(true) - end - - it 'calls GroupFolderPropertiesSyncService for each automatically managed storage' do - storage1 = create(:nextcloud_storage, :as_automatically_managed) - storage2 = create(:nextcloud_storage, :as_not_automatically_managed) - - allow(Storages::NextcloudGroupFolderPropertiesSyncService) - .to receive(:call).with(storage1).and_return(ServiceResult.success) - - expect(subject).to be(true) - - expect(Storages::NextcloudGroupFolderPropertiesSyncService).to have_received(:call).with(storage1).once - expect(Storages::NextcloudGroupFolderPropertiesSyncService).not_to have_received(:call).with(storage2) - end - - it 'marks storage as healthy if sync was successful' do - storage1 = create(:nextcloud_storage, :as_automatically_managed) - - allow(Storages::NextcloudGroupFolderPropertiesSyncService) - .to receive(:call).with(storage1).and_return(ServiceResult.success) - - Timecop.freeze('2023-03-14T15:17:00Z') do - expect do - subject - storage1.reload - end.to( - change(storage1, :health_changed_at).to(Time.now.utc) - .and(change(storage1, :health_status).from('pending').to('healthy')) - ) - end - end - - it 'marks storage as unhealthy if sync was unsuccessful' do - storage1 = create(:nextcloud_storage, :as_automatically_managed) - - allow(Storages::NextcloudGroupFolderPropertiesSyncService) - .to receive(:call).with(storage1).and_return(ServiceResult.failure(errors: Storages::StorageError.new(code: :not_found))) - - Timecop.freeze('2023-03-14T15:17:00Z') do - expect do - subject - storage1.reload - end.to( - change(storage1, :health_changed_at).to(Time.now.utc) - .and(change(storage1, :health_status).from('pending').to('unhealthy')) - .and(change(storage1, :health_reason).from(nil).to('not_found')) - ) - end - end - end - - context 'when lock is unfree' do - it 'responds with false' do - allow(ApplicationRecord).to receive(:with_advisory_lock).and_return(false) - - expect(subject).to be(false) - expect(ApplicationRecord).to have_received(:with_advisory_lock).with( - 'sync_all_group_folders', - timeout_seconds: 0, - transaction: false - ).once - end - end - end -end diff --git a/modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb b/modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb deleted file mode 100644 index 4a200133426..00000000000 --- a/modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# frozen_string_literal: true - -#-- 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. -#++ - -require 'spec_helper' -require_module_spec_helper - -RSpec.describe Storages::ManageNextcloudIntegrationEventsJob, type: :job do - describe '.priority' do - it 'has a maximum priority' do - expect(described_class.priority).to eq(7) - end - end - - describe '.debounce' do - context 'when has been debounced by other thread' do - before do - Rails.cache.write(described_class::KEY, Time.current) - end - - it 'does nothing' do - expect { described_class.debounce }.not_to change(enqueued_jobs, :count) - end - end - - context 'when has not been debounced by other thread' do - it 'schedules a job' do - expect { described_class.debounce }.to change(enqueued_jobs, :count).from(0).to(1) - end - - it 'hits cache once when called 1000 times in a short period of time' do - allow(Rails.cache).to receive(:fetch).and_call_original - - expect do - 1000.times { described_class.debounce } - end.to change(enqueued_jobs, :count).from(0).to(1) - - expect(Rails.cache).to have_received(:fetch).once - end - end - end - - describe '#perform' do - it 'responds with true when parent perform responds with true' do - allow(OpenProject::Mutex).to receive(:with_advisory_lock).and_return(true) - allow(described_class).to receive(:debounce) - - expect(described_class.new.perform).to be(true) - - expect(described_class).not_to have_received(:debounce) - end - - it 'debounces itself when parent perform responds with false' do - allow(OpenProject::Mutex).to receive(:with_advisory_lock).and_return(false) - allow(described_class).to receive(:debounce) - - expect(described_class.new.perform).to be(false) - - expect(described_class).to have_received(:debounce).once - end - end -end diff --git a/modules/storages/spec/workers/storages/manage_nextcloud_integration_job_spec.rb b/modules/storages/spec/workers/storages/manage_nextcloud_integration_job_spec.rb new file mode 100644 index 00000000000..4e47dbb821d --- /dev/null +++ b/modules/storages/spec/workers/storages/manage_nextcloud_integration_job_spec.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true + +#-- 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. +#++ + +require 'spec_helper' +require_module_spec_helper + +RSpec.describe Storages::ManageNextcloudIntegrationJob, :webmock, type: :job do + describe '.debounce' do + context 'when has been debounced by other thread' do + before { ActiveJob::Base.disable_test_adapter } + + it 'does not change the number of enqueued jobs' do + expect(GoodJob::Job.count).to eq(0) + expect(described_class.perform_later.successfully_enqueued?).to be(true) + expect(described_class.perform_later).to be(false) + expect(GoodJob::Job.count).to eq(1) + + expect { described_class.debounce }.not_to change(GoodJob::Job, :count) + end + end + + context 'when has not been debounced by other thread' do + it 'schedules a job' do + expect { described_class.debounce }.to change(enqueued_jobs, :count).from(0).to(1) + end + + it 'tries to schedule once when called 1000 times in a short period of time' do + expect_any_instance_of(ActiveJob::ConfiguredJob) + .to receive(:perform_later).once.and_call_original + + expect do + 1000.times { described_class.debounce } + end.to change(enqueued_jobs, :count).from(0).to(1) + end + end + end + + describe '.disable_cron_job_if_needed' do + before { ActiveJob::Base.disable_test_adapter } + + subject { described_class.disable_cron_job_if_needed } + + context 'when there is an active nextcloud project storage' do + shared_let(:storage1) { create(:nextcloud_storage, :as_automatically_managed) } + shared_let(:project_storage) { create(:project_storage, :as_automatically_managed, storage: storage1) } + + it 'enables the cron_job if was disabled before' do + GoodJob::Setting.cron_key_disable(described_class::CRON_JOB_KEY) + + good_job_setting = GoodJob::Setting.first + expect(good_job_setting.key).to eq("cron_keys_disabled") + expect(good_job_setting.value).to eq(["Storages::ManageNextcloudIntegrationJob"]) + + expect { subject }.not_to change(GoodJob::Setting, :count).from(1) + + good_job_setting.reload + expect(good_job_setting.key).to eq("cron_keys_disabled") + expect(good_job_setting.value).to eq([]) + end + + it 'does nothing if the cron_job is not disabled' do + expect(GoodJob::Setting.cron_key_enabled?(described_class::CRON_JOB_KEY)).to be(true) + + expect { subject }.not_to change(GoodJob::Setting, :count).from(0) + + expect(GoodJob::Setting.cron_key_enabled?(described_class::CRON_JOB_KEY)).to be(true) + end + end + + context 'when there is no active nextcloud project storage' do + it 'disables the cron job' do + expect { subject }.to change(GoodJob::Setting, :count).from(0).to(1) + + good_job_setting = GoodJob::Setting.first + expect(good_job_setting.key).to eq("cron_keys_disabled") + expect(good_job_setting.value).to eq(["Storages::ManageNextcloudIntegrationJob"]) + end + end + end + + describe '.perform' do + subject { described_class.new.perform } + + it 'calls NextcloudGroupFolderPropertiesSyncService for each automatically managed storage' do + storage1 = create(:nextcloud_storage, :as_automatically_managed) + storage2 = create(:nextcloud_storage, :as_not_automatically_managed) + + allow(Storages::NextcloudGroupFolderPropertiesSyncService) + .to receive(:call).with(storage1).and_return(ServiceResult.success) + + subject + + expect(Storages::NextcloudGroupFolderPropertiesSyncService).to have_received(:call).with(storage1).once + expect(Storages::NextcloudGroupFolderPropertiesSyncService).not_to have_received(:call).with(storage2) + end + + it 'marks storage as healthy if sync was successful' do + storage1 = create(:nextcloud_storage, :as_automatically_managed) + + allow(Storages::NextcloudGroupFolderPropertiesSyncService) + .to receive(:call).with(storage1).and_return(ServiceResult.success) + + Timecop.freeze('2023-03-14T15:17:00Z') do + expect do + subject + storage1.reload + end.to( + change(storage1, :health_changed_at).to(Time.now.utc) + .and(change(storage1, :health_status).from('pending').to('healthy')) + ) + end + end + + it 'marks storage as unhealthy if sync was unsuccessful' do + storage1 = create(:nextcloud_storage, :as_automatically_managed) + + allow(Storages::NextcloudGroupFolderPropertiesSyncService) + .to receive(:call).with(storage1).and_return(ServiceResult.failure(errors: Storages::StorageError.new(code: :not_found))) + + Timecop.freeze('2023-03-14T15:17:00Z') do + expect do + subject + storage1.reload + end.to( + change(storage1, :health_changed_at).to(Time.now.utc) + .and(change(storage1, :health_status).from('pending').to('unhealthy')) + .and(change(storage1, :health_reason).from(nil).to('not_found')) + ) + end + end + end +end diff --git a/modules/webhooks/app/workers/cleanup_webhook_logs_job.rb b/modules/webhooks/app/workers/cleanup_webhook_logs_job.rb index 4a59ae264ce..c3b31073710 100644 --- a/modules/webhooks/app/workers/cleanup_webhook_logs_job.rb +++ b/modules/webhooks/app/workers/cleanup_webhook_logs_job.rb @@ -26,10 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class CleanupWebhookLogsJob < Cron::CronJob - # runs at 5:28 on Sunday - self.cron_expression = '28 5 * * 7' - +class CleanupWebhookLogsJob < ApplicationJob # Clean any logs older than 7 days def perform ::Webhooks::Log diff --git a/modules/webhooks/lib/open_project/webhooks/engine.rb b/modules/webhooks/lib/open_project/webhooks/engine.rb index 5bd5cd54914..ead276ce770 100644 --- a/modules/webhooks/lib/open_project/webhooks/engine.rb +++ b/modules/webhooks/lib/open_project/webhooks/engine.rb @@ -51,6 +51,13 @@ module OpenProject::Webhooks end end - add_cron_jobs { CleanupWebhookLogsJob } + add_cron_jobs do + { + CleanupWebhookLogsJob: { + cron: '28 5 * * 7', # runs at 5:28 on Sunday + class: 'CleanupWebhookLogsJob' + } + } + end end end diff --git a/packaging/scripts/check b/packaging/scripts/check index bd98c43d9bd..5c3ae264771 100755 --- a/packaging/scripts/check +++ b/packaging/scripts/check @@ -29,7 +29,7 @@ else log_ko "openproject server is NOT running" fi -if ps -u "$APP_USER" -f | grep -q "rake jobs:work" ; then +if ps -u "$APP_USER" -f | grep -q "good_job start" ; then log_ok "openproject background job worker is running" else log_ko "openproject background job worker is NOT running" diff --git a/packaging/scripts/worker b/packaging/scripts/worker index ea691e75539..cb29dbeace3 100755 --- a/packaging/scripts/worker +++ b/packaging/scripts/worker @@ -1,3 +1,3 @@ #!/bin/bash -e -QUIET=true bundle exec rake jobs:work +QUIET=true bundle exec good_job start diff --git a/spec/contracts/settings/working_days_params_contract_spec.rb b/spec/contracts/settings/working_days_params_contract_spec.rb index 79b995423c6..f67f074e0a2 100644 --- a/spec/contracts/settings/working_days_params_contract_spec.rb +++ b/spec/contracts/settings/working_days_params_contract_spec.rb @@ -37,13 +37,6 @@ RSpec.describe Settings::WorkingDaysParamsContract do let(:contract) do described_class.new(setting, current_user, params:) end - let(:apply_job_scheduled) { false } - - before do - allow(WorkPackages::ApplyWorkingDaysChangeJob) - .to receive(:scheduled?) - .and_return(apply_job_scheduled) - end it_behaves_like 'contract is valid for active admins and invalid for regular users' @@ -55,7 +48,10 @@ RSpec.describe Settings::WorkingDaysParamsContract do context 'with an ApplyWorkingDaysChangeJob already existing' do let(:params) { { working_days: [1, 2, 3] } } - let(:apply_job_scheduled) { true } + before do + ActiveJob::Base.disable_test_adapter + WorkPackages::ApplyWorkingDaysChangeJob.perform_later + end include_examples 'contract is invalid', base: :previous_working_day_changes_unprocessed end diff --git a/spec/controllers/roles_controller_spec.rb b/spec/controllers/roles_controller_spec.rb index 74cb660e332..b0ed566a780 100644 --- a/spec/controllers/roles_controller_spec.rb +++ b/spec/controllers/roles_controller_spec.rb @@ -387,7 +387,7 @@ RSpec.describe RolesController do subject expect(enqueued_jobs.count).to eq(1) - expect(enqueued_jobs[0][:job]).to eq(Storages::ManageNextcloudIntegrationEventsJob) + expect(enqueued_jobs[0][:job]).to eq(Storages::ManageNextcloudIntegrationJob) expect(response).to redirect_to roles_path expect(Role.count).to eq(0) end diff --git a/spec/features/admin/working_days_spec.rb b/spec/features/admin/working_days_spec.rb index aa049ff7d01..5b7a0c33e86 100644 --- a/spec/features/admin/working_days_spec.rb +++ b/spec/features/admin/working_days_spec.rb @@ -163,11 +163,8 @@ RSpec.describe 'Working Days', :js, :with_cuprite do it 'shows an error when a previous change to the working days configuration isn\'t processed yet' do # Have a job already scheduled - # Attempting to set the job via simply using the UI would require to change the test setup of how - # delayed jobs are handled. - ActiveJob::QueueAdapters::DelayedJobAdapter - .new - .enqueue(WorkPackages::ApplyWorkingDaysChangeJob.new(user_id: 5)) + ActiveJob::Base.disable_test_adapter + WorkPackages::ApplyWorkingDaysChangeJob.perform_later(user_id: 5) uncheck 'Tuesday' click_on 'Apply changes' diff --git a/spec/features/notifications/reminder_mail_spec.rb b/spec/features/notifications/reminder_mail_spec.rb index 1ffc320e21a..680b6e36fe5 100644 --- a/spec/features/notifications/reminder_mail_spec.rb +++ b/spec/features/notifications/reminder_mail_spec.rb @@ -9,10 +9,10 @@ RSpec.describe "Reminder email sending", :js, :with_cuprite do 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) } - # The run_at time of the delayed job used for scheduling the reminder mails + # GoodJob::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 run_at will in production always move between the quarters of an hour. + # The job is scheduled to run every 15 min so the scheduled_at will in production always move between the quarters of an hour. # The current time can be way behind that. let(:current_utc_time) { ActiveSupport::TimeZone['Pacific/Honolulu'].parse("2021-09-30T08:34:10").utc } let(:job_run_at) { ActiveSupport::TimeZone['Pacific/Honolulu'].parse("2021-09-30T08:00:00").utc } @@ -57,13 +57,10 @@ RSpec.describe "Reminder email sending", :js, :with_cuprite do work_package involved_work_package - ActiveJob::Base.queue_adapter.enqueued_jobs.clear + ActiveJob::Base.disable_test_adapter - # There is no delayed_job associated when using the testing backend of ActiveJob - # so we have to mock it. - allow(Notifications::ScheduleReminderMailsJob) - .to receive(:delayed_job) - .and_return(instance_double(Delayed::Backend::ActiveRecord::Job, run_at: job_run_at)) + scheduled_job = Notifications::ScheduleReminderMailsJob.perform_later + GoodJob::Job.where(id: scheduled_job.job_id).update_all(scheduled_at: job_run_at) end it 'sends a digest mail based on the configuration', with_settings: { journal_aggregation_time_minutes: 0 } do @@ -88,13 +85,9 @@ RSpec.describe "Reminder email sending", :js, :with_cuprite do involved_work_package.save! end - # The Job is triggered by time so we mock it and the jobs started by it being triggered - Notifications::ScheduleReminderMailsJob.perform_later - 2.times { perform_enqueued_jobs } - - expect(ActionMailer::Base.deliveries.length) - .to be 1 + 2.times { GoodJob.perform_inline } + expect(ActionMailer::Base.deliveries.length).to be 1 expect(ActionMailer::Base.deliveries.first.subject) .to eql "OpenProject - 1 unread notification including a mention" end diff --git a/spec/features/projects/destroy_spec.rb b/spec/features/projects/destroy_spec.rb index ff5e8e45b30..ab8ad34473f 100644 --- a/spec/features/projects/destroy_spec.rb +++ b/spec/features/projects/destroy_spec.rb @@ -35,26 +35,17 @@ RSpec.describe 'Projects#destroy', :js, :with_cuprite do current_user { create(:admin) } - before do - # Disable background worker - allow(Delayed::Worker) - .to receive(:delay_jobs) - .and_return(false) - - project_page.visit! - end + before { project_page.visit! } it 'destroys the project' do # Confirm the deletion # Without confirmation, the button is disabled - expect(danger_zone) - .to be_disabled + expect(danger_zone).to be_disabled # With wrong confirmation, the button is disabled danger_zone.confirm_with("#{project.identifier}_wrong") - expect(danger_zone) - .to be_disabled + expect(danger_zone).to be_disabled # With correct confirmation, the button is enabled # and the project can be deleted @@ -63,6 +54,7 @@ RSpec.describe 'Projects#destroy', :js, :with_cuprite do danger_zone.danger_button.click expect(page).to have_css '.op-toast.-success', text: I18n.t('projects.delete.scheduled') + expect(project.reload).to eq(project) perform_enqueued_jobs diff --git a/spec/lib/open_project/events_spec.rb b/spec/lib/open_project/events_spec.rb index b56d87e5342..80713097a8b 100644 --- a/spec/lib/open_project/events_spec.rb +++ b/spec/lib/open_project/events_spec.rb @@ -37,7 +37,7 @@ RSpec.describe OpenProject::Events do end before do - allow(Storages::ManageNextcloudIntegrationEventsJob).to receive(:debounce) + allow(Storages::ManageNextcloudIntegrationJob).to receive(:debounce) end %w[ @@ -53,7 +53,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) end end @@ -62,16 +62,13 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) end it do - # Check for message being called instead of checking the job arrival to the queue - # because it is not simple adding to the queue in ::Storages::ManageNextcloudIntegrationCronJob.ensure_scheduled! - # With this method cron_job can be actually removed if not needed. - allow(Storages::ManageNextcloudIntegrationCronJob).to receive(:ensure_scheduled!) + allow(Storages::ManageNextcloudIntegrationJob).to receive(:disable_cron_job_if_needed) subject - expect(Storages::ManageNextcloudIntegrationCronJob).to have_received(:ensure_scheduled!) + expect(Storages::ManageNextcloudIntegrationJob).to have_received(:disable_cron_job_if_needed) end end end @@ -93,7 +90,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) end end end @@ -106,7 +103,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) end end @@ -115,7 +112,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) end end end @@ -128,7 +125,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) end end @@ -137,7 +134,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) end end end @@ -150,7 +147,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) end end @@ -159,7 +156,7 @@ RSpec.describe OpenProject::Events do it do subject - expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) end end end diff --git a/spec/seeders/setting_seeder_spec.rb b/spec/seeders/setting_seeder_spec.rb index f62a35adc8a..0dcc5775456 100644 --- a/spec/seeders/setting_seeder_spec.rb +++ b/spec/seeders/setting_seeder_spec.rb @@ -36,11 +36,6 @@ RSpec.describe BasicData::SettingSeeder do let(:new_project_role) { basic_seed_data.find_reference(:default_role_project_admin) } let(:closed_status) { basic_seed_data.find_reference(:default_status_closed) } - before do - allow(ActionMailer::Base).to receive(:perform_deliveries).and_return(false) - allow(Delayed::Worker).to receive(:delay_jobs).and_return(false) - end - it 'applies initial settings' do expect(setting_seeder).to be_applicable diff --git a/spec/workers/non_existing_job_class_spec.rb b/spec/workers/non_existing_job_class_spec.rb deleted file mode 100644 index d63caa8cbee..00000000000 --- a/spec/workers/non_existing_job_class_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -#-- 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. -#++ - -require 'spec_helper' - -RSpec.describe "NonExistingJobClass" do - let!(:job_with_non_existing_class) do - handler = <<~JOB.strip - --- !ruby/object:ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper - job_data: - job_class: WhichShallNotBeNamedJob - job_id: 8f72c3c9-a1e0-4e46-b0f2-b517288bb76c - provider_job_id: - queue_name: default - priority: 5 - arguments: - - 42 - executions: 0 - exception_executions: {} - locale: en - timezone: UTC - enqueued_at: '2022-12-05T09:41:39Z' - JOB - Delayed::Job.create(handler:) - end - - before do - # allow to inspect the job is marked as failed after failure in the test - allow(Delayed::Worker).to receive(:destroy_failed_jobs).and_return(false) - end - - it 'does not crash the worker when processed' do - expect { Delayed::Worker.new(exit_on_complete: true).start } - .not_to raise_error - - job_with_non_existing_class.reload - expect(job_with_non_existing_class.last_error).to include("uninitialized constant WhichShallNotBeNamedJob") - expect(job_with_non_existing_class.failed_at).to be_within(1).of(Time.current) - end -end diff --git a/spec/workers/notifications/schedule_date_alerts_notifications_job_spec.rb b/spec/workers/notifications/schedule_date_alerts_notifications_job_spec.rb index fe29cb48763..eb434f54937 100644 --- a/spec/workers/notifications/schedule_date_alerts_notifications_job_spec.rb +++ b/spec/workers/notifications/schedule_date_alerts_notifications_job_spec.rb @@ -32,42 +32,30 @@ RSpec.describe Notifications::ScheduleDateAlertsNotificationsJob, type: :job, wi include ActiveSupport::Testing::TimeHelpers shared_let(:project) { create(:project, name: 'main') } - # Paris and Berlin are both UTC+01:00 (CET) or UTC+02:00 (CEST) shared_let(:timezone_paris) { ActiveSupport::TimeZone['Europe/Paris'] } # Kathmandu is UTC+05:45 (no DST) shared_let(:timezone_kathmandu) { ActiveSupport::TimeZone['Asia/Kathmandu'] } - shared_let(:user_paris) do - create( - :user, - firstname: 'Paris', - preferences: { time_zone: timezone_paris.name } - ) + create(:user, + firstname: 'Paris', + preferences: { time_zone: timezone_paris.name }) end shared_let(:user_kathmandu) do - create( - :user, - firstname: 'Kathmandu', - preferences: { time_zone: timezone_kathmandu.name } - ) + create(:user, + firstname: 'Kathmandu', + preferences: { time_zone: timezone_kathmandu.name }) end - let(:schedule_job) do - described_class.ensure_scheduled! - described_class.delayed_job - end + let(:scheduled_job) { described_class.perform_later } before do - # We need to access the job as stored in the database to get at the run_at time persisted there - allow(ActiveJob::Base) - .to receive(:queue_adapter) - .and_return(ActiveJob::QueueAdapters::DelayedJobAdapter.new) - schedule_job + ActiveJob::Base.disable_test_adapter + scheduled_job end - def set_scheduled_time(run_at) - schedule_job.update_column(:run_at, run_at) + def set_scheduled_time(scheduled_at) + GoodJob::Job.where(id: scheduled_job.job_id).update_all(scheduled_at:) end # Converts "hh:mm" into { hour: h, min: m } @@ -82,28 +70,25 @@ RSpec.describe Notifications::ScheduleDateAlertsNotificationsJob, type: :job, wi def run_job(scheduled_at: '1:00', local_time: '1:04', timezone: timezone_paris) set_scheduled_time(timezone_time(scheduled_at, timezone)) travel_to(timezone_time(local_time, timezone)) do - schedule_job.reload.invoke_job + GoodJob.perform_inline + # scheduled_job.reload.invoke_job yield if block_given? end end - def deserialized_of_job(job) - deserializer_class = Class.new do - include(ActiveJob::Arguments) - end - - deserializer = deserializer_class.new - - deserializer.deserialize(job.payload_object.job_data).to_h + def deserialize_job(job) + deserializer_class = Class.new { include(ActiveJob::Arguments) } + deserializer_class.new + .deserialize(job.serialized_params) + .to_h end - def expect_job(job, klass, *arguments) - job_data = deserialized_of_job(job) - expect(job_data['job_class']) - .to eql klass - expect(job_data['arguments']) - .to match_array arguments + def expect_job(job, *arguments) + job_data = deserialize_job(job) + expect(job_data['job_class']).to eql(job.job_class) + expect(job_data['arguments']).to match_array arguments + expect(job_data['executions']).to eq 0 end shared_examples_for 'job execution creates date alerts creation job' do @@ -115,9 +100,12 @@ RSpec.describe Notifications::ScheduleDateAlertsNotificationsJob, type: :job, wi it 'creates the job for the user' do expect do run_job(timezone:, scheduled_at:, local_time:) do - expect_job(Delayed::Job.last, "Notifications::CreateDateAlertsNotificationsJob", user) + j = GoodJob::Job.where(job_class: "Notifications::CreateDateAlertsNotificationsJob") + .order(created_at: :desc) + .last + expect_job(j, user) end - end.to change(Delayed::Job, :count).by 1 + end.to change(GoodJob::Job, :count).by 1 end end @@ -129,7 +117,7 @@ RSpec.describe Notifications::ScheduleDateAlertsNotificationsJob, type: :job, wi it 'creates no job' do expect do run_job(timezone:, scheduled_at:, local_time:) - end.not_to change(Delayed::Job, :count) + end.not_to change(GoodJob::Job, :count) end end diff --git a/spec/workers/notifications/schedule_reminder_mails_job_spec.rb b/spec/workers/notifications/schedule_reminder_mails_job_spec.rb index 99538d4f7b8..94c629da2eb 100644 --- a/spec/workers/notifications/schedule_reminder_mails_job_spec.rb +++ b/spec/workers/notifications/schedule_reminder_mails_job_spec.rb @@ -29,69 +29,46 @@ require 'spec_helper' RSpec.describe Notifications::ScheduleReminderMailsJob, type: :job do - subject(:job) { scheduled_job.invoke_job } - - let(:scheduled_job) do - described_class.ensure_scheduled! - - Delayed::Job.first - end - + let(:scheduled_job) { described_class.perform_later } let(:ids) { [23, 42] } - let(:run_at) { scheduled_job.run_at } before do - # We need to access the job as stored in the database to get at the run_at time persisted there - allow(ActiveJob::Base) - .to receive(:queue_adapter) - .and_return(ActiveJob::QueueAdapters::DelayedJobAdapter.new) - - scheduled_job.update_column(:run_at, run_at) + # We need to access the job as stored in the database to get at the scheduled_at time persisted there + ActiveJob::Base.disable_test_adapter + scheduled_job scope = instance_double(ActiveRecord::Relation) - allow(User) - .to receive(:having_reminder_mail_to_send) - .and_return(scope) - - allow(scope) - .to receive(:pluck) - .with(:id) - .and_return(ids) + allow(User).to receive(:having_reminder_mail_to_send).and_return(scope) + allow(scope).to receive(:pluck).with(:id).and_return(ids) end describe '#perform' do shared_examples_for 'schedules reminder mails' do it 'schedules reminder jobs for every user with a reminder mails to be sent' do - expect { subject } - .to change(Delayed::Job, :count) - .by(2) + expect { GoodJob.perform_inline }.to change(GoodJob::Job, :count).by(2) - jobs = Delayed::Job.all.map do |job| - YAML.safe_load(job.handler, permitted_classes: [ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper]) - end - - reminder_jobs = jobs.select { |job| job.job_data['job_class'] == "Mails::ReminderJob" } - - expect(reminder_jobs[0].job_data['arguments']) - .to contain_exactly(23) - - expect(reminder_jobs[1].job_data['arguments']) - .to contain_exactly(42) + arguments_from_both_jobs = + GoodJob::Job.where(job_class: "Mails::ReminderJob") + .flat_map {|i| i.serialized_params["arguments"]} + .sort + expect(arguments_from_both_jobs).to eq(ids) end it 'queries with the intended job execution time (which might have been missed due to high load)' do - subject + GoodJob.perform_inline - expect(User) - .to have_received(:having_reminder_mail_to_send) - .with(run_at) + expect(User).to have_received(:having_reminder_mail_to_send).with(scheduled_job.good_job_scheduled_at) end end it_behaves_like 'schedules reminder mails' context 'with a job that missed some runs' do - let(:run_at) { scheduled_job.run_at - 3.hours } + before do + GoodJob::Job + .where(id: scheduled_job.job_id) + .update_all(scheduled_at: scheduled_job.good_job_scheduled_at - 3.hours) + end it_behaves_like 'schedules reminder mails' end diff --git a/spec/workers/work_packages/apply_working_days_change_job_spec.rb b/spec/workers/work_packages/apply_working_days_change_job_spec.rb index cfbf896d515..5726bf8b704 100644 --- a/spec/workers/work_packages/apply_working_days_change_job_spec.rb +++ b/spec/workers/work_packages/apply_working_days_change_job_spec.rb @@ -1811,26 +1811,4 @@ RSpec.describe WorkPackages::ApplyWorkingDaysChangeJob do end end end - - describe '.scheduled?' do - context 'with a job already scheduled in the DB' do - before do - ActiveJob::QueueAdapters::DelayedJobAdapter - .new - .enqueue(described_class.new(user_id: 5, previous_non_working_days: [1, 2])) - end - - it 'is true' do - expect(described_class) - .to be_scheduled - end - end - - context 'without a job already scheduled in the DB' do - it 'is false' do - expect(described_class) - .not_to be_scheduled - end - end - end end From 6b8e1dcbecdad4725ea3082045e8547581477c05 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Fri, 16 Feb 2024 10:11:09 +0100 Subject: [PATCH 02/19] Modify good_job_settings only if required. --- .../app/workers/storages/manage_nextcloud_integration_job.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb index f476544dbe4..b626405d23a 100644 --- a/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb +++ b/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb @@ -65,9 +65,9 @@ module Storages def disable_cron_job_if_needed if ::Storages::ProjectStorage.active_automatically_managed.exists? - GoodJob::Setting.cron_key_enable(CRON_JOB_KEY) + 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) + GoodJob::Setting.cron_key_disable(CRON_JOB_KEY) if GoodJob::Setting.cron_key_enabled?(CRON_JOB_KEY) end end From bddc471c589602afe38bf571b2a9ca719b76e4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 19 Feb 2024 21:03:27 +0100 Subject: [PATCH 03/19] Replace background check with good_job check --- config/constants/settings/definition.rb | 7 ---- config/initializers/health_checks.rb | 32 ++++++------------- .../lib/open_project/webhooks/engine.rb | 2 +- 3 files changed, 11 insertions(+), 30 deletions(-) diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index 0c278d7fdd3..faea1a21b54 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -475,13 +475,6 @@ module Settings format: :string, default: nil }, - # Maximum number of backed up jobs (that are not yet executed) - # before health check fails - health_checks_jobs_queue_count_threshold: { - description: 'Set threshold of backed up background jobs to fail health check', - format: :integer, - default: 50 - }, ## Maximum number of minutes that jobs have not yet run after their designated 'run_at' time health_checks_jobs_never_ran_minutes_ago: { description: 'Set threshold of outstanding background jobs to fail health check', diff --git a/config/initializers/health_checks.rb b/config/initializers/health_checks.rb index 8905f2d7378..dd7d2a42b66 100644 --- a/config/initializers/health_checks.rb +++ b/config/initializers/health_checks.rb @@ -1,20 +1,16 @@ require 'ok_computer/ok_computer_controller' -class DelayedJobNeverRanCheck < OkComputer::Check - attr_reader :threshold - - def initialize(minute_threshold) - @threshold = minute_threshold.to_i - end +class GoodJobCheck < OkComputer::Check + def initialize;end def check - never_ran = Delayed::Job.where('run_at < ?', threshold.minutes.ago).count + count = GoodJob::Process.active.count - if never_ran.zero? - mark_message "All previous jobs have completed within the past #{threshold} minutes." - else + if count.zero? mark_failure - mark_message "#{never_ran} jobs waiting to be executed for more than #{threshold} minutes" + mark_message "No good_job processes are active." + else + mark_message "#{count} good_job processes are active." end end end @@ -67,20 +63,13 @@ class PumaCheck < OkComputer::Check end end -# Register delayed_job backed up test -dj_max = OpenProject::Configuration.health_checks_jobs_queue_count_threshold -OkComputer::Registry.register "delayed_jobs_backed_up", - OkComputer::DelayedJobBackedUpCheck.new(0, dj_max) - -dj_never_ran_max = OpenProject::Configuration.health_checks_jobs_never_ran_minutes_ago -OkComputer::Registry.register "delayed_jobs_never_ran", - DelayedJobNeverRanCheck.new(dj_never_ran_max) +OkComputer::Registry.register "worker", GoodJobCheck.new backlog_threshold = OpenProject::Configuration.health_checks_backlog_threshold OkComputer::Registry.register "puma", PumaCheck.new(backlog_threshold) # Make dj backed up optional due to bursts -OkComputer.make_optional %w(delayed_jobs_backed_up puma) +OkComputer.make_optional %w(puma) # Register web worker check for web + database OkComputer::CheckCollection.new('web').tap do |collection| @@ -94,8 +83,7 @@ OkComputer::CheckCollection.new('full').tap do |collection| collection.register :default, OkComputer::Registry.fetch('default') collection.register :database, OkComputer::Registry.fetch('database') collection.register :mail, OkComputer::ActionMailerCheck.new - collection.register :delayed_jobs_backed_up, OkComputer::Registry.fetch('delayed_jobs_backed_up') - collection.register :delayed_jobs_never_ran, OkComputer::Registry.fetch('delayed_jobs_never_ran') + collection.register :worker, OkComputer::Registry.fetch('worker') collection.register :puma, OkComputer::Registry.fetch('puma') OkComputer::Registry.default_collection.register 'full', collection end diff --git a/modules/webhooks/lib/open_project/webhooks/engine.rb b/modules/webhooks/lib/open_project/webhooks/engine.rb index ead276ce770..2309193c735 100644 --- a/modules/webhooks/lib/open_project/webhooks/engine.rb +++ b/modules/webhooks/lib/open_project/webhooks/engine.rb @@ -54,7 +54,7 @@ module OpenProject::Webhooks add_cron_jobs do { CleanupWebhookLogsJob: { - cron: '28 5 * * 7', # runs at 5:28 on Sunday + cron: '*/1 * * * *', # runs at 5:28 on Sunday class: 'CleanupWebhookLogsJob' } } From e1ce114da0dfed43eeb923b56a92e4ae2c23bfeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 19 Feb 2024 21:37:18 +0100 Subject: [PATCH 04/19] Extract health checks into separate files --- config/initializers/health_checks.rb | 118 +++++------------- .../health_checks/good_job_backed_up_check.rb | 48 +++++++ .../health_checks/good_job_check.rb | 43 +++++++ lib/open_project/health_checks/puma_check.rb | 78 ++++++++++++ lib/open_project/health_checks/smtp_check.rb | 45 +++++++ 5 files changed, 244 insertions(+), 88 deletions(-) create mode 100644 lib/open_project/health_checks/good_job_backed_up_check.rb create mode 100644 lib/open_project/health_checks/good_job_check.rb create mode 100644 lib/open_project/health_checks/puma_check.rb create mode 100644 lib/open_project/health_checks/smtp_check.rb diff --git a/config/initializers/health_checks.rb b/config/initializers/health_checks.rb index dd7d2a42b66..d0a09268c55 100644 --- a/config/initializers/health_checks.rb +++ b/config/initializers/health_checks.rb @@ -1,95 +1,37 @@ require 'ok_computer/ok_computer_controller' -class GoodJobCheck < OkComputer::Check - def initialize;end +Rails.application.configure do + config.after_initialize do + OkComputer::Registry.register "worker", OpenProject::HealthChecks::GoodJobCheck.new + OkComputer::Registry.register "worker_backed_up", OpenProject::HealthChecks::GoodJobBackedUpCheck.new - def check - count = GoodJob::Process.active.count + OkComputer::Registry.register "puma", OpenProject::HealthChecks::PumaCheck.new - if count.zero? - mark_failure - mark_message "No good_job processes are active." - else - mark_message "#{count} good_job processes are active." + # Make dj backed up optional due to bursts + OkComputer.make_optional %w(worker_backed_up puma) + + # Register web worker check for web + database + OkComputer::CheckCollection.new('web').tap do |collection| + collection.register :default, OkComputer::Registry.fetch('default') + collection.register :database, OkComputer::Registry.fetch('database') + OkComputer::Registry.default_collection.register 'web', collection + end + + # Register full check for web + database + dj worker + OkComputer::CheckCollection.new('full').tap do |collection| + collection.register :default, OkComputer::Registry.fetch('default') + collection.register :database, OkComputer::Registry.fetch('database') + collection.register :mail, OpenProject::HealthChecks::SmtpCheck.new + collection.register :worker, OkComputer::Registry.fetch('worker') + collection.register :worker_backed_up, OkComputer::Registry.fetch('worker_backed_up') + collection.register :puma, OkComputer::Registry.fetch('puma') + OkComputer::Registry.default_collection.register 'full', collection + end + + # Check if authentication required + authentication_password = OpenProject::Configuration.health_checks_authentication_password + if authentication_password.present? + OkComputer.require_authentication('health_checks', authentication_password) end end end - -class PumaCheck < OkComputer::Check - attr_reader :threshold - - def initialize(backlog_threshold) - @threshold = backlog_threshold.to_i - end - - def check - stats = self.stats - - return mark_message "N/A as Puma is not used." if stats.nil? - - if stats[:running] > 0 - mark_message "Puma is running" - else - mark_failure - mark_message "Puma is not running" - end - - if stats[:backlog] < threshold - mark_message "Backlog ok" - else - mark_failure - mark_message "Backlog congested" - end - end - - def stats - return nil unless applicable? - - server = Puma::Server.current - return nil if server.nil? - - { - backlog: server.backlog || 0, - running: server.running || 0, - pool_capacity: server.pool_capacity || 0, - max_threads: server.max_threads || 0 - } - end - - def applicable? - return @applicable unless @applicable.nil? - - @applicable = Object.const_defined?("Puma::Server") && !Puma::Server.current.nil? - end -end - -OkComputer::Registry.register "worker", GoodJobCheck.new - -backlog_threshold = OpenProject::Configuration.health_checks_backlog_threshold -OkComputer::Registry.register "puma", PumaCheck.new(backlog_threshold) - -# Make dj backed up optional due to bursts -OkComputer.make_optional %w(puma) - -# Register web worker check for web + database -OkComputer::CheckCollection.new('web').tap do |collection| - collection.register :default, OkComputer::Registry.fetch('default') - collection.register :database, OkComputer::Registry.fetch('database') - OkComputer::Registry.default_collection.register 'web', collection -end - -# Register full check for web + database + dj worker -OkComputer::CheckCollection.new('full').tap do |collection| - collection.register :default, OkComputer::Registry.fetch('default') - collection.register :database, OkComputer::Registry.fetch('database') - collection.register :mail, OkComputer::ActionMailerCheck.new - collection.register :worker, OkComputer::Registry.fetch('worker') - collection.register :puma, OkComputer::Registry.fetch('puma') - OkComputer::Registry.default_collection.register 'full', collection -end - -# Check if authentication required -authentication_password = OpenProject::Configuration.health_checks_authentication_password -if authentication_password.present? - OkComputer.require_authentication('health_checks', authentication_password) -end diff --git a/lib/open_project/health_checks/good_job_backed_up_check.rb b/lib/open_project/health_checks/good_job_backed_up_check.rb new file mode 100644 index 00000000000..9c7c131ea82 --- /dev/null +++ b/lib/open_project/health_checks/good_job_backed_up_check.rb @@ -0,0 +1,48 @@ +#-- 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 docs/COPYRIGHT.rdoc for more details. +#++ +module OpenProject + module HealthChecks + class GoodJobBackedUpCheck < OkComputer::Check + def initialize(threshold = OpenProject::Configuration.health_checks_jobs_never_ran_minutes_ago) + @threshold = threshold.to_i + super() + end + + def check + count = GoodJob::Job.queued.where('scheduled_at < ?', @threshold.minutes.ago).count + + if count > 0 + mark_message "#{count} jobs are waiting to be picked up for more than #{@threshold} minutes." + mark_failure + else + mark_message "No jobs are waiting to be picked up." + end + end + end + end +end diff --git a/lib/open_project/health_checks/good_job_check.rb b/lib/open_project/health_checks/good_job_check.rb new file mode 100644 index 00000000000..9a996572bd9 --- /dev/null +++ b/lib/open_project/health_checks/good_job_check.rb @@ -0,0 +1,43 @@ +#-- 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 docs/COPYRIGHT.rdoc for more details. +#++ +module OpenProject + module HealthChecks + class GoodJobCheck < OkComputer::Check + def check + count = GoodJob::Process.active.count + + if count.zero? + mark_failure + mark_message "No good_job processes are active." + else + mark_message "#{count} good_job processes are active." + end + end + end + end +end diff --git a/lib/open_project/health_checks/puma_check.rb b/lib/open_project/health_checks/puma_check.rb new file mode 100644 index 00000000000..583a4a00817 --- /dev/null +++ b/lib/open_project/health_checks/puma_check.rb @@ -0,0 +1,78 @@ +#-- 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 docs/COPYRIGHT.rdoc for more details. +#++ +module OpenProject + module HealthChecks + class PumaCheck < OkComputer::Check + attr_reader :threshold + + def initialize(threshold = OpenProject::Configuration.health_checks_backlog_threshold) + @threshold = threshold.to_i + @applicable = Object.const_defined?("Puma::Server") && !Puma::Server.current.nil? + super() + end + + def check + stats = self.stats + + return mark_message "N/A as Puma is not used." if stats.nil? + + if stats[:running] > 0 + mark_message "Puma is running" + else + mark_failure + mark_message "Puma is not running" + end + + if stats[:backlog] < threshold + mark_message "Backlog ok" + else + mark_failure + mark_message "Backlog congested" + end + end + + def stats + return nil unless applicable? + + server = Puma::Server.current + return nil if server.nil? + + { + backlog: server.backlog || 0, + running: server.running || 0, + pool_capacity: server.pool_capacity || 0, + max_threads: server.max_threads || 0 + } + end + + def applicable? + !!@applicable + end + end + end +end diff --git a/lib/open_project/health_checks/smtp_check.rb b/lib/open_project/health_checks/smtp_check.rb new file mode 100644 index 00000000000..81b09ce4d1f --- /dev/null +++ b/lib/open_project/health_checks/smtp_check.rb @@ -0,0 +1,45 @@ +#-- 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 docs/COPYRIGHT.rdoc for more details. +#++ +module OpenProject + module HealthChecks + class SmtpCheck < OkComputer::ActionMailerCheck + def check + unless Setting.email_delivery_method == :smtp + mark_message "Mail delivery method is not SMTP" + return + end + + # settings might change between calls + @host = Setting.smtp_address + @port = Setting.smtp_port + + super + end + end + end +end From 8c86d127ec265c53159d09805707e8fb27d791c3 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 20 Feb 2024 11:11:49 +0100 Subject: [PATCH 05/19] Enable GoodJob dashboard only in development. --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index f90494edc42..82a8c3b042f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -640,7 +640,7 @@ Rails.application.routes.draw do mount Lookbook::Engine, at: "/lookbook" end - constraints(->(req) { User.exists?(id: req.session[:user_id], admin: true) }) do + if Rails.env.development? mount GoodJob::Engine => 'good_job' end end From e6dcb82cb90a6d54ca0468ec637bedc0912f2272 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 20 Feb 2024 11:13:14 +0100 Subject: [PATCH 06/19] Resurrect removed migrations. --- ...20220202140507_reorder_project_children.rb | 33 +++++++++++++++++ ...4112533_remove_notification_cleanup_job.rb | 35 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 db/migrate/20220202140507_reorder_project_children.rb create mode 100644 db/migrate/20220804112533_remove_notification_cleanup_job.rb diff --git a/db/migrate/20220202140507_reorder_project_children.rb b/db/migrate/20220202140507_reorder_project_children.rb new file mode 100644 index 00000000000..d964d0d077e --- /dev/null +++ b/db/migrate/20220202140507_reorder_project_children.rb @@ -0,0 +1,33 @@ +#-- 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 ReorderProjectChildren < ActiveRecord::Migration[6.1] + def up + ::Projects::ReorderHierarchyJob.perform_later + end +end diff --git a/db/migrate/20220804112533_remove_notification_cleanup_job.rb b/db/migrate/20220804112533_remove_notification_cleanup_job.rb new file mode 100644 index 00000000000..440ddb2fe30 --- /dev/null +++ b/db/migrate/20220804112533_remove_notification_cleanup_job.rb @@ -0,0 +1,35 @@ +#-- 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 RemoveNotificationCleanupJob < ActiveRecord::Migration[7.0] + def up + Setting + .where(name: 'notification_retention_period_days') + .delete_all + end +end From e615525a597ab29ce834d314d793c669215e90ac Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 20 Feb 2024 12:12:54 +0100 Subject: [PATCH 07/19] Run ReorderHierarchyJob synchronously in migration --- db/migrate/20220202140507_reorder_project_children.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20220202140507_reorder_project_children.rb b/db/migrate/20220202140507_reorder_project_children.rb index d964d0d077e..f865f4054de 100644 --- a/db/migrate/20220202140507_reorder_project_children.rb +++ b/db/migrate/20220202140507_reorder_project_children.rb @@ -28,6 +28,6 @@ class ReorderProjectChildren < ActiveRecord::Migration[6.1] def up - ::Projects::ReorderHierarchyJob.perform_later + ::Projects::ReorderHierarchyJob.new.perform end end From f17ad241b12ee9ccc482306797da845bfaee6e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 20 Feb 2024 12:39:40 +0100 Subject: [PATCH 08/19] Remove the reorder_hierarchy_job and inline it in the migration --- app/workers/projects/reorder_hierarchy_job.rb | 73 ------------------- ...20220202140507_reorder_project_children.rb | 49 ++++++++++++- .../reorder_project_children_spec.rb} | 6 +- 3 files changed, 52 insertions(+), 76 deletions(-) delete mode 100644 app/workers/projects/reorder_hierarchy_job.rb rename spec/{workers/projects/reorder_children_job_integration_spec.rb => migrations/reorder_project_children_spec.rb} (89%) diff --git a/app/workers/projects/reorder_hierarchy_job.rb b/app/workers/projects/reorder_hierarchy_job.rb deleted file mode 100644 index eafa3ed0efd..00000000000 --- a/app/workers/projects/reorder_hierarchy_job.rb +++ /dev/null @@ -1,73 +0,0 @@ -#-- 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. -#++ - -module Projects - class ReorderHierarchyJob < ApplicationJob - def perform - Rails.logger.info { "Resorting siblings by name in the project's nested set." } - Project.transaction { reorder! } - end - - private - - def reorder! - # Reorder the project roots - reorder_siblings Project.roots - - # Reorder every project hierarchy - Project - .where(id: unique_parent_ids) - .find_each { |project| reorder_siblings(project.children) } - end - - def unique_parent_ids - Project - .where.not(parent_id: nil) - .select(:parent_id) - .distinct - end - - def reorder_siblings(siblings) - return unless siblings.many? - - # Resort children manually - sorted = siblings.sort_by { |project| project.name.downcase } - - # Get the current first child - first = siblings.first - - sorted.each_with_index do |child, i| - if i == 0 - child.move_to_left_of(first) unless child == first - else - child.move_to_right_of(sorted[i - 1]) - end - end - end - end -end diff --git a/db/migrate/20220202140507_reorder_project_children.rb b/db/migrate/20220202140507_reorder_project_children.rb index f865f4054de..9970c0da3c9 100644 --- a/db/migrate/20220202140507_reorder_project_children.rb +++ b/db/migrate/20220202140507_reorder_project_children.rb @@ -27,7 +27,54 @@ #++ class ReorderProjectChildren < ActiveRecord::Migration[6.1] + class ProjectMigration < ApplicationRecord + include ::Projects::Hierarchy + self.table_name = 'projects' + end + def up - ::Projects::ReorderHierarchyJob.new.perform + Rails.logger.info { "Resorting siblings by name in the project's nested set." } + ProjectMigration.transaction { reorder! } + end + + def down + # Nothing to do + end + + private + + def reorder! + # Reorder the project roots + reorder_siblings ProjectMigration.roots + + # Reorder every project hierarchy + ProjectMigration + .where(id: unique_parent_ids) + .find_each { |project| reorder_siblings(project.children) } + end + + def unique_parent_ids + ProjectMigration + .where.not(parent_id: nil) + .select(:parent_id) + .distinct + end + + def reorder_siblings(siblings) + return unless siblings.many? + + # Resort children manually + sorted = siblings.sort_by { |project| project.name.downcase } + + # Get the current first child + first = siblings.first + + sorted.each_with_index do |child, i| + if i == 0 + child.move_to_left_of(first) unless child == first + else + child.move_to_right_of(sorted[i - 1]) + end + end end end diff --git a/spec/workers/projects/reorder_children_job_integration_spec.rb b/spec/migrations/reorder_project_children_spec.rb similarity index 89% rename from spec/workers/projects/reorder_children_job_integration_spec.rb rename to spec/migrations/reorder_project_children_spec.rb index 7f63c6258e9..ce2098186bc 100644 --- a/spec/workers/projects/reorder_children_job_integration_spec.rb +++ b/spec/migrations/reorder_project_children_spec.rb @@ -27,9 +27,11 @@ #++ require 'spec_helper' +require Rails.root.join("db/migrate/20220202140507_reorder_project_children.rb") -RSpec.describe Projects::ReorderHierarchyJob, type: :model do - subject(:job) { described_class.perform_now } +RSpec.describe ReorderProjectChildren, type: :model do + # Silencing migration logs, since we are not interested in that during testing + subject(:run_migration) { ActiveRecord::Migration.suppress_messages { described_class.new.up } } shared_let(:parent_project_a) { create(:project, name: 'ParentA') } shared_let(:parent_project_b) { create(:project, name: 'ParentB') } From 8ab39f1393a36d36a27079f3e7519a737a41f0a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 22 Feb 2024 15:07:33 +0100 Subject: [PATCH 09/19] Fix testing value for cron --- modules/webhooks/lib/open_project/webhooks/engine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/webhooks/lib/open_project/webhooks/engine.rb b/modules/webhooks/lib/open_project/webhooks/engine.rb index 2309193c735..ead276ce770 100644 --- a/modules/webhooks/lib/open_project/webhooks/engine.rb +++ b/modules/webhooks/lib/open_project/webhooks/engine.rb @@ -54,7 +54,7 @@ module OpenProject::Webhooks add_cron_jobs do { CleanupWebhookLogsJob: { - cron: '*/1 * * * *', # runs at 5:28 on Sunday + cron: '28 5 * * 7', # runs at 5:28 on Sunday class: 'CleanupWebhookLogsJob' } } From dab95b6d6435f1f57ed9eb306a540aef1e4a0e89 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Mon, 26 Feb 2024 16:06:38 +0100 Subject: [PATCH 10/19] Use plain sql to cleanup removed cron_jobs. --- .../20201125121949_remove_renamed_cron_job.rb | 36 +++++++++++++++++++ ...4112533_remove_notification_cleanup_job.rb | 1 + ...105073117_remove_renamed_date_alert_job.rb | 35 ++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 db/migrate/20201125121949_remove_renamed_cron_job.rb create mode 100644 db/migrate/20230105073117_remove_renamed_date_alert_job.rb diff --git a/db/migrate/20201125121949_remove_renamed_cron_job.rb b/db/migrate/20201125121949_remove_renamed_cron_job.rb new file mode 100644 index 00000000000..befb6701611 --- /dev/null +++ b/db/migrate/20201125121949_remove_renamed_cron_job.rb @@ -0,0 +1,36 @@ +#-- 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 RemoveRenamedCronJob < ActiveRecord::Migration[6.0] + def up + # The job has been renamed to JobStatus::Cron::ClearOldJobStatusJob + # the new job will be added on restarting the application but the old will still be in the database + # and will cause 'uninitialized constant' errors. + execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Cron::ClearOldJobStatusJob%'") + end +end diff --git a/db/migrate/20220804112533_remove_notification_cleanup_job.rb b/db/migrate/20220804112533_remove_notification_cleanup_job.rb index 440ddb2fe30..f9f6cb6a7ec 100644 --- a/db/migrate/20220804112533_remove_notification_cleanup_job.rb +++ b/db/migrate/20220804112533_remove_notification_cleanup_job.rb @@ -28,6 +28,7 @@ class RemoveNotificationCleanupJob < ActiveRecord::Migration[7.0] def up + execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CleanupJob%'") Setting .where(name: 'notification_retention_period_days') .delete_all diff --git a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb new file mode 100644 index 00000000000..1ba4f86c533 --- /dev/null +++ b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb @@ -0,0 +1,35 @@ +#-- 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 RemoveRenamedDateAlertJob < ActiveRecord::Migration[6.0] + def up + # The job has been renamed to Notifications::ScheduleDateAlertsNotificationsJob. + # The new job will be added on restarting the application. + execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CreateDateAlertsNotificationsJob%'") + end +end From 2267a0a1e3fdcdc543b2972c2a5c51e3a6e8d013 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 27 Feb 2024 22:35:33 +0100 Subject: [PATCH 11/19] 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. --- .../settings/working_days_params_contract.rb | 2 +- app/workers/application_job.rb | 5 +- .../schedule_date_alerts_notifications_job.rb | 2 +- .../schedule_reminder_mails_job.rb | 2 +- bin/check-worker-liveness | 4 +- config/application.rb | 16 ++++--- config/constants/settings/definition.rb | 30 ++++++++++++ config/initializers/cronjobs.rb | 22 ++++----- .../20240227154544_remove_delayed_jobs.rb | 47 +++++++++++++++++++ .../installation-faq/README.md | 2 +- .../operation/monitoring/README.md | 6 ++- .../integrations/nextcloud/README.md | 2 +- .../work-packages/work-packages-faq/README.md | 2 +- .../open_project/github_integration/engine.rb | 4 +- .../lib/open_project/job_status/engine.rb | 2 +- .../lib/open_project/ldap_groups/engine.rb | 2 +- .../manage_nextcloud_integration_job.rb | 18 ++++--- .../lib/open_project/storages/engine.rb | 4 +- ...eanup_uncontainered_file_links_job_spec.rb | 2 +- .../lib/open_project/webhooks/engine.rb | 2 +- .../notifications/reminder_mail_spec.rb | 2 +- .../schedule_reminder_mails_job_spec.rb | 4 +- 22 files changed, 133 insertions(+), 49 deletions(-) create mode 100644 db/migrate/20240227154544_remove_delayed_jobs.rb diff --git a/app/contracts/settings/working_days_params_contract.rb b/app/contracts/settings/working_days_params_contract.rb index 1c8f16c28c5..71c0aeaaf56 100644 --- a/app/contracts/settings/working_days_params_contract.rb +++ b/app/contracts/settings/working_days_params_contract.rb @@ -45,7 +45,7 @@ module Settings 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 diff --git a/app/workers/application_job.rb b/app/workers/application_job.rb index 703cedbfe11..399968dcb40 100644 --- a/app/workers/application_job.rb +++ b/app/workers/application_job.rb @@ -92,9 +92,8 @@ class ApplicationJob < ActiveJob::Base 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 diff --git a/app/workers/notifications/schedule_date_alerts_notifications_job.rb b/app/workers/notifications/schedule_date_alerts_notifications_job.rb index a351ec983fb..525a0473bdd 100644 --- a/app/workers/notifications/schedule_date_alerts_notifications_job.rb +++ b/app/workers/notifications/schedule_date_alerts_notifications_job.rb @@ -53,7 +53,7 @@ module Notifications 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 diff --git a/app/workers/notifications/schedule_reminder_mails_job.rb b/app/workers/notifications/schedule_reminder_mails_job.rb index 6c985161b75..0c17199d1c7 100644 --- a/app/workers/notifications/schedule_reminder_mails_job.rb +++ b/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) diff --git a/bin/check-worker-liveness b/bin/check-worker-liveness index fe248d9dfb6..7d02aff9ff4 100755 --- a/bin/check-worker-liveness +++ b/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` diff --git a/config/application.rb b/config/application.rb index 2290c287a96..abcf69011f0 100644 --- a/config/application.rb +++ b/config/application.rb @@ -214,15 +214,17 @@ module OpenProject 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 diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index 1eed394480f..f88bccbff1e 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -483,6 +483,36 @@ module Settings 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" }, diff --git a/config/initializers/cronjobs.rb b/config/initializers/cronjobs.rb index ba22d94574e..ab3a200a808 100644 --- a/config/initializers/cronjobs.rb +++ b/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 } } ) diff --git a/db/migrate/20240227154544_remove_delayed_jobs.rb b/db/migrate/20240227154544_remove_delayed_jobs.rb new file mode 100644 index 00000000000..ea929b73d4e --- /dev/null +++ b/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 diff --git a/docs/installation-and-operations/installation-faq/README.md b/docs/installation-and-operations/installation-faq/README.md index 802e11c602c..a086e93eec3 100644 --- a/docs/installation-and-operations/installation-faq/README.md +++ b/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. diff --git a/docs/installation-and-operations/operation/monitoring/README.md b/docs/installation-and-operations/operation/monitoring/README.md index 40d740b3346..145dba04978 100644 --- a/docs/installation-and-operations/operation/monitoring/README.md +++ b/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 diff --git a/docs/system-admin-guide/integrations/nextcloud/README.md b/docs/system-admin-guide/integrations/nextcloud/README.md index f4787b9eac9..aa4ea7617d8 100644 --- a/docs/system-admin-guide/integrations/nextcloud/README.md +++ b/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:///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:///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. diff --git a/docs/user-guide/work-packages/work-packages-faq/README.md b/docs/user-guide/work-packages/work-packages-faq/README.md index 5ad656c8c6d..fa3a7715a8d 100644 --- a/docs/user-guide/work-packages/work-packages-faq/README.md +++ b/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"). diff --git a/modules/github_integration/lib/open_project/github_integration/engine.rb b/modules/github_integration/lib/open_project/github_integration/engine.rb index 14cd9a03d6d..20ff1a0eaa5 100644 --- a/modules/github_integration/lib/open_project/github_integration/engine.rb +++ b/modules/github_integration/lib/open_project/github_integration/engine.rb @@ -87,9 +87,9 @@ module OpenProject::GithubIntegration add_cron_jobs do { - ClearOldPullRequestsJob: { + 'Cron::ClearOldPullRequestsJob': { cron: '25 1 * * *', # runs at 1:25 nightly - class: 'ClearOldPullRequestsJob' + class: ::Cron::ClearOldPullRequestsJob.name } } end diff --git a/modules/job_status/lib/open_project/job_status/engine.rb b/modules/job_status/lib/open_project/job_status/engine.rb index e5147bde388..543067144e7 100644 --- a/modules/job_status/lib/open_project/job_status/engine.rb +++ b/modules/job_status/lib/open_project/job_status/engine.rb @@ -50,7 +50,7 @@ module OpenProject::JobStatus { 'JobStatus::Cron::ClearOldJobStatusJob': { cron: '15 4 * * *', # runs at 4:15 nightly - class: '::JobStatus::Cron::ClearOldJobStatusJob' + class: ::JobStatus::Cron::ClearOldJobStatusJob.name } } end diff --git a/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb b/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb index 6bb2faff47d..3b6c387b81c 100644 --- a/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb +++ b/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb @@ -23,7 +23,7 @@ module OpenProject::LdapGroups { 'Ldap::SynchronizationJob': { cron: '30 23 * * *', # Run once per night at 11:30pm - class: 'Ldap::SynchronizationJob' + class: Ldap::SynchronizationJob.name } } end diff --git a/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb index b626405d23a..dae51a8433f 100644 --- a/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb +++ b/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb @@ -66,8 +66,8 @@ module Storages 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 @@ module Storages 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 @@ module Storages 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? diff --git a/modules/storages/lib/open_project/storages/engine.rb b/modules/storages/lib/open_project/storages/engine.rb index 6dc9488260d..cdf8c7c8176 100644 --- a/modules/storages/lib/open_project/storages/engine.rb +++ b/modules/storages/lib/open_project/storages/engine.rb @@ -287,12 +287,12 @@ module OpenProject::Storages { 'Storages::CleanupUncontaineredFileLinksJob': { cron: "06 22 * * *", - class: "Storages::CleanupUncontaineredFileLinksJob" + class: ::Storages::CleanupUncontaineredFileLinksJob.name }, 'Storages::ManageNextcloudIntegrationJob': { cron: "*/5 * * * *", - class: "Storages::ManageNextcloudIntegrationJob" + class: ::Storages::ManageNextcloudIntegrationJob.name } } end diff --git a/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb b/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb index a5924b5a746..5ae9c52fac3 100644 --- a/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb +++ b/modules/storages/spec/workers/storages/cleanup_uncontainered_file_links_job_spec.rb @@ -32,7 +32,7 @@ require 'spec_helper' 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) diff --git a/modules/webhooks/lib/open_project/webhooks/engine.rb b/modules/webhooks/lib/open_project/webhooks/engine.rb index ead276ce770..7f540ee1667 100644 --- a/modules/webhooks/lib/open_project/webhooks/engine.rb +++ b/modules/webhooks/lib/open_project/webhooks/engine.rb @@ -55,7 +55,7 @@ module OpenProject::Webhooks { CleanupWebhookLogsJob: { cron: '28 5 * * 7', # runs at 5:28 on Sunday - class: 'CleanupWebhookLogsJob' + class: ::CleanupWebhookLogsJob.name } } end diff --git a/spec/features/notifications/reminder_mail_spec.rb b/spec/features/notifications/reminder_mail_spec.rb index 680b6e36fe5..dd28217d025 100644 --- a/spec/features/notifications/reminder_mail_spec.rb +++ b/spec/features/notifications/reminder_mail_spec.rb @@ -9,7 +9,7 @@ RSpec.describe "Reminder email sending", :js, :with_cuprite do 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. diff --git a/spec/workers/notifications/schedule_reminder_mails_job_spec.rb b/spec/workers/notifications/schedule_reminder_mails_job_spec.rb index 94c629da2eb..d295ab6feba 100644 --- a/spec/workers/notifications/schedule_reminder_mails_job_spec.rb +++ b/spec/workers/notifications/schedule_reminder_mails_job_spec.rb @@ -57,7 +57,7 @@ RSpec.describe Notifications::ScheduleReminderMailsJob, type: :job do 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 @@ RSpec.describe Notifications::ScheduleReminderMailsJob, type: :job do 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' From f2ffac9e77b832c0db725a7bbf693be3d34af015 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Wed, 28 Feb 2024 09:28:59 +0100 Subject: [PATCH 12/19] Make execute_sql work. --- db/migrate/20201125121949_remove_renamed_cron_job.rb | 2 ++ db/migrate/20220804112533_remove_notification_cleanup_job.rb | 2 ++ db/migrate/20230105073117_remove_renamed_date_alert_job.rb | 2 ++ 3 files changed, 6 insertions(+) diff --git a/db/migrate/20201125121949_remove_renamed_cron_job.rb b/db/migrate/20201125121949_remove_renamed_cron_job.rb index befb6701611..c2515075fc6 100644 --- a/db/migrate/20201125121949_remove_renamed_cron_job.rb +++ b/db/migrate/20201125121949_remove_renamed_cron_job.rb @@ -27,6 +27,8 @@ #++ class RemoveRenamedCronJob < ActiveRecord::Migration[6.0] + include ::Migration::Utils + def up # The job has been renamed to JobStatus::Cron::ClearOldJobStatusJob # the new job will be added on restarting the application but the old will still be in the database diff --git a/db/migrate/20220804112533_remove_notification_cleanup_job.rb b/db/migrate/20220804112533_remove_notification_cleanup_job.rb index f9f6cb6a7ec..f92ae07203c 100644 --- a/db/migrate/20220804112533_remove_notification_cleanup_job.rb +++ b/db/migrate/20220804112533_remove_notification_cleanup_job.rb @@ -27,6 +27,8 @@ #++ class RemoveNotificationCleanupJob < ActiveRecord::Migration[7.0] + include ::Migration::Utils + def up execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CleanupJob%'") Setting diff --git a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb index 1ba4f86c533..aec7f000008 100644 --- a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb +++ b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb @@ -27,6 +27,8 @@ #++ class RemoveRenamedDateAlertJob < ActiveRecord::Migration[6.0] + include ::Migration::Utils + def up # The job has been renamed to Notifications::ScheduleDateAlertsNotificationsJob. # The new job will be added on restarting the application. From f296c54ee75c8121d5093b4500c7aafbf0025d77 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Wed, 28 Feb 2024 17:11:19 +0100 Subject: [PATCH 13/19] Fix #upsert_status race condition. Rescue ActiveRecord::RecordNotUnique and then retry. --- .../job_status/application_job_with_status.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/modules/job_status/app/workers/job_status/application_job_with_status.rb b/modules/job_status/app/workers/job_status/application_job_with_status.rb index e754700cde2..9aa2c8291b8 100644 --- a/modules/job_status/app/workers/job_status/application_job_with_status.rb +++ b/modules/job_status/app/workers/job_status/application_job_with_status.rb @@ -27,8 +27,7 @@ #++ module JobStatus module ApplicationJobWithStatus - # Delayed jobs can have a status: - # Delayed::Job::Status + # Backgroun jobs can have a status JobStatus::Status # which is related to the job via a reference which is an AR model instance. def status_reference nil @@ -61,8 +60,6 @@ module JobStatus ## # Update the status code for a given job def upsert_status(status:, **args) - # Can't use upsert, as we only want to insert the user_id once - # and not update it repeatedly resource = ::JobStatus::Status.find_or_initialize_by(job_id:) if resource.new_record? @@ -77,7 +74,16 @@ module JobStatus resource.attributes = build_status_attributes(args.merge(status:)) end + # There is a possible race condition because of unique delayed_job_statuses.job_id + # Can't use upsert easily, because before updating we need to know user_id + # to set proper locale. Probably, it is possible to get it from + # a job's payload, then it would be possible to correctly prepare attributes before using upsert. + # Therefore, it is up to possible optimization in future. Now the race condition is handled with + # handling ActiveRecord::RecordNotUnique and trying again. resource.save! + rescue ActiveRecord::RecordNotUnique + OpenProject.logger.info("Retrying ApplicationJobWithStatus#upsert_status.") + retry end protected From 8eb567444579d8f8ae38b578458ec3387452fd6e Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Wed, 28 Feb 2024 17:21:06 +0100 Subject: [PATCH 14/19] Remove TODO comment in favor of dedicated ticket. https://community.openproject.org/wp/53122 --- app/contracts/settings/working_days_params_contract.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/contracts/settings/working_days_params_contract.rb b/app/contracts/settings/working_days_params_contract.rb index 71c0aeaaf56..cd0faf86d21 100644 --- a/app/contracts/settings/working_days_params_contract.rb +++ b/app/contracts/settings/working_days_params_contract.rb @@ -41,7 +41,6 @@ module Settings end end - # TODO: consider implementing using GoodJob concurrency control mechanisms def unique_job if GoodJob::Job .where(finished_at: nil) From a59e01e3efcb96787a38c60e3594ae031d98e25c Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Thu, 29 Feb 2024 14:53:00 +0100 Subject: [PATCH 15/19] Restore delayed_job related migration statements. --- ...6132134_bigint_primary_and_foreign_keys.rb | 1 + .../20231009135807_remove_renamed_cronjobs.rb | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb diff --git a/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb b/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb index 9a60233b59a..795c3473c55 100644 --- a/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb +++ b/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb @@ -61,6 +61,7 @@ class BigintPrimaryAndForeignKeys < ActiveRecord::Migration[7.0] CustomValue => %i[id customized_id custom_field_id], Journal::CustomizableJournal => %i[id journal_id custom_field_id], DesignColor => [:id], + :delayed_jobs => [:id], # delayed job removed in favour of good_job see WP #42 or PR #42 Journal::DocumentJournal => %i[id project_id category_id], Document => %i[id project_id category_id], :done_statuses_for_project => %i[project_id status_id], diff --git a/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb b/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb new file mode 100644 index 00000000000..7454c8c893b --- /dev/null +++ b/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb @@ -0,0 +1,38 @@ +#-- 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 RemoveRenamedCronjobs < ActiveRecord::Migration[7.0] + include ::Migration::Utils + + def up + execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: CleanupUncontaineredFileLinksJob%'") + execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: ManageNextcloudIntegrationJob%'") + end + + def down; end +end From 0294e4bc4ae3a6a5d22f0f9a23f8f6fb5c469d64 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Thu, 29 Feb 2024 14:54:01 +0100 Subject: [PATCH 16/19] Rename delayed_job_statuses to job_statuses. --- ...40229133250_rename_delayed_job_statuses.rb | 33 +++++++++++++++++++ .../app/models/job_status/status.rb | 30 ++++++++++++++++- .../job_status/application_job_with_status.rb | 2 +- 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20240229133250_rename_delayed_job_statuses.rb diff --git a/db/migrate/20240229133250_rename_delayed_job_statuses.rb b/db/migrate/20240229133250_rename_delayed_job_statuses.rb new file mode 100644 index 00000000000..43ffd4dd457 --- /dev/null +++ b/db/migrate/20240229133250_rename_delayed_job_statuses.rb @@ -0,0 +1,33 @@ +#-- 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 RenameDelayedJobStatuses < ActiveRecord::Migration[7.1] + def change + rename_table :delayed_job_statuses, :job_statuses + end +end diff --git a/modules/job_status/app/models/job_status/status.rb b/modules/job_status/app/models/job_status/status.rb index 6f4b3bc6a8e..928b40d91fa 100644 --- a/modules/job_status/app/models/job_status/status.rb +++ b/modules/job_status/app/models/job_status/status.rb @@ -1,6 +1,34 @@ +#-- 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. +#++ + module JobStatus class Status < ApplicationRecord - self.table_name = 'delayed_job_statuses' + self.table_name = 'job_statuses' belongs_to :user belongs_to :reference, polymorphic: true diff --git a/modules/job_status/app/workers/job_status/application_job_with_status.rb b/modules/job_status/app/workers/job_status/application_job_with_status.rb index 9aa2c8291b8..e0c6e128fce 100644 --- a/modules/job_status/app/workers/job_status/application_job_with_status.rb +++ b/modules/job_status/app/workers/job_status/application_job_with_status.rb @@ -74,7 +74,7 @@ module JobStatus resource.attributes = build_status_attributes(args.merge(status:)) end - # There is a possible race condition because of unique delayed_job_statuses.job_id + # There is a possible race condition because of unique job_statuses.job_id # Can't use upsert easily, because before updating we need to know user_id # to set proper locale. Probably, it is possible to get it from # a job's payload, then it would be possible to correctly prepare attributes before using upsert. From fa67e0436dae61236aede19072657f97138ae0d7 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Thu, 29 Feb 2024 15:21:40 +0100 Subject: [PATCH 17/19] Use execute over execute_sql. Due to no need in sanitization. --- db/migrate/20201125121949_remove_renamed_cron_job.rb | 4 +--- .../20220804112533_remove_notification_cleanup_job.rb | 4 +--- db/migrate/20230105073117_remove_renamed_date_alert_job.rb | 4 +--- .../db/migrate/20231009135807_remove_renamed_cronjobs.rb | 6 ++---- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/db/migrate/20201125121949_remove_renamed_cron_job.rb b/db/migrate/20201125121949_remove_renamed_cron_job.rb index c2515075fc6..be642846ee2 100644 --- a/db/migrate/20201125121949_remove_renamed_cron_job.rb +++ b/db/migrate/20201125121949_remove_renamed_cron_job.rb @@ -27,12 +27,10 @@ #++ class RemoveRenamedCronJob < ActiveRecord::Migration[6.0] - include ::Migration::Utils - def up # The job has been renamed to JobStatus::Cron::ClearOldJobStatusJob # the new job will be added on restarting the application but the old will still be in the database # and will cause 'uninitialized constant' errors. - execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Cron::ClearOldJobStatusJob%'") + execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Cron::ClearOldJobStatusJob%'") end end diff --git a/db/migrate/20220804112533_remove_notification_cleanup_job.rb b/db/migrate/20220804112533_remove_notification_cleanup_job.rb index f92ae07203c..5f64f6d455a 100644 --- a/db/migrate/20220804112533_remove_notification_cleanup_job.rb +++ b/db/migrate/20220804112533_remove_notification_cleanup_job.rb @@ -27,10 +27,8 @@ #++ class RemoveNotificationCleanupJob < ActiveRecord::Migration[7.0] - include ::Migration::Utils - def up - execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CleanupJob%'") + execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CleanupJob%'") Setting .where(name: 'notification_retention_period_days') .delete_all diff --git a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb index aec7f000008..e59bc347aa6 100644 --- a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb +++ b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb @@ -27,11 +27,9 @@ #++ class RemoveRenamedDateAlertJob < ActiveRecord::Migration[6.0] - include ::Migration::Utils - def up # The job has been renamed to Notifications::ScheduleDateAlertsNotificationsJob. # The new job will be added on restarting the application. - execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CreateDateAlertsNotificationsJob%'") + execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CreateDateAlertsNotificationsJob%'") end end diff --git a/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb b/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb index 7454c8c893b..c6fe00093ae 100644 --- a/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb +++ b/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb @@ -27,11 +27,9 @@ #++ class RemoveRenamedCronjobs < ActiveRecord::Migration[7.0] - include ::Migration::Utils - def up - execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: CleanupUncontaineredFileLinksJob%'") - execute_sql("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: ManageNextcloudIntegrationJob%'") + execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: CleanupUncontaineredFileLinksJob%'") + execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: ManageNextcloudIntegrationJob%'") end def down; end From c69d5eb03d21eef7a9486873e4cd450f6ba6a017 Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 5 Mar 2024 12:19:21 +0100 Subject: [PATCH 18/19] Migrate 'in queue' jobs to good_jobs table. Update good_job version as well. --- Gemfile | 3 +- Gemfile.lock | 9 ++++-- .../20240227154544_remove_delayed_jobs.rb | 29 +++++++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 629cb0e8879..6174cbae775 100644 --- a/Gemfile +++ b/Gemfile @@ -124,7 +124,8 @@ gem 'multi_json', '~> 1.15.0' gem 'oj', '~> 3.16.0' gem 'daemons' -gem 'good_job' +gem 'delayed_job', require: false # only needed for ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper to be available in db/migrate/20240227154544_remove_delayed_jobs.rb +gem 'good_job', '~> 3.26.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 6840e70bdc1..ae94811ef53 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -445,6 +445,8 @@ GEM deckar01-task_list (2.3.4) html-pipeline (~> 2.0) declarative (0.0.20) + delayed_job (4.1.11) + activesupport (>= 3.0, < 8.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.5.1) @@ -543,7 +545,7 @@ GEM friendly_id (5.5.1) activerecord (>= 4.0.0) front_matter_parser (1.0.1) - fugit (1.9.0) + fugit (1.10.1) et-orbi (~> 1, >= 1.2.7) raabro (~> 1.4) fuubar (2.5.1) @@ -557,7 +559,7 @@ GEM i18n (>= 0.7) multi_json request_store (>= 1.0) - good_job (3.21.5) + good_job (3.26.1) activejob (>= 6.0.0) activerecord (>= 6.0.0) concurrent-ruby (>= 1.0.2) @@ -1160,6 +1162,7 @@ DEPENDENCIES date_validator (~> 0.12.0) debug deckar01-task_list (~> 2.3.1) + delayed_job disposable (~> 0.6.2) doorkeeper (~> 5.6.6) dotenv-rails @@ -1177,7 +1180,7 @@ DEPENDENCIES friendly_id (~> 5.5.0) fuubar (~> 2.5.0) gon (~> 6.4.0) - good_job + good_job (~> 3.26.1) google-apis-gmail_v1 googleauth grape (~> 2.0.0) diff --git a/db/migrate/20240227154544_remove_delayed_jobs.rb b/db/migrate/20240227154544_remove_delayed_jobs.rb index ea929b73d4e..bff7e2421e8 100644 --- a/db/migrate/20240227154544_remove_delayed_jobs.rb +++ b/db/migrate/20240227154544_remove_delayed_jobs.rb @@ -28,6 +28,35 @@ class RemoveDelayedJobs < ActiveRecord::Migration[7.1] def change + reversible do |direction| + direction.up do + tuples = execute <<~SQL + select * from delayed_jobs + where locked_by is null -- not in progress + and handler NOT LIKE '%job_class: Storages::ManageNextcloudIntegrationEventsJob%' -- no need to migrate. It will be run later with cron. + and cron is null; -- not cron scheduled + SQL + tuples.each do |tuple| + job_data = YAML.load(tuple['handler'], + permitted_classes: [ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper]) + .job_data + new_uuid = SecureRandom.uuid + good_job_record = GoodJob::BaseExecution.new + good_job_record.id = new_uuid + good_job_record.serialized_params = job_data + good_job_record.serialized_params['job_id'] = new_uuid + good_job_record.queue_name = job_data['queue_name'] + good_job_record.priority = job_data['priority'] + good_job_record.scheduled_at = job_data['scheduled_at'] + good_job_record.active_job_id = new_uuid + good_job_record.concurrency_key = nil + good_job_record.job_class = job_data['job_class'] + good_job_record.save! + end + end + direction.down {} + end + 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. From dbab61ad3f637ee79241a9af737d4628b8f41e4c Mon Sep 17 00:00:00 2001 From: Pavel Balashou Date: Tue, 5 Mar 2024 17:39:27 +0100 Subject: [PATCH 19/19] Remove delayed_job dependecy. Lock jobs. - Remove delayed_job dependecy completely from Gemfile. - Lock jobs during migration from delayed_jobs to good_jobs using SELECT FOR UPDATE. It means that potentially not shut down DJ worker will not be able to pick them up for performing during the migration process. After migration has been finished the jobs in question will be in good_jobs table and delayed_jobs table will be removed. So, duplicate execution should not happen. --- Gemfile | 1 - Gemfile.lock | 3 -- .../20240227154544_remove_delayed_jobs.rb | 41 ++++++++++++------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/Gemfile b/Gemfile index 6174cbae775..0a102b3e323 100644 --- a/Gemfile +++ b/Gemfile @@ -124,7 +124,6 @@ gem 'multi_json', '~> 1.15.0' gem 'oj', '~> 3.16.0' gem 'daemons' -gem 'delayed_job', require: false # only needed for ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper to be available in db/migrate/20240227154544_remove_delayed_jobs.rb gem 'good_job', '~> 3.26.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 ae94811ef53..8be2ae2c4ea 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -445,8 +445,6 @@ GEM deckar01-task_list (2.3.4) html-pipeline (~> 2.0) declarative (0.0.20) - delayed_job (4.1.11) - activesupport (>= 3.0, < 8.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.5.1) @@ -1162,7 +1160,6 @@ DEPENDENCIES date_validator (~> 0.12.0) debug deckar01-task_list (~> 2.3.1) - delayed_job disposable (~> 0.6.2) doorkeeper (~> 5.6.6) dotenv-rails diff --git a/db/migrate/20240227154544_remove_delayed_jobs.rb b/db/migrate/20240227154544_remove_delayed_jobs.rb index bff7e2421e8..e5bfb6413df 100644 --- a/db/migrate/20240227154544_remove_delayed_jobs.rb +++ b/db/migrate/20240227154544_remove_delayed_jobs.rb @@ -27,19 +27,32 @@ #++ class RemoveDelayedJobs < ActiveRecord::Migration[7.1] + # it is needed, because ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper + # can not be used without required delayed_job + # See https://github.com/rails/rails/blob/6f0d1ad14b92b9f5906e44740fce8b4f1c7075dc/activejob/lib/active_job/queue_adapters/delayed_job_adapter.rb + class JobWrapperDeserializationMock + attr_accessor :job_data + + def initialize(job_data) + @job_data = job_data + end + end + def change reversible do |direction| direction.up do tuples = execute <<~SQL select * from delayed_jobs - where locked_by is null -- not in progress - and handler NOT LIKE '%job_class: Storages::ManageNextcloudIntegrationEventsJob%' -- no need to migrate. It will be run later with cron. - and cron is null; -- not cron scheduled + where locked_by is null -- not in progress + and handler NOT LIKE '%job_class: Storages::ManageNextcloudIntegrationEventsJob%' -- no need to migrate. It will be run later with cron. + and cron is null -- not cron schedule + FOR UPDATE; -- to prevent potentialy running delayed_job process working on these jobs(delayed_job uses SELECT FOR UPDATE to get workable jobs) SQL tuples.each do |tuple| - job_data = YAML.load(tuple['handler'], - permitted_classes: [ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper]) - .job_data + handler = tuple['handler'].gsub('ruby/object:ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper', + "ruby/object:#{RemoveDelayedJobs::JobWrapperDeserializationMock.name}") + job_data = YAML.load(handler, permitted_classes: [RemoveDelayedJobs::JobWrapperDeserializationMock]) + .job_data new_uuid = SecureRandom.uuid good_job_record = GoodJob::BaseExecution.new good_job_record.id = new_uuid @@ -58,14 +71,14 @@ class RemoveDelayedJobs < ActiveRecord::Migration[7.1] end 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.integer :priority, default: 0 + t.integer :attempts, default: 0 + t.text :handler + t.text :last_error + t.datetime :run_at + t.datetime :locked_at + t.datetime :failed_at + t.string :locked_by t.timestamps null: true t.string :queue t.string :cron