From 22ae46445a8674eb49b52b8cd480e4e53ace0cf8 Mon Sep 17 00:00:00 2001 From: Eric Guo Date: Thu, 7 Mar 2024 11:38:34 +0800 Subject: [PATCH] Revert "Merge pull request #12998 from opf/task/48717-replace-delayedjob" --- Gemfile | 3 +- Gemfile.lock | 57 +++++-- Procfile.dev | 2 +- .../settings/working_days_params_contract.rb | 4 +- app/seeders/root_seeder.rb | 6 +- app/workers/application_job.rb | 4 - .../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 +- .../workers/cron/cron_job.rb | 68 ++++---- 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 +- app/workers/projects/reorder_hierarchy_job.rb | 73 ++++++++ .../apply_working_days_change_job.rb | 1 + bin/check-worker-liveness | 4 +- bin/check-worker-readiness | 2 +- bin/delayed_job | 5 + bin/setup_dev | 2 +- config/application.rb | 15 +- config/constants/settings/definition.rb | 37 +--- config/initializers/cronjobs.rb | 82 ++------- config/initializers/database_pool_size.rb | 28 --- config/initializers/delayed_job_config.rb | 55 ++++++ config/initializers/health_checks.rb | 120 ++++++++++--- .../initializers/time_with_zone_as_json.rb | 6 +- config/routes.rb | 3 - .../20201125121949_remove_renamed_cron_job.rb | 4 +- ...20220202140507_reorder_project_children.rb | 49 +----- ...4112533_remove_notification_cleanup_job.rb | 7 +- ...6132134_bigint_primary_and_foreign_keys.rb | 2 +- ...105073117_remove_renamed_date_alert_job.rb | 4 +- 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 ----- .../20240227154544_remove_delayed_jobs.rb | 89 ---------- 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 +- .../installation-faq/README.md | 2 +- .../operation/monitoring/README.md | 6 +- .../integrations/nextcloud/README.md | 4 +- .../work-packages/work-packages-faq/README.md | 2 +- .../delayed_job_adapter.rb} | 30 ++-- .../delivery_job.rb} | 23 ++- 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 +- .../app/models/job_status/status.rb | 30 +--- .../job_status/application_job_with_status.rb | 14 +- .../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 | 4 +- .../manage_nextcloud_integration_cron_job.rb | 26 +-- ...manage_nextcloud_integration_events_job.rb | 63 +++++++ .../manage_nextcloud_integration_job.rb | 109 ------------ .../manage_nextcloud_integration_job_mixin.rb | 66 ++++++++ .../20231009135807_remove_renamed_cronjobs.rb | 4 +- .../lib/open_project/storages/engine.rb | 27 ++- ...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 | 160 ------------------ .../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 ++++--- .../reorder_children_job_integration_spec.rb} | 6 +- .../apply_working_days_change_job_spec.rb | 22 +++ 94 files changed, 1241 insertions(+), 1137 deletions(-) create mode 100644 app/workers/concerns/scheduled_job.rb rename lib/open_project/health_checks/puma_check.rb => app/workers/cron/cron_job.rb (51%) create mode 100644 app/workers/projects/reorder_hierarchy_job.rb create mode 100755 bin/delayed_job create mode 100644 config/initializers/delayed_job_config.rb rename db/migrate/20240229133250_rename_delayed_job_statuses.rb => config/initializers/time_with_zone_as_json.rb (90%) delete mode 100644 db/migrate/20240123151246_create_good_jobs.rb delete mode 100644 db/migrate/20240123151247_create_good_job_settings.rb delete mode 100644 db/migrate/20240123151248_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb delete mode 100644 db/migrate/20240123151249_create_good_job_batches.rb delete mode 100644 db/migrate/20240123151250_create_good_job_executions.rb delete mode 100644 db/migrate/20240123151251_create_good_jobs_error_event.rb delete mode 100644 db/migrate/20240123151252_recreate_good_job_cron_indexes_with_conditional.rb delete mode 100644 db/migrate/20240227154544_remove_delayed_jobs.rb rename lib/open_project/{health_checks/good_job_backed_up_check.rb => patches/delayed_job_adapter.rb} (66%) rename lib/open_project/{health_checks/good_job_check.rb => patches/delivery_job.rb} (76%) create mode 100644 lib/tasks/delayed_job.rake rename lib/open_project/health_checks/smtp_check.rb => modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb (74%) create mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb delete mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb create mode 100644 modules/storages/app/workers/storages/manage_nextcloud_integration_job_mixin.rb create mode 100644 modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb create mode 100644 modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb delete mode 100644 modules/storages/spec/workers/storages/manage_nextcloud_integration_job_spec.rb create mode 100644 spec/workers/non_existing_job_class_spec.rb rename spec/{migrations/reorder_project_children_spec.rb => workers/projects/reorder_children_job_integration_spec.rb} (89%) diff --git a/Gemfile b/Gemfile index f66f1c74f3ad..d14b20979f89 100644 --- a/Gemfile +++ b/Gemfile @@ -125,7 +125,8 @@ gem "multi_json", "~> 1.15.0" gem "oj", "~> 3.16.0" gem "daemons" -gem "good_job", "= 3.26.2" # update should be done manually in sync with saas-openproject version. +gem "delayed_cron_job", "~> 0.9.0" +gem "delayed_job_active_record", "~> 4.1.5" gem "rack-protection", "~> 3.2.0" diff --git a/Gemfile.lock b/Gemfile.lock index f5c7f8c901bc..806f8fb9e419 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -496,8 +496,10 @@ GEM descendants_tracker (~> 0.0.1) color_conversion (0.1.1) colored2 (4.0.0) - commonmarker (1.0.4) - rb_sys (~> 0.9) + commonmarker (1.0.4-aarch64-linux) + commonmarker (1.0.4-arm64-darwin) + commonmarker (1.0.4-x86_64-darwin) + commonmarker (1.0.4-x86_64-linux) compare-xml (0.66) nokogiri (~> 1.8) concurrent-ruby (1.2.3) @@ -531,6 +533,13 @@ GEM deckar01-task_list (2.3.4) html-pipeline (~> 2.0) 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) @@ -648,13 +657,6 @@ GEM i18n (>= 0.7) multi_json request_store (>= 1.0) - good_job (3.26.2) - 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.14.1) addressable (~> 2.5, >= 2.5.1) googleauth (~> 1.9) @@ -667,7 +669,10 @@ GEM google-apis-core (>= 0.14.0, < 2.a) google-cloud-env (2.1.1) faraday (>= 1.0, < 3.a) - google-protobuf (3.25.3) + google-protobuf (3.25.3-aarch64-linux) + google-protobuf (3.25.3-arm64-darwin) + google-protobuf (3.25.3-x86_64-darwin) + google-protobuf (3.25.3-x86_64-linux) googleapis-common-protos-types (1.14.0) google-protobuf (~> 3.18) googleauth (1.11.0) @@ -688,7 +693,16 @@ GEM grape rack gravatar_image_tag (1.2.0) - grpc (1.62.0) + grpc (1.62.0-aarch64-linux) + google-protobuf (~> 3.25) + googleapis-common-protos-types (~> 1.0) + grpc (1.62.0-arm64-darwin) + google-protobuf (~> 3.25) + googleapis-common-protos-types (~> 1.0) + grpc (1.62.0-x86_64-darwin) + google-protobuf (~> 3.25) + googleapis-common-protos-types (~> 1.0) + grpc (1.62.0-x86_64-linux) google-protobuf (~> 3.25) googleapis-common-protos-types (~> 1.0) grpc-tools (1.62.0) @@ -829,7 +843,6 @@ GEM mime-types-data (3.2024.0305) mini_magick (4.12.0) mini_mime (1.1.5) - mini_portile2 (2.8.5) minitest (5.22.3) msgpack (1.7.2) multi_json (1.15.0) @@ -857,8 +870,13 @@ GEM net-protocol net-ssh (7.2.1) nio4r (2.7.0) - nokogiri (1.16.3) - mini_portile2 (~> 2.8.2) + nokogiri (1.16.3-aarch64-linux) + racc (~> 1.4) + nokogiri (1.16.3-arm64-darwin) + racc (~> 1.4) + nokogiri (1.16.3-x86_64-darwin) + racc (~> 1.4) + nokogiri (1.16.3-x86_64-linux) racc (~> 1.4) oj (3.16.3) bigdecimal (>= 3.0) @@ -1028,7 +1046,6 @@ GEM rb-fsevent (0.11.2) rb-inotify (0.10.1) ffi (~> 1.0) - rb_sys (0.9.90) rbtree3 (0.7.1) rdoc (6.6.3.1) psych (>= 4.0.0) @@ -1158,7 +1175,7 @@ GEM unf (~> 0.1.4) slop (4.10.1) smart_properties (1.17.0) - sorbet-runtime (0.5.11295) + sorbet-runtime (0.5.11301) spreadsheet (1.3.1) bigdecimal ruby-ole @@ -1279,7 +1296,10 @@ GEM zeitwerk (2.6.13) PLATFORMS - ruby + aarch64-linux + arm64-darwin + x86_64-darwin + x86_64-linux DEPENDENCIES actionpack-xml_parser (~> 2.0.0) @@ -1331,6 +1351,8 @@ DEPENDENCIES debug debugbar 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 @@ -1349,7 +1371,6 @@ DEPENDENCIES friendly_id (~> 5.5.0) fuubar (~> 2.5.0) gon (~> 6.4.0) - good_job (= 3.26.2) google-apis-gmail_v1 googleauth grape (~> 2.0.0) diff --git a/Procfile.dev b/Procfile.dev index 4b67b9243799..a18244ee467c 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: yarn run serve -worker: bundle exec good_job start +worker: bundle exec rake jobs:work diff --git a/app/contracts/settings/working_days_params_contract.rb b/app/contracts/settings/working_days_params_contract.rb index cd0faf86d21e..c19f63062f85 100644 --- a/app/contracts/settings/working_days_params_contract.rb +++ b/app/contracts/settings/working_days_params_contract.rb @@ -42,9 +42,7 @@ def working_days_are_present end def unique_job - if GoodJob::Job - .where(finished_at: nil) - .exists?(job_class: WorkPackages::ApplyWorkingDaysChangeJob.name) + if WorkPackages::ApplyWorkingDaysChangeJob.scheduled? 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 840fe59b85e1..21277eb3e103 100644 --- a/app/seeders/root_seeder.rb +++ b/app/seeders/root_seeder.rb @@ -113,13 +113,13 @@ def prepare_seed! ActionMailer::Base.perform_deliveries = false # Avoid asynchronous DeliverWorkPackageCreatedJob - previous_execution_mode = Rails.configuration.good_job.execution_mode - Rails.configuration.good_job.execution_mode = :inline + previous_delay_jobs = Delayed::Worker.delay_jobs + Delayed::Worker.delay_jobs = false yield ensure ActionMailer::Base.perform_deliveries = previous_perform_deliveries - Rails.configuration.good_job.execution_mode = previous_execution_mode + Delayed::Worker.delay_jobs = previous_delay_jobs end def seed_basic_data diff --git a/app/workers/application_job.rb b/app/workers/application_job.rb index 44643a51945d..1c904d15cee8 100644 --- a/app/workers/application_job.rb +++ b/app/workers/application_job.rb @@ -92,10 +92,6 @@ def reload_mailer_settings! Setting.reload_mailer_settings! end - def 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 5b0dc599ef53..edb45c306123 100644 --- a/app/workers/attachments/cleanup_uncontainered_job.rb +++ b/app/workers/attachments/cleanup_uncontainered_job.rb @@ -26,9 +26,12 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Attachments::CleanupUncontaineredJob < ApplicationJob +class Attachments::CleanupUncontaineredJob < Cron::CronJob queue_with_priority :low + # runs at 10:03 pm + self.cron_expression = "03 22 * * *" + def perform Attachment .where(container: nil) @@ -47,7 +50,7 @@ def too_old attachment_table = Attachment.arel_table attachment_table[:created_at] - .lteq(Time.zone.now - OpenProject::Configuration.attachments_grace_period.minutes) + .lteq(Time.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 new file mode 100644 index 000000000000..5dcda3710023 --- /dev/null +++ b/app/workers/concerns/scheduled_job.rb @@ -0,0 +1,41 @@ +# 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 8c50a5b8e531..c96771952c5e 100644 --- a/app/workers/cron/clear_old_sessions_job.rb +++ b/app/workers/cron/clear_old_sessions_job.rb @@ -27,9 +27,12 @@ #++ module Cron - class ClearOldSessionsJob < ApplicationJob + class ClearOldSessionsJob < CronJob 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 17d78e6cfd2d..c2aafb2e4323 100644 --- a/app/workers/cron/clear_tmp_cache_job.rb +++ b/app/workers/cron/clear_tmp_cache_job.rb @@ -27,9 +27,12 @@ #++ module Cron - class ClearTmpCacheJob < ApplicationJob + class ClearTmpCacheJob < CronJob 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 5086ea9dc3e9..0eadb3984e31 100644 --- a/app/workers/cron/clear_uploaded_files_job.rb +++ b/app/workers/cron/clear_uploaded_files_job.rb @@ -27,9 +27,12 @@ #++ module Cron - class ClearUploadedFilesJob < ApplicationJob + class ClearUploadedFilesJob < CronJob include ::RakeJob + # Runs 23pm fridays + self.cron_expression = "0 23 * * 5" + def perform super("attachments:clear") end diff --git a/lib/open_project/health_checks/puma_check.rb b/app/workers/cron/cron_job.rb similarity index 51% rename from lib/open_project/health_checks/puma_check.rb rename to app/workers/cron/cron_job.rb index 583a4a00817d..a2afe9f260d6 100644 --- a/lib/open_project/health_checks/puma_check.rb +++ b/app/workers/cron/cron_job.rb @@ -23,55 +23,53 @@ # 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. +# See COPYRIGHT and LICENSE files 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 +module Cron + class CronJob < ApplicationJob + class_attribute :cron_expression + + # List of registered jobs, requires eager load in dev(!) + class_attribute :registered_jobs, default: [] - def check - stats = self.stats + include ScheduledJob - return mark_message "N/A as Puma is not used." if stats.nil? + 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) - if stats[:running] > 0 - mark_message "Puma is running" - else - mark_failure - mark_message "Puma is not running" + registered_jobs << clz end + end - if stats[:backlog] < threshold - mark_message "Backlog ok" - else - mark_failure - mark_message "Backlog congested" + def schedule_registered_jobs! + registered_jobs.each do |job_class| + job_class.ensure_scheduled! end end - def stats - return nil unless applicable? + ## + # Ensure the job is scheduled unless it is already + def ensure_scheduled! + # Ensure scheduled only once + return if scheduled? - server = Puma::Server.current - return nil if server.nil? + Rails.logger.info { "Scheduling #{name} recurrent background job." } + set(cron: cron_expression).perform_later + end - { - backlog: server.backlog || 0, - running: server.running || 0, - pool_capacity: server.pool_capacity || 0, - max_threads: server.max_threads || 0 - } + ## + # Remove the scheduled job, if any + def remove + delayed_job&.destroy end - def applicable? - !!@applicable + def delayed_job + delayed_job_query.first end end end diff --git a/app/workers/ldap/synchronization_job.rb b/app/workers/ldap/synchronization_job.rb index a100d5b87898..97416f1c31d2 100644 --- a/app/workers/ldap/synchronization_job.rb +++ b/app/workers/ldap/synchronization_job.rb @@ -27,7 +27,10 @@ #++ module Ldap - class SynchronizationJob < ApplicationJob + class SynchronizationJob < ::Cron::CronJob + # Run once per night at 11:30pm + self.cron_expression = "30 23 * * *" + 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 525a0473bddf..8c7fee44dc34 100644 --- a/app/workers/notifications/schedule_date_alerts_notifications_job.rb +++ b/app/workers/notifications/schedule_date_alerts_notifications_job.rb @@ -28,11 +28,15 @@ module Notifications # Creates date alert jobs for users whose local time is 1:00 am. - class ScheduleDateAlertsNotificationsJob < ApplicationJob + 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 * * * *" + def perform return unless EnterpriseToken.allows_to?(:date_alerts) - Service.new(times_from_scheduled_to_execution).call + service = Service.new(times_from_scheduled_to_execution) + service.call end # Returns times from scheduled execution time to current time in 15 minutes @@ -53,7 +57,7 @@ def times_from_scheduled_to_execution end def scheduled_time - job_scheduled_at.then { |t| t.change(min: t.min / 15 * 15) } + self.class.delayed_job.run_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 0c17199d1c79..64ac1e730925 100644 --- a/app/workers/notifications/schedule_reminder_mails_job.rb +++ b/app/workers/notifications/schedule_reminder_mails_job.rb @@ -27,13 +27,18 @@ #++ module Notifications - class ScheduleReminderMailsJob < ApplicationJob + class ScheduleReminderMailsJob < Cron::CronJob + # runs every quarter of an hour, so 00:00, 00:15... + self.cron_expression = "*/15 * * * *" + def perform - User.having_reminder_mail_to_send(job_scheduled_at) - .pluck(:id) - .each do |user_id| + User.having_reminder_mail_to_send(run_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 f08ea238262a..e68f6ed6e6fb 100644 --- a/app/workers/oauth/cleanup_job.rb +++ b/app/workers/oauth/cleanup_job.rb @@ -27,9 +27,12 @@ #++ module OAuth - class CleanupJob < ApplicationJob + class CleanupJob < ::Cron::CronJob 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 c593ab4c93e4..aa383c84980a 100644 --- a/app/workers/paper_trail_audits/cleanup_job.rb +++ b/app/workers/paper_trail_audits/cleanup_job.rb @@ -27,7 +27,10 @@ #++ module PaperTrailAudits - class CleanupJob < ApplicationJob + class CleanupJob < ::Cron::CronJob + # runs at 4:03 on Saturday + self.cron_expression = "3 4 * * 6" + # Clean any paper trails older than 60 days def perform ::PaperTrailAudit diff --git a/app/workers/projects/reorder_hierarchy_job.rb b/app/workers/projects/reorder_hierarchy_job.rb new file mode 100644 index 000000000000..eafa3ed0efd5 --- /dev/null +++ b/app/workers/projects/reorder_hierarchy_job.rb @@ -0,0 +1,73 @@ +#-- 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/app/workers/work_packages/apply_working_days_change_job.rb b/app/workers/work_packages/apply_working_days_change_job.rb index 70c5051f0e64..e574e08ab4e4 100644 --- a/app/workers/work_packages/apply_working_days_change_job.rb +++ b/app/workers/work_packages/apply_working_days_change_job.rb @@ -28,6 +28,7 @@ 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-liveness b/bin/check-worker-liveness index 7d02aff9ff47..fe248d9dfb66 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 good_jobs where finished_at is null and scheduled_at < (now() - interval '15 minutes');" | tail -n +3 | head -n1 | tr -d ' '` +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 ' '` 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 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` + 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` 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/bin/check-worker-readiness b/bin/check-worker-readiness index 1f9beb41ac30..84fbd973fe48 100755 --- a/bin/check-worker-readiness +++ b/bin/check-worker-readiness @@ -1,6 +1,6 @@ #!/bin/bash -if ps aux | grep 'good_job start' | grep -v grep; then +if ps aux | grep 'rake jobs:work' | grep -v grep; then echo "background worker running" exit 0 fi diff --git a/bin/delayed_job b/bin/delayed_job new file mode 100755 index 000000000000..edf195985f69 --- /dev/null +++ b/bin/delayed_job @@ -0,0 +1,5 @@ +#!/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 7236581a7fc5..b69721264a9c 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 '- Good Job worker: `RAILS_ENV=development bundle exec good_job start`' +echo '- Delayed Job worker: `RAILS_ENV=development bin/rails jobs:work`' 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 9bbaa0e1a90d..7a9f7c66d01d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -212,20 +212,7 @@ class Application < Rails::Application # 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 = :good_job - - config.good_job.retry_on_unhandled_error = false - # 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.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.active_job.queue_adapter = :delayed_job config.action_controller.asset_host = OpenProject::Configuration::AssetHost.value diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index 7593b7b1cb72..04b34cb176df 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -492,36 +492,6 @@ class Definition description: "Forced page size for manually sorted work package views", default: 250 }, - good_job_queues: { - description: "", - format: :string, - writable: false, - default: "*" - }, - good_job_max_threads: { - description: "", - format: :integer, - writable: false, - default: 20 - }, - good_job_max_cache: { - description: "", - format: :integer, - writable: false, - default: 10_000 - }, - good_job_enable_cron: { - description: "", - format: :boolean, - writable: false, - default: true - }, - good_job_cleanup_preserved_jobs_before_seconds_ago: { - description: "", - format: :integer, - writable: false, - default: 7.days - }, host_name: { default: "localhost:3000" }, @@ -531,6 +501,13 @@ class Definition 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/cronjobs.rb b/config/initializers/cronjobs.rb index af7c61e8d6f7..6dda0beb89ed 100644 --- a/config/initializers/cronjobs.rb +++ b/config/initializers/cronjobs.rb @@ -1,71 +1,15 @@ -#-- 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. -#++ - # Register "Cron-like jobs" -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.name - }, - "Cron::ClearTmpCacheJob": { - cron: "45 2 * * 7", # runs at 02:45 sundays - class: Cron::ClearTmpCacheJob.name - }, - "Cron::ClearUploadedFilesJob": { - cron: "0 23 * * 5", # runs 23:00 fridays - class: Cron::ClearUploadedFilesJob.name - }, - "OAuth::CleanupJob": { - cron: "52 1 * * *", - class: OAuth::CleanupJob.name - }, - "PaperTrailAudits::CleanupJob": { - cron: "3 4 * * 6", - class: PaperTrailAudits::CleanupJob.name - }, - "Attachments::CleanupUncontaineredJob": { - cron: "03 22 * * *", - class: Attachments::CleanupUncontaineredJob.name - }, - "Notifications::ScheduleDateAlertsNotificationsJob": { - cron: "*/15 * * * *", - class: Notifications::ScheduleDateAlertsNotificationsJob.name - }, - "Notifications::ScheduleReminderMailsJob": { - cron: "*/15 * * * *", - class: Notifications::ScheduleReminderMailsJob.name - }, - "Ldap::SynchronizationJob": { - cron: "30 23 * * *", - class: Ldap::SynchronizationJob.name - } - } - ) + +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 end diff --git a/config/initializers/database_pool_size.rb b/config/initializers/database_pool_size.rb index f561765d1abb..24b4a8e12237 100644 --- a/config/initializers/database_pool_size.rb +++ b/config/initializers/database_pool_size.rb @@ -1,31 +1,3 @@ -#-- 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 new file mode 100644 index 000000000000..197df4b3c42c --- /dev/null +++ b/config/initializers/delayed_job_config.rb @@ -0,0 +1,55 @@ +#-- 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/health_checks.rb b/config/initializers/health_checks.rb index 7f23b0f09156..8558794c9a43 100644 --- a/config/initializers/health_checks.rb +++ b/config/initializers/health_checks.rb @@ -1,37 +1,107 @@ require "ok_computer/ok_computer_controller" -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 +class DelayedJobNeverRanCheck < OkComputer::Check + attr_reader :threshold - OkComputer::Registry.register "puma", OpenProject::HealthChecks::PumaCheck.new + def initialize(minute_threshold) + @threshold = minute_threshold.to_i + end - # Make dj backed up optional due to bursts - OkComputer.make_optional %w(worker_backed_up puma) + def check + never_ran = Delayed::Job.where("run_at < ?", threshold.minutes.ago).count - # 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 + if never_ran.zero? + mark_message "All previous jobs have completed within the past #{threshold} minutes." + else + mark_failure + mark_message "#{never_ran} jobs waiting to be executed for more than #{threshold} minutes" 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 - # 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 + 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 - # Check if authentication required - authentication_password = OpenProject::Configuration.health_checks_authentication_password - if authentication_password.present? - OkComputer.require_authentication("health_checks", authentication_password) + 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 + +# 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) + +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) + +# 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 :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 :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/db/migrate/20240229133250_rename_delayed_job_statuses.rb b/config/initializers/time_with_zone_as_json.rb similarity index 90% rename from db/migrate/20240229133250_rename_delayed_job_statuses.rb rename to config/initializers/time_with_zone_as_json.rb index 43ffd4dd4577..781baa74af1c 100644 --- a/db/migrate/20240229133250_rename_delayed_job_statuses.rb +++ b/config/initializers/time_with_zone_as_json.rb @@ -26,8 +26,8 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class RenameDelayedJobStatuses < ActiveRecord::Migration[7.1] - def change - rename_table :delayed_job_statuses, :job_statuses +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 4c2c38ae5659..1b5878f9f02c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -658,8 +658,5 @@ mount Lookbook::Engine, at: "/lookbook" end - if Rails.env.development? - mount GoodJob::Engine => "good_job" - end get "/wechat/auth/callback", to: "auth_wechat#index", as: :wechat_auth_callback end diff --git a/db/migrate/20201125121949_remove_renamed_cron_job.rb b/db/migrate/20201125121949_remove_renamed_cron_job.rb index be642846ee2a..e99edca48b6c 100644 --- a/db/migrate/20201125121949_remove_renamed_cron_job.rb +++ b/db/migrate/20201125121949_remove_renamed_cron_job.rb @@ -31,6 +31,8 @@ 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("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Cron::ClearOldJobStatusJob%'") + 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 index 760467ec60f9..d964d0d077e8 100644 --- a/db/migrate/20220202140507_reorder_project_children.rb +++ b/db/migrate/20220202140507_reorder_project_children.rb @@ -27,54 +27,7 @@ #++ class ReorderProjectChildren < ActiveRecord::Migration[6.1] - class ProjectMigration < ApplicationRecord - include ::Projects::Hierarchy - self.table_name = "projects" - end - def up - 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 + ::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 index a041d7bdd07a..0b012f2114c1 100644 --- a/db/migrate/20220804112533_remove_notification_cleanup_job.rb +++ b/db/migrate/20220804112533_remove_notification_cleanup_job.rb @@ -28,7 +28,12 @@ class RemoveNotificationCleanupJob < ActiveRecord::Migration[7.0] def up - execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CleanupJob%'") + # 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 diff --git a/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb b/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb index 795c3473c559..c641d892044f 100644 --- a/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb +++ b/db/migrate/20221026132134_bigint_primary_and_foreign_keys.rb @@ -60,8 +60,8 @@ 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], - :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/db/migrate/20230105073117_remove_renamed_date_alert_job.rb b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb index e59bc347aa68..3e3a67445720 100644 --- a/db/migrate/20230105073117_remove_renamed_date_alert_job.rb +++ b/db/migrate/20230105073117_remove_renamed_date_alert_job.rb @@ -30,6 +30,8 @@ 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("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: Notifications::CreateDateAlertsNotificationsJob%'") + 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 deleted file mode 100644 index 83cb7893fae8..000000000000 --- a/db/migrate/20240123151246_create_good_jobs.rb +++ /dev/null @@ -1,40 +0,0 @@ -# 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 deleted file mode 100644 index fa3540174a62..000000000000 --- a/db/migrate/20240123151247_create_good_job_settings.rb +++ /dev/null @@ -1,20 +0,0 @@ -# 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 deleted file mode 100644 index 7b9dbab3816f..000000000000 --- a/db/migrate/20240123151248_create_index_good_jobs_jobs_on_priority_created_at_when_unfinished.rb +++ /dev/null @@ -1,19 +0,0 @@ -# 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 deleted file mode 100644 index ac6c5b37e982..000000000000 --- a/db/migrate/20240123151249_create_good_job_batches.rb +++ /dev/null @@ -1,35 +0,0 @@ -# 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 deleted file mode 100644 index 32723220ccec..000000000000 --- a/db/migrate/20240123151250_create_good_job_executions.rb +++ /dev/null @@ -1,33 +0,0 @@ -# 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 deleted file mode 100644 index b07e0f14e7f1..000000000000 --- a/db/migrate/20240123151251_create_good_jobs_error_event.rb +++ /dev/null @@ -1,16 +0,0 @@ -# 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 deleted file mode 100644 index aff2d4eae9a8..000000000000 --- a/db/migrate/20240123151252_recreate_good_job_cron_indexes_with_conditional.rb +++ /dev/null @@ -1,45 +0,0 @@ -# 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/db/migrate/20240227154544_remove_delayed_jobs.rb b/db/migrate/20240227154544_remove_delayed_jobs.rb deleted file mode 100644 index cc47c1c9d066..000000000000 --- a/db/migrate/20240227154544_remove_delayed_jobs.rb +++ /dev/null @@ -1,89 +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 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 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| - 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 - 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 - 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 - - t.index %i[priority run_at], name: "delayed_jobs_priority" - end - end -end diff --git a/docker-compose.yml b/docker-compose.yml index 6ee045bc57e2..e414de20bc5b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -66,7 +66,7 @@ services: worker: <<: *backend - command: bundle exec good_job start + command: bundle exec rake jobs:work depends_on: - db - cache diff --git a/docker/prod/bimworker b/docker/prod/bimworker index 01b7766a1b57..a63e2777dbc1 100755 --- a/docker/prod/bimworker +++ b/docker/prod/bimworker @@ -1,3 +1,3 @@ #!/bin/bash -e export QUEUE=bim,ifc_conversion -exec bundle exec good_job start +exec bundle exec rake jobs:work diff --git a/docker/prod/worker b/docker/prod/worker index 87130302b193..bc4d194cbaaf 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 good_job start +QUIET=true bundle exec rake jobs:work diff --git a/docs/development/development-environment-docker/README.md b/docs/development/development-environment-docker/README.md index dbfe1b08f6cb..ffa759efbe7a 100644 --- a/docs/development/development-environment-docker/README.md +++ b/docs/development/development-environment-docker/README.md @@ -141,8 +141,15 @@ 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 8a6e2a77c9bd..c484e6ea84f3 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 bundle exec good_job start +RAILS_ENV=development bin/rails jobs:work ``` This will start a Delayed::Job worker to perform asynchronous jobs like sending emails. @@ -298,6 +298,12 @@ 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 52ed199ef6e6..25c551a6adb1 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 bundle exec good_job start +RAILS_ENV=development bin/rails jobs:work ``` This will start a Delayed::Job worker to perform asynchronous jobs like sending emails. @@ -310,6 +310,12 @@ 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/installation-and-operations/installation-faq/README.md b/docs/installation-and-operations/installation-faq/README.md index a086e93eec30..802e11c602c6 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 "worker" and "worker_backed_up". 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 "delayed_jobs_backed_up" and "delayed_jobs_never_ran". 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 145dba04978c..40d740b33464 100644 --- a/docs/installation-and-operations/operation/monitoring/README.md +++ b/docs/installation-and-operations/operation/monitoring/README.md @@ -124,10 +124,8 @@ 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/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/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/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 aa4ea7617d89..1fc9d5a245a0 100644 --- a/docs/system-admin-guide/integrations/nextcloud/README.md +++ b/docs/system-admin-guide/integrations/nextcloud/README.md @@ -348,9 +348,9 @@ 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 bundle exec good_job start` + The result should show lines containing `bundle exec rake jobs:work` - 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.` + 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` 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 fa3a7715a8db..5ad656c8c6d3 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 "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. +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. 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/lib/open_project/health_checks/good_job_backed_up_check.rb b/lib/open_project/patches/delayed_job_adapter.rb similarity index 66% rename from lib/open_project/health_checks/good_job_backed_up_check.rb rename to lib/open_project/patches/delayed_job_adapter.rb index bf9de01a8932..1a3af2f472b1 100644 --- a/lib/open_project/health_checks/good_job_backed_up_check.rb +++ b/lib/open_project/patches/delayed_job_adapter.rb @@ -23,26 +23,26 @@ # 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. +# See COPYRIGHT and LICENSE files 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 +# This patch adds our job status extension to background jobs carried out when mailing with +# perform_later. - 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." +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/health_checks/good_job_check.rb b/lib/open_project/patches/delivery_job.rb similarity index 76% rename from lib/open_project/health_checks/good_job_check.rb rename to lib/open_project/patches/delivery_job.rb index 9a996572bd9a..99f29caa0472 100644 --- a/lib/open_project/health_checks/good_job_check.rb +++ b/lib/open_project/patches/delivery_job.rb @@ -23,21 +23,18 @@ # 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. +# See COPYRIGHT and LICENSE files 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 +# 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 8c70a1b23305..92fa09d3de5b 100644 --- a/lib/open_project/plugins/acts_as_op_engine.rb +++ b/lib/open_project/plugins/acts_as_op_engine.rb @@ -293,9 +293,13 @@ def activity_provider(event_type, options = {}) OpenProject::Activity.register(event_type, options) end - def add_cron_jobs(&block) + ## + # Register a "cron"-like background job + def add_cron_jobs config.to_prepare do - Rails.application.config.good_job.cron.merge!(block.call) + Array(yield).each do |clz| + ::Cron::CronJob.register!(clz.is_a?(Class) ? clz : clz.to_s.constantize) + end end end diff --git a/lib/tasks/cron.rake b/lib/tasks/cron.rake index c821efa35c31..a690288c4571 100644 --- a/lib/tasks/cron.rake +++ b/lib/tasks/cron.rake @@ -27,8 +27,15 @@ #++ namespace "openproject:cron" do - desc "Ensure the cron-like background jobs are properly unscheduled if needed" + 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" task schedule: [:environment] do - Storages::ManageNextcloudIntegrationJob.disable_cron_job_if_needed + Cron::CronJob.schedule_registered_jobs! end end diff --git a/lib/tasks/delayed_job.rake b/lib/tasks/delayed_job.rake new file mode 100644 index 000000000000..546feeefa0d4 --- /dev/null +++ b/lib/tasks/delayed_job.rake @@ -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 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 242892c5e321..313038b01f9a 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,9 +27,12 @@ #++ module Cron - class ClearOldPullRequestsJob < ApplicationJob + class ClearOldPullRequestsJob < CronJob 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 75950c7d335a..5a66fc330c6e 100644 --- a/modules/github_integration/lib/open_project/github_integration/engine.rb +++ b/modules/github_integration/lib/open_project/github_integration/engine.rb @@ -85,13 +85,9 @@ class Engine < ::Rails::Engine mount ::API::V3::GithubPullRequests::GithubPullRequestsAPI end - add_cron_jobs do - { - "Cron::ClearOldPullRequestsJob": { - cron: "25 1 * * *", # runs at 1:25 nightly - class: ::Cron::ClearOldPullRequestsJob.name - } - } + config.to_prepare do + # Register the cron job to clean up old github pull requests + ::Cron::CronJob.register! ::Cron::ClearOldPullRequestsJob end 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 1372b5b2aa5a..19d8cf487c72 100644 --- a/modules/job_status/app/models/job_status/status.rb +++ b/modules/job_status/app/models/job_status/status.rb @@ -1,34 +1,6 @@ -#-- 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 = "job_statuses" + self.table_name = "delayed_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 e0c6e128fce0..e754700cde23 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,7 +27,8 @@ #++ module JobStatus module ApplicationJobWithStatus - # Backgroun jobs can have a status JobStatus::Status + # Delayed jobs can have a status: + # Delayed::Job::Status # which is related to the job via a reference which is an AR model instance. def status_reference nil @@ -60,6 +61,8 @@ def job_status ## # 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? @@ -74,16 +77,7 @@ def upsert_status(status:, **args) resource.attributes = build_status_attributes(args.merge(status:)) end - # 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. - # 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 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 5069d9b39ea6..43a28b7b21e4 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,12 +28,15 @@ module JobStatus module Cron - class ClearOldJobStatusJob < ApplicationJob + class ClearOldJobStatusJob < ::Cron::CronJob + # runs at 4:15 nightly + self.cron_expression = "15 4 * * *" + RETENTION_PERIOD = 2.days.freeze def perform ::JobStatus::Status - .where(::JobStatus::Status.arel_table[:updated_at].lteq(Time.zone.now - RETENTION_PERIOD)) + .where(::JobStatus::Status.arel_table[:updated_at].lteq(Time.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 85bd828a6456..fb17dfb3c068 100644 --- a/modules/job_status/lib/open_project/job_status/engine.rb +++ b/modules/job_status/lib/open_project/job_status/engine.rb @@ -46,16 +46,10 @@ class Engine < ::Rails::Engine "#{root}/job_statuses/#{uuid}" end - add_cron_jobs do - { - "JobStatus::Cron::ClearOldJobStatusJob": { - cron: "15 4 * * *", # runs at 4:15 nightly - class: ::JobStatus::Cron::ClearOldJobStatusJob.name - } - } - end - config.to_prepare do + # Register the cron job to clear statuses periodically + ::Cron::CronJob.register! ::JobStatus::Cron::ClearOldJobStatusJob + # 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 afde20b08a36..4be25b216419 100644 --- a/modules/ldap_groups/app/workers/ldap_groups/synchronization_job.rb +++ b/modules/ldap_groups/app/workers/ldap_groups/synchronization_job.rb @@ -27,7 +27,10 @@ #++ module LdapGroups - class SynchronizationJob < ApplicationJob + class SynchronizationJob < ::Cron::CronJob + # Run every 30 minutes + self.cron_expression = "*/30 * * * *" + 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 50e9b44d4fdb..c2e61c2835bf 100644 --- a/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb +++ b/modules/ldap_groups/lib/open_project/ldap_groups/engine.rb @@ -19,14 +19,7 @@ class Engine < ::Rails::Engine enterprise_feature: "ldap_groups" end - add_cron_jobs do - { - "Ldap::SynchronizationJob": { - cron: "30 23 * * *", # Run once per night at 11:30pm - class: Ldap::SynchronizationJob.name - } - } - end + add_cron_jobs { LdapGroups::SynchronizationJob } 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 cc5dbf1048c1..bf698faa63ef 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,9 +26,11 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class Storages::CleanupUncontaineredFileLinksJob < ApplicationJob +class Storages::CleanupUncontaineredFileLinksJob < Cron::CronJob queue_with_priority :low + self.cron_expression = "06 22 * * *" + def perform Storages::FileLink .where(container: nil) diff --git a/lib/open_project/health_checks/smtp_check.rb b/modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb similarity index 74% rename from lib/open_project/health_checks/smtp_check.rb rename to modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb index 81b09ce4d1f1..d5ae10bd6398 100644 --- a/lib/open_project/health_checks/smtp_check.rb +++ b/modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) 2012-2024 the OpenProject GmbH @@ -23,22 +25,22 @@ # 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. +# See COPYRIGHT and LICENSE files 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 +module Storages + class ManageNextcloudIntegrationCronJob < Cron::CronJob + include ManageNextcloudIntegrationJobMixin + + queue_with_priority :low + + self.cron_expression = "1 * * * *" + def self.ensure_scheduled! + if ::Storages::ProjectStorage.active_automatically_managed.exists? super + else + remove 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 new file mode 100644 index 000000000000..89485d35f3c0 --- /dev/null +++ b/modules/storages/app/workers/storages/manage_nextcloud_integration_events_job.rb @@ -0,0 +1,63 @@ +#-- 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 deleted file mode 100644 index 8dab5b01fc14..000000000000 --- a/modules/storages/app/workers/storages/manage_nextcloud_integration_job.rb +++ /dev/null @@ -1,109 +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 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) unless 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 - - 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 - find_storages do |storage| - next unless storage.configured? - 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 find_storages(&block) - ::Storages::Storage - .automatic_management_enabled - .includes(:oauth_client) - .find_each(&block) - end - - 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 new file mode 100644 index 000000000000..8da1903ea7bc --- /dev/null +++ b/modules/storages/app/workers/storages/manage_nextcloud_integration_job_mixin.rb @@ -0,0 +1,66 @@ +# 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 index c6fe00093aeb..b4042f391e2e 100644 --- a/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb +++ b/modules/storages/db/migrate/20231009135807_remove_renamed_cronjobs.rb @@ -28,8 +28,8 @@ class RemoveRenamedCronjobs < ActiveRecord::Migration[7.0] def up - execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: CleanupUncontaineredFileLinksJob%'") - execute("DELETE FROM delayed_jobs WHERE handler LIKE '%job_class: ManageNextcloudIntegrationJob%'") + Delayed::Job.where("handler LIKE '%job_class: CleanupUncontaineredFileLinksJob%'").delete_all + Delayed::Job.where("handler LIKE '%job_class: ManageNextcloudIntegrationJob%'").delete_all end def down; end diff --git a/modules/storages/lib/open_project/storages/engine.rb b/modules/storages/lib/open_project/storages/engine.rb index b65189d99b4d..3eae0ce4005e 100644 --- a/modules/storages/lib/open_project/storages/engine.rb +++ b/modules/storages/lib/open_project/storages/engine.rb @@ -62,7 +62,7 @@ def self.permissions OpenProject::Events::PROJECT_UNARCHIVED ].each do |event| OpenProject::Notifications.subscribe(event) do |_payload| - ::Storages::ManageNextcloudIntegrationJob.debounce + ::Storages::ManageNextcloudIntegrationEventsJob.debounce end end @@ -70,21 +70,21 @@ def self.permissions OpenProject::Events::OAUTH_CLIENT_TOKEN_CREATED ) do |payload| if payload[:integration_type] == "Storages::Storage" - ::Storages::ManageNextcloudIntegrationJob.debounce + ::Storages::ManageNextcloudIntegrationEventsJob.debounce end end OpenProject::Notifications.subscribe( OpenProject::Events::ROLE_UPDATED ) do |payload| if payload[:permissions_diff]&.intersect?(OpenProject::Storages::Engine.permissions) - ::Storages::ManageNextcloudIntegrationJob.debounce + ::Storages::ManageNextcloudIntegrationEventsJob.debounce end end OpenProject::Notifications.subscribe( OpenProject::Events::ROLE_DESTROYED ) do |payload| if payload[:permissions]&.intersect?(OpenProject::Storages::Engine.permissions) - ::Storages::ManageNextcloudIntegrationJob.debounce + ::Storages::ManageNextcloudIntegrationEventsJob.debounce end end @@ -95,8 +95,8 @@ def self.permissions ].each do |event| OpenProject::Notifications.subscribe(event) do |payload| if payload[:project_folder_mode] == :automatic - ::Storages::ManageNextcloudIntegrationJob.debounce - ::Storages::ManageNextcloudIntegrationJob.disable_cron_job_if_needed + ::Storages::ManageNextcloudIntegrationEventsJob.debounce + ::Storages::ManageNextcloudIntegrationCronJob.ensure_scheduled! end end end @@ -287,17 +287,10 @@ def self.permissions end add_cron_jobs do - { - "Storages::CleanupUncontaineredFileLinksJob": { - cron: "06 22 * * *", - class: ::Storages::CleanupUncontaineredFileLinksJob.name - }, - - "Storages::ManageNextcloudIntegrationJob": { - cron: "1 * * * *", - class: ::Storages::ManageNextcloudIntegrationJob.name - } - } + [ + Storages::CleanupUncontaineredFileLinksJob, + Storages::ManageNextcloudIntegrationCronJob + ] 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 f60f3286a663..89cf1e8a446f 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,37 +32,39 @@ require_module_spec_helper RSpec.describe Storages::CleanupUncontaineredFileLinksJob, type: :job do - describe "#perform" 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 "has a schedule set" do + expect(described_class.cron_expression).to eq("06 22 * * *") + end - expect(Storages::FileLink.count).to eq(0) + 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) - 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, - container_id: nil, - container_type: nil) - containered_old = create(:file_link, - container_id: 1, + 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) - containered_young = create(:file_link, - container_id: 1) + 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) - end + 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 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 new file mode 100644 index 000000000000..ddb0942e14d5 --- /dev/null +++ b/modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb @@ -0,0 +1,154 @@ +# 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("1 * * * *") + 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 new file mode 100644 index 000000000000..699a0a8ff584 --- /dev/null +++ b/modules/storages/spec/workers/storages/manage_nextcloud_integration_events_job_spec.rb @@ -0,0 +1,88 @@ +# 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 deleted file mode 100644 index 7e150dbed67a..000000000000 --- a/modules/storages/spec/workers/storages/manage_nextcloud_integration_job_spec.rb +++ /dev/null @@ -1,160 +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::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 - let(:storage1) { create(:nextcloud_storage_configured, :as_automatically_managed) } - - subject { described_class.new.perform } - - it "calls NextcloudGroupFolderPropertiesSyncService for each automatically managed storage" do - storage2 = create(:nextcloud_storage, :as_not_automatically_managed) - storage3 = create(:nextcloud_storage, :as_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) - expect(Storages::NextcloudGroupFolderPropertiesSyncService).not_to have_received(:call).with(storage3) - end - - it "marks storage as healthy if sync was successful" do - 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 - 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 00fd8f2233ae..4b72639a408a 100644 --- a/modules/webhooks/app/workers/cleanup_webhook_logs_job.rb +++ b/modules/webhooks/app/workers/cleanup_webhook_logs_job.rb @@ -26,7 +26,10 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class CleanupWebhookLogsJob < ApplicationJob +class CleanupWebhookLogsJob < Cron::CronJob + # runs at 5:28 on Sunday + self.cron_expression = "28 5 * * 7" + # 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 4b65c8343fa1..df9d6f9df56b 100644 --- a/modules/webhooks/lib/open_project/webhooks/engine.rb +++ b/modules/webhooks/lib/open_project/webhooks/engine.rb @@ -51,13 +51,6 @@ class Engine < ::Rails::Engine end end - add_cron_jobs do - { - CleanupWebhookLogsJob: { - cron: "28 5 * * 7", # runs at 5:28 on Sunday - class: ::CleanupWebhookLogsJob.name - } - } - end + add_cron_jobs { CleanupWebhookLogsJob } end end diff --git a/packaging/scripts/check b/packaging/scripts/check index 5c3ae2647711..bd98c43d9bd2 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 "good_job start" ; then +if ps -u "$APP_USER" -f | grep -q "rake jobs:work" ; 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 cb29dbeace3b..ea691e755397 100755 --- a/packaging/scripts/worker +++ b/packaging/scripts/worker @@ -1,3 +1,3 @@ #!/bin/bash -e -QUIET=true bundle exec good_job start +QUIET=true bundle exec rake jobs:work diff --git a/spec/contracts/settings/working_days_params_contract_spec.rb b/spec/contracts/settings/working_days_params_contract_spec.rb index e7132af4ae05..7ed611eb3aee 100644 --- a/spec/contracts/settings/working_days_params_contract_spec.rb +++ b/spec/contracts/settings/working_days_params_contract_spec.rb @@ -37,6 +37,13 @@ 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" @@ -48,10 +55,7 @@ context "with an ApplyWorkingDaysChangeJob already existing" do let(:params) { { working_days: [1, 2, 3] } } - before do - ActiveJob::Base.disable_test_adapter - WorkPackages::ApplyWorkingDaysChangeJob.perform_later - end + let(:apply_job_scheduled) { true } 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 ef52683f50d2..e9492430edf6 100644 --- a/spec/controllers/roles_controller_spec.rb +++ b/spec/controllers/roles_controller_spec.rb @@ -387,7 +387,7 @@ subject expect(enqueued_jobs.count).to eq(1) - expect(enqueued_jobs[0][:job]).to eq(Storages::ManageNextcloudIntegrationJob) + expect(enqueued_jobs[0][:job]).to eq(Storages::ManageNextcloudIntegrationEventsJob) 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 23c605c5f76e..02158d487fde 100644 --- a/spec/features/admin/working_days_spec.rb +++ b/spec/features/admin/working_days_spec.rb @@ -163,8 +163,11 @@ def working_days_setting it "shows an error when a previous change to the working days configuration isn't processed yet" do # Have a job already scheduled - ActiveJob::Base.disable_test_adapter - WorkPackages::ApplyWorkingDaysChangeJob.perform_later(user_id: 5) + # 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)) 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 06f6364a7b82..fb604b3a48df 100644 --- a/spec/features/notifications/reminder_mail_spec.rb +++ b/spec/features/notifications/reminder_mail_spec.rb @@ -9,10 +9,10 @@ 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) } - # ApplicationJob#job_scheduled_at is used for scheduling the reminder mails + # The run_at time of the delayed job 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. + # 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 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,10 +57,13 @@ work_package involved_work_package - ActiveJob::Base.disable_test_adapter + ActiveJob::Base.queue_adapter.enqueued_jobs.clear - scheduled_job = Notifications::ScheduleReminderMailsJob.perform_later - GoodJob::Job.where(id: scheduled_job.job_id).update_all(scheduled_at: job_run_at) + # 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)) end it "sends a digest mail based on the configuration", with_settings: { journal_aggregation_time_minutes: 0 } do @@ -85,9 +88,13 @@ involved_work_package.save! end - 2.times { GoodJob.perform_inline } + # 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 - 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 e3a1b36eda7f..7cd70325f396 100644 --- a/spec/features/projects/destroy_spec.rb +++ b/spec/features/projects/destroy_spec.rb @@ -35,17 +35,26 @@ current_user { create(:admin) } - before { project_page.visit! } + before do + # Disable background worker + allow(Delayed::Worker) + .to receive(:delay_jobs) + .and_return(false) + + project_page.visit! + end 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 @@ -54,7 +63,6 @@ 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 14d968e6e069..375a8f546988 100644 --- a/spec/lib/open_project/events_spec.rb +++ b/spec/lib/open_project/events_spec.rb @@ -37,7 +37,7 @@ def fire_event(event_constant_name) end before do - allow(Storages::ManageNextcloudIntegrationJob).to receive(:debounce) + allow(Storages::ManageNextcloudIntegrationEventsJob).to receive(:debounce) end %w[ @@ -53,7 +53,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) end end @@ -62,13 +62,16 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) end it do - allow(Storages::ManageNextcloudIntegrationJob).to receive(:disable_cron_job_if_needed) + # 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!) subject - expect(Storages::ManageNextcloudIntegrationJob).to have_received(:disable_cron_job_if_needed) + expect(Storages::ManageNextcloudIntegrationCronJob).to have_received(:ensure_scheduled!) end end end @@ -90,7 +93,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) end end end @@ -103,7 +106,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) end end @@ -112,7 +115,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) end end end @@ -125,7 +128,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) end end @@ -134,7 +137,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) end end end @@ -147,7 +150,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).not_to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).not_to have_received(:debounce) end end @@ -156,7 +159,7 @@ def fire_event(event_constant_name) it do subject - expect(Storages::ManageNextcloudIntegrationJob).to have_received(:debounce) + expect(Storages::ManageNextcloudIntegrationEventsJob).to have_received(:debounce) end end end diff --git a/spec/seeders/setting_seeder_spec.rb b/spec/seeders/setting_seeder_spec.rb index f5ac088b7320..6ea3537d3f5e 100644 --- a/spec/seeders/setting_seeder_spec.rb +++ b/spec/seeders/setting_seeder_spec.rb @@ -36,6 +36,11 @@ 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 new file mode 100644 index 000000000000..8a52eef36fe1 --- /dev/null +++ b/spec/workers/non_existing_job_class_spec.rb @@ -0,0 +1,65 @@ +#-- 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 a22ab39837b4..42dcf9305b04 100644 --- a/spec/workers/notifications/schedule_date_alerts_notifications_job_spec.rb +++ b/spec/workers/notifications/schedule_date_alerts_notifications_job_spec.rb @@ -32,30 +32,42 @@ 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(:scheduled_job) { described_class.perform_later } + let(:schedule_job) do + described_class.ensure_scheduled! + described_class.delayed_job + end before do - ActiveJob::Base.disable_test_adapter - scheduled_job + # 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 end - def set_scheduled_time(scheduled_at) - GoodJob::Job.where(id: scheduled_job.job_id).update_all(scheduled_at:) + def set_scheduled_time(run_at) + schedule_job.update_column(:run_at, run_at) end # Converts "hh:mm" into { hour: h, min: m } @@ -70,25 +82,28 @@ def timezone_time(time, timezone) 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 - GoodJob.perform_inline - # scheduled_job.reload.invoke_job + schedule_job.reload.invoke_job yield if block_given? end end - def deserialize_job(job) - deserializer_class = Class.new { include(ActiveJob::Arguments) } - deserializer_class.new - .deserialize(job.serialized_params) - .to_h + 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 end - 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 + 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 end shared_examples_for "job execution creates date alerts creation job" do @@ -100,12 +115,9 @@ def expect_job(job, *arguments) it "creates the job for the user" do expect do run_job(timezone:, scheduled_at:, local_time:) do - j = GoodJob::Job.where(job_class: "Notifications::CreateDateAlertsNotificationsJob") - .order(created_at: :desc) - .last - expect_job(j, user) + expect_job(Delayed::Job.last, "Notifications::CreateDateAlertsNotificationsJob", user) end - end.to change(GoodJob::Job, :count).by 1 + end.to change(Delayed::Job, :count).by 1 end end @@ -117,7 +129,7 @@ def expect_job(job, *arguments) it "creates no job" do expect do run_job(timezone:, scheduled_at:, local_time:) - end.not_to change(GoodJob::Job, :count) + end.not_to change(Delayed::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 a8c3a2fcfa48..bcbe5a00687c 100644 --- a/spec/workers/notifications/schedule_reminder_mails_job_spec.rb +++ b/spec/workers/notifications/schedule_reminder_mails_job_spec.rb @@ -29,46 +29,69 @@ require "spec_helper" RSpec.describe Notifications::ScheduleReminderMailsJob, type: :job do - let(:scheduled_job) { described_class.perform_later } + subject(:job) { scheduled_job.invoke_job } + + let(:scheduled_job) do + described_class.ensure_scheduled! + + Delayed::Job.first + end + 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 scheduled_at time persisted there - ActiveJob::Base.disable_test_adapter - scheduled_job + # 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) 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 { GoodJob.perform_inline }.to change(GoodJob::Job, :count).by(2) + expect { subject } + .to change(Delayed::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" } - 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) + expect(reminder_jobs[0].job_data["arguments"]) + .to contain_exactly(23) + + expect(reminder_jobs[1].job_data["arguments"]) + .to contain_exactly(42) end it "queries with the intended job execution time (which might have been missed due to high load)" do - GoodJob.perform_inline + subject - expect(User).to have_received(:having_reminder_mail_to_send).with(scheduled_job.job_scheduled_at) + expect(User) + .to have_received(:having_reminder_mail_to_send) + .with(run_at) end end it_behaves_like "schedules reminder mails" context "with a job that missed some runs" do - before do - GoodJob::Job - .where(id: scheduled_job.job_id) - .update_all(scheduled_at: scheduled_job.job_scheduled_at - 3.hours) - end + let(:run_at) { scheduled_job.run_at - 3.hours } it_behaves_like "schedules reminder mails" end diff --git a/spec/migrations/reorder_project_children_spec.rb b/spec/workers/projects/reorder_children_job_integration_spec.rb similarity index 89% rename from spec/migrations/reorder_project_children_spec.rb rename to spec/workers/projects/reorder_children_job_integration_spec.rb index 8a1f34532b40..5ce0aa966199 100644 --- a/spec/migrations/reorder_project_children_spec.rb +++ b/spec/workers/projects/reorder_children_job_integration_spec.rb @@ -27,11 +27,9 @@ #++ require "spec_helper" -require Rails.root.join("db/migrate/20220202140507_reorder_project_children.rb") -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 } } +RSpec.describe Projects::ReorderHierarchyJob, type: :model do + subject(:job) { described_class.perform_now } shared_let(:parent_project_a) { create(:project, name: "ParentA") } shared_let(:parent_project_b) { create(:project, name: "ParentB") } 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 e296de7ca974..d3b1629995a9 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,4 +1811,26 @@ 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