diff options
117 files changed, 371 insertions, 304 deletions
diff --git a/app/models/group.rb b/app/models/group.rb index 27287f079a4..505e943e464 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -2,6 +2,7 @@ require 'carrierwave/orm/activerecord' class Group < Namespace include Gitlab::ConfigHelper + include AfterCommitQueue include AccessRequestable include Avatarable include Referable diff --git a/app/models/key.rb b/app/models/key.rb index 815fd1de909..a3f8a5d6dc7 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -2,6 +2,7 @@ require 'digest/md5' class Key < ActiveRecord::Base include Gitlab::CurrentSettings + include AfterCommitQueue include Sortable belongs_to :user diff --git a/app/models/member.rb b/app/models/member.rb index cbbd58f2eaf..2fe5fda985f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -1,4 +1,5 @@ class Member < ActiveRecord::Base + include AfterCommitQueue include Sortable include Importable include Expirable diff --git a/app/models/service.rb b/app/models/service.rb index fdd2605e3e3..3c4f1885dd0 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -211,7 +211,7 @@ class Service < ActiveRecord::Base def async_execute(data) return unless supported_events.include?(data[:object_kind]) - Sidekiq::Client.enqueue(ProjectServiceWorker, id, data) + ProjectServiceWorker.perform_async(id, data) end def issue_tracker? diff --git a/app/models/user.rb b/app/models/user.rb index 76fd395be9a..38ee4ed50c1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,6 +7,7 @@ class User < ActiveRecord::Base include Gitlab::ConfigHelper include Gitlab::CurrentSettings include Gitlab::SQL::Pattern + include AfterCommitQueue include Avatarable include Referable include Sortable @@ -903,6 +904,7 @@ class User < ActiveRecord::Base def post_destroy_hook log_info("User \"#{name}\" (#{email}) was removed") + system_hook_service.execute_hooks_for(self, :destroy) end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 911cc919bb8..690918b4a00 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -1,6 +1,10 @@ class SystemHooksService def execute_hooks_for(model, event) - execute_hooks(build_event_data(model, event)) + data = build_event_data(model, event) + + model.run_after_commit_or_now do + SystemHooksService.new.execute_hooks(data) + end end def execute_hooks(data, hooks_scope = :all) diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index cd99e0b90f9..6ebc7c89500 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -63,7 +63,7 @@ class WebHookService end def async_execute - Sidekiq::Client.enqueue(WebHookWorker, hook.id, data, hook_name) + WebHookWorker.perform_async(hook.id, data, hook_name) end private diff --git a/app/workers/admin_email_worker.rb b/app/workers/admin_email_worker.rb index c2dc955b27c..bec0a003a1c 100644 --- a/app/workers/admin_email_worker.rb +++ b/app/workers/admin_email_worker.rb @@ -1,5 +1,5 @@ class AdminEmailWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/authorized_projects_worker.rb b/app/workers/authorized_projects_worker.rb index 55d8d0c69d1..09559e3b696 100644 --- a/app/workers/authorized_projects_worker.rb +++ b/app/workers/authorized_projects_worker.rb @@ -1,6 +1,5 @@ class AuthorizedProjectsWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker # Schedules multiple jobs and waits for them to be completed. def self.bulk_perform_and_wait(args_list) @@ -17,11 +16,6 @@ class AuthorizedProjectsWorker waiter.wait end - # Schedules multiple jobs to run in sidekiq without waiting for completion - def self.bulk_perform_async(args_list) - Sidekiq::Client.push_bulk('class' => self, 'queue' => sidekiq_options['queue'], 'args' => args_list) - end - # Performs multiple jobs directly. Failed jobs will be put into sidekiq so # they can benefit from retries def self.bulk_perform_inline(args_list) diff --git a/app/workers/background_migration_worker.rb b/app/workers/background_migration_worker.rb index 45ce49bb5c0..aeb3bc019b9 100644 --- a/app/workers/background_migration_worker.rb +++ b/app/workers/background_migration_worker.rb @@ -1,34 +1,5 @@ class BackgroundMigrationWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue - - # Enqueues a number of jobs in bulk. - # - # The `jobs` argument should be an Array of Arrays, each sub-array must be in - # the form: - # - # [migration-class, [arg1, arg2, ...]] - def self.perform_bulk(jobs) - Sidekiq::Client.push_bulk('class' => self, - 'queue' => sidekiq_options['queue'], - 'args' => jobs) - end - - # Schedules multiple jobs in bulk, with a delay. - # - def self.perform_bulk_in(delay, jobs) - now = Time.now.to_i - schedule = now + delay.to_i - - if schedule <= now - raise ArgumentError, 'The schedule time must be in the future!' - end - - Sidekiq::Client.push_bulk('class' => self, - 'queue' => sidekiq_options['queue'], - 'args' => jobs, - 'at' => schedule) - end + include ApplicationWorker # Performs the background migration. # diff --git a/app/workers/build_coverage_worker.rb b/app/workers/build_coverage_worker.rb index cd4af85d047..62b212c79be 100644 --- a/app/workers/build_coverage_worker.rb +++ b/app/workers/build_coverage_worker.rb @@ -1,5 +1,5 @@ class BuildCoverageWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue def perform(build_id) diff --git a/app/workers/build_finished_worker.rb b/app/workers/build_finished_worker.rb index 52e7d346e74..5efa9180f5e 100644 --- a/app/workers/build_finished_worker.rb +++ b/app/workers/build_finished_worker.rb @@ -1,5 +1,5 @@ class BuildFinishedWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/build_hooks_worker.rb b/app/workers/build_hooks_worker.rb index dedaf2835e6..6705a1c2709 100644 --- a/app/workers/build_hooks_worker.rb +++ b/app/workers/build_hooks_worker.rb @@ -1,5 +1,5 @@ class BuildHooksWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :hooks diff --git a/app/workers/build_queue_worker.rb b/app/workers/build_queue_worker.rb index e5ceb9ef715..fc775a84dc0 100644 --- a/app/workers/build_queue_worker.rb +++ b/app/workers/build_queue_worker.rb @@ -1,5 +1,5 @@ class BuildQueueWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index 20ec24bd18a..ec049821ad7 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -1,5 +1,5 @@ class BuildSuccessWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/build_trace_sections_worker.rb b/app/workers/build_trace_sections_worker.rb index 8c57e8f767b..c0f5c144e10 100644 --- a/app/workers/build_trace_sections_worker.rb +++ b/app/workers/build_trace_sections_worker.rb @@ -1,5 +1,5 @@ class BuildTraceSectionsWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue def perform(build_id) diff --git a/app/workers/cluster_install_app_worker.rb b/app/workers/cluster_install_app_worker.rb index 899aed904e4..f771cb4939f 100644 --- a/app/workers/cluster_install_app_worker.rb +++ b/app/workers/cluster_install_app_worker.rb @@ -1,5 +1,5 @@ class ClusterInstallAppWorker - include Sidekiq::Worker + include ApplicationWorker include ClusterQueue include ClusterApplications diff --git a/app/workers/cluster_provision_worker.rb b/app/workers/cluster_provision_worker.rb index b01f9708424..1ab4de3b647 100644 --- a/app/workers/cluster_provision_worker.rb +++ b/app/workers/cluster_provision_worker.rb @@ -1,5 +1,5 @@ class ClusterProvisionWorker - include Sidekiq::Worker + include ApplicationWorker include ClusterQueue def perform(cluster_id) diff --git a/app/workers/cluster_wait_for_app_installation_worker.rb b/app/workers/cluster_wait_for_app_installation_worker.rb index 4bb8c293e5d..d564d5e48bf 100644 --- a/app/workers/cluster_wait_for_app_installation_worker.rb +++ b/app/workers/cluster_wait_for_app_installation_worker.rb @@ -1,5 +1,5 @@ class ClusterWaitForAppInstallationWorker - include Sidekiq::Worker + include ApplicationWorker include ClusterQueue include ClusterApplications diff --git a/app/workers/concerns/application_worker.rb b/app/workers/concerns/application_worker.rb new file mode 100644 index 00000000000..9c3bdabc49e --- /dev/null +++ b/app/workers/concerns/application_worker.rb @@ -0,0 +1,40 @@ +Sidekiq::Worker.extend ActiveSupport::Concern + +module ApplicationWorker + extend ActiveSupport::Concern + + include Sidekiq::Worker + + included do + sidekiq_options queue: base_queue_name + end + + module ClassMethods + def base_queue_name + name + .sub(/\AGitlab::/, '') + .sub(/Worker\z/, '') + .underscore + .tr('/', '_') + end + + def queue + get_sidekiq_options['queue'].to_s + end + + def bulk_perform_async(args_list) + Sidekiq::Client.push_bulk('class' => self, 'args' => args_list) + end + + def bulk_perform_in(delay, args_list) + now = Time.now.to_i + schedule = now + delay.to_i + + if schedule <= now + raise ArgumentError, 'The schedule time must be in the future!' + end + + Sidekiq::Client.push_bulk('class' => self, 'args' => args_list, 'at' => schedule) + end + end +end diff --git a/app/workers/concerns/dedicated_sidekiq_queue.rb b/app/workers/concerns/dedicated_sidekiq_queue.rb deleted file mode 100644 index 132bae6022b..00000000000 --- a/app/workers/concerns/dedicated_sidekiq_queue.rb +++ /dev/null @@ -1,9 +0,0 @@ -# Concern that sets the queue of a Sidekiq worker based on the worker's class -# name/namespace. -module DedicatedSidekiqQueue - extend ActiveSupport::Concern - - included do - sidekiq_options queue: name.sub(/Worker\z/, '').underscore.tr('/', '_') - end -end diff --git a/app/workers/concerns/gitlab/github_import/object_importer.rb b/app/workers/concerns/gitlab/github_import/object_importer.rb index 67e36c811de..9a9fbaad653 100644 --- a/app/workers/concerns/gitlab/github_import/object_importer.rb +++ b/app/workers/concerns/gitlab/github_import/object_importer.rb @@ -8,7 +8,7 @@ module Gitlab extend ActiveSupport::Concern included do - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include ReschedulingMethods include NotifyUponDeath diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index 9b5ff17aafa..f371731f68c 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -1,6 +1,5 @@ class CreateGpgSignatureWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(commit_sha, project_id) project = Project.find_by(id: project_id) diff --git a/app/workers/create_pipeline_worker.rb b/app/workers/create_pipeline_worker.rb index 865ad1ba420..00cd7b85b9f 100644 --- a/app/workers/create_pipeline_worker.rb +++ b/app/workers/create_pipeline_worker.rb @@ -1,5 +1,5 @@ class CreatePipelineWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :creation diff --git a/app/workers/delete_merged_branches_worker.rb b/app/workers/delete_merged_branches_worker.rb index f870da4ecfd..07cd1f02fb5 100644 --- a/app/workers/delete_merged_branches_worker.rb +++ b/app/workers/delete_merged_branches_worker.rb @@ -1,6 +1,5 @@ class DeleteMergedBranchesWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(project_id, user_id) begin diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb index 3340a7be4fe..6c431b02979 100644 --- a/app/workers/delete_user_worker.rb +++ b/app/workers/delete_user_worker.rb @@ -1,6 +1,5 @@ class DeleteUserWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(current_user_id, delete_user_id, options = {}) delete_user = User.find(delete_user_id) diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index 05cb116a623..dd8a6cbbef1 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -1,6 +1,5 @@ class EmailReceiverWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(raw) return unless Gitlab::IncomingEmail.enabled? diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index f5ccc84c160..21da27973fe 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -1,6 +1,5 @@ class EmailsOnPushWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker attr_reader :email, :skip_premailer diff --git a/app/workers/expire_build_artifacts_worker.rb b/app/workers/expire_build_artifacts_worker.rb index a27585fd389..87e5dca01fd 100644 --- a/app/workers/expire_build_artifacts_worker.rb +++ b/app/workers/expire_build_artifacts_worker.rb @@ -1,5 +1,5 @@ class ExpireBuildArtifactsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform @@ -8,6 +8,6 @@ class ExpireBuildArtifactsWorker build_ids = Ci::Build.with_expired_artifacts.pluck(:id) build_ids = build_ids.map { |build_id| [build_id] } - Sidekiq::Client.push_bulk('class' => ExpireBuildInstanceArtifactsWorker, 'args' => build_ids ) + ExpireBuildInstanceArtifactsWorker.bulk_perform_async(build_ids) end end diff --git a/app/workers/expire_build_instance_artifacts_worker.rb b/app/workers/expire_build_instance_artifacts_worker.rb index 7b59e976492..234b4357cf7 100644 --- a/app/workers/expire_build_instance_artifacts_worker.rb +++ b/app/workers/expire_build_instance_artifacts_worker.rb @@ -1,6 +1,5 @@ class ExpireBuildInstanceArtifactsWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(build_id) build = Ci::Build diff --git a/app/workers/expire_job_cache_worker.rb b/app/workers/expire_job_cache_worker.rb index 98a7500bffe..a591e2da519 100644 --- a/app/workers/expire_job_cache_worker.rb +++ b/app/workers/expire_job_cache_worker.rb @@ -1,5 +1,5 @@ class ExpireJobCacheWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :cache diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb index 1a0e7f92875..a3ac32b437d 100644 --- a/app/workers/expire_pipeline_cache_worker.rb +++ b/app/workers/expire_pipeline_cache_worker.rb @@ -1,5 +1,5 @@ class ExpirePipelineCacheWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :cache diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb index ec65d3ff65e..8e26275669e 100644 --- a/app/workers/git_garbage_collect_worker.rb +++ b/app/workers/git_garbage_collect_worker.rb @@ -1,6 +1,5 @@ class GitGarbageCollectWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include Gitlab::CurrentSettings sidekiq_options retry: false diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb index 877f88c043f..400396d5755 100644 --- a/app/workers/gitlab/github_import/advance_stage_worker.rb +++ b/app/workers/gitlab/github_import/advance_stage_worker.rb @@ -7,7 +7,7 @@ module Gitlab # been completed this worker will advance the import process to the next # stage. class AdvanceStageWorker - include Sidekiq::Worker + include ApplicationWorker sidekiq_options queue: 'github_importer_advance_stage', dead: false diff --git a/app/workers/gitlab/github_import/refresh_import_jid_worker.rb b/app/workers/gitlab/github_import/refresh_import_jid_worker.rb index 45a38927225..7108b531bc2 100644 --- a/app/workers/gitlab/github_import/refresh_import_jid_worker.rb +++ b/app/workers/gitlab/github_import/refresh_import_jid_worker.rb @@ -3,7 +3,7 @@ module Gitlab module GithubImport class RefreshImportJidWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue # The interval to schedule new instances of this job at. diff --git a/app/workers/gitlab/github_import/stage/finish_import_worker.rb b/app/workers/gitlab/github_import/stage/finish_import_worker.rb index 1a09497780a..073d6608082 100644 --- a/app/workers/gitlab/github_import/stage/finish_import_worker.rb +++ b/app/workers/gitlab/github_import/stage/finish_import_worker.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Stage class FinishImportWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include StageMethods diff --git a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb index f8a3684c6ba..5726fbb573d 100644 --- a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Stage class ImportBaseDataWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include StageMethods diff --git a/app/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker.rb b/app/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker.rb index e110b7c1c36..7007754ff2e 100644 --- a/app/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Stage class ImportIssuesAndDiffNotesWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include StageMethods diff --git a/app/workers/gitlab/github_import/stage/import_notes_worker.rb b/app/workers/gitlab/github_import/stage/import_notes_worker.rb index 9810ed25cf9..5f4678a595f 100644 --- a/app/workers/gitlab/github_import/stage/import_notes_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_notes_worker.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Stage class ImportNotesWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include StageMethods diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb index c531f26e897..1c5a7139802 100644 --- a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Stage class ImportPullRequestsWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include StageMethods diff --git a/app/workers/gitlab/github_import/stage/import_repository_worker.rb b/app/workers/gitlab/github_import/stage/import_repository_worker.rb index aa5762e773d..4d16cef1130 100644 --- a/app/workers/gitlab/github_import/stage/import_repository_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_repository_worker.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Stage class ImportRepositoryWorker - include Sidekiq::Worker + include ApplicationWorker include GithubImport::Queue include StageMethods diff --git a/app/workers/gitlab_shell_worker.rb b/app/workers/gitlab_shell_worker.rb index 0ec871e00e1..a0028e41332 100644 --- a/app/workers/gitlab_shell_worker.rb +++ b/app/workers/gitlab_shell_worker.rb @@ -1,7 +1,6 @@ class GitlabShellWorker - include Sidekiq::Worker + include ApplicationWorker include Gitlab::ShellAdapter - include DedicatedSidekiqQueue def perform(action, *arg) gitlab_shell.__send__(action, *arg) # rubocop:disable GitlabSecurity/PublicSend diff --git a/app/workers/gitlab_usage_ping_worker.rb b/app/workers/gitlab_usage_ping_worker.rb index 0a55aab63fd..6dd281b1147 100644 --- a/app/workers/gitlab_usage_ping_worker.rb +++ b/app/workers/gitlab_usage_ping_worker.rb @@ -1,7 +1,7 @@ class GitlabUsagePingWorker LEASE_TIMEOUT = 86400 - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/group_destroy_worker.rb b/app/workers/group_destroy_worker.rb index bd8e212e928..f577b310b20 100644 --- a/app/workers/group_destroy_worker.rb +++ b/app/workers/group_destroy_worker.rb @@ -1,6 +1,5 @@ class GroupDestroyWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include ExceptionBacktrace def perform(group_id, user_id) diff --git a/app/workers/import_export_project_cleanup_worker.rb b/app/workers/import_export_project_cleanup_worker.rb index 7957ed807ab..9788c8df3a3 100644 --- a/app/workers/import_export_project_cleanup_worker.rb +++ b/app/workers/import_export_project_cleanup_worker.rb @@ -1,5 +1,5 @@ class ImportExportProjectCleanupWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/invalid_gpg_signature_update_worker.rb b/app/workers/invalid_gpg_signature_update_worker.rb index db6b1ea8e8d..6774ab307c6 100644 --- a/app/workers/invalid_gpg_signature_update_worker.rb +++ b/app/workers/invalid_gpg_signature_update_worker.rb @@ -1,6 +1,5 @@ class InvalidGpgSignatureUpdateWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(gpg_key_id) gpg_key = GpgKey.find_by(id: gpg_key_id) diff --git a/app/workers/irker_worker.rb b/app/workers/irker_worker.rb index 311fc187e49..9ae5456be4c 100644 --- a/app/workers/irker_worker.rb +++ b/app/workers/irker_worker.rb @@ -2,8 +2,7 @@ require 'json' require 'socket' class IrkerWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(project_id, chans, colors, push_data, settings) project = Project.find(project_id) diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 48e2da338f6..ba832fe30c6 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -1,6 +1,5 @@ class MergeWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(merge_request_id, current_user_id, params) params = params.with_indifferent_access diff --git a/app/workers/namespaceless_project_destroy_worker.rb b/app/workers/namespaceless_project_destroy_worker.rb index f1cd1769421..adb25c2a170 100644 --- a/app/workers/namespaceless_project_destroy_worker.rb +++ b/app/workers/namespaceless_project_destroy_worker.rb @@ -5,14 +5,9 @@ # The worker will reject doing anything for projects that *do* have a # namespace. For those use ProjectDestroyWorker instead. class NamespacelessProjectDestroyWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include ExceptionBacktrace - def self.bulk_perform_async(args_list) - Sidekiq::Client.push_bulk('class' => self, 'queue' => sidekiq_options['queue'], 'args' => args_list) - end - def perform(project_id) begin project = Project.unscoped.find(project_id) diff --git a/app/workers/new_issue_worker.rb b/app/workers/new_issue_worker.rb index d9a8e892e90..3bc030f9c62 100644 --- a/app/workers/new_issue_worker.rb +++ b/app/workers/new_issue_worker.rb @@ -1,6 +1,5 @@ class NewIssueWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include NewIssuable def perform(issue_id, user_id) diff --git a/app/workers/new_merge_request_worker.rb b/app/workers/new_merge_request_worker.rb index 1910c490159..bda2a0ab59d 100644 --- a/app/workers/new_merge_request_worker.rb +++ b/app/workers/new_merge_request_worker.rb @@ -1,6 +1,5 @@ class NewMergeRequestWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include NewIssuable def perform(merge_request_id, user_id) diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index 926162b8c53..67c54fbf10e 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -1,6 +1,5 @@ class NewNoteWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker # Keep extra parameter to preserve backwards compatibility with # old `NewNoteWorker` jobs (can remove later) diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb index 64788da7299..62f733c02fc 100644 --- a/app/workers/pages_worker.rb +++ b/app/workers/pages_worker.rb @@ -1,5 +1,5 @@ class PagesWorker - include Sidekiq::Worker + include ApplicationWorker sidekiq_options queue: :pages, retry: false diff --git a/app/workers/pipeline_hooks_worker.rb b/app/workers/pipeline_hooks_worker.rb index 30a75ec8435..661c29efe88 100644 --- a/app/workers/pipeline_hooks_worker.rb +++ b/app/workers/pipeline_hooks_worker.rb @@ -1,5 +1,5 @@ class PipelineHooksWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :hooks diff --git a/app/workers/pipeline_metrics_worker.rb b/app/workers/pipeline_metrics_worker.rb index 070943f1ecc..d46d1f122fc 100644 --- a/app/workers/pipeline_metrics_worker.rb +++ b/app/workers/pipeline_metrics_worker.rb @@ -1,5 +1,5 @@ class PipelineMetricsWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue def perform(pipeline_id) diff --git a/app/workers/pipeline_notification_worker.rb b/app/workers/pipeline_notification_worker.rb index cdb860b6675..a9a1168a6e3 100644 --- a/app/workers/pipeline_notification_worker.rb +++ b/app/workers/pipeline_notification_worker.rb @@ -1,5 +1,5 @@ class PipelineNotificationWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue def perform(pipeline_id, recipients = nil) diff --git a/app/workers/pipeline_process_worker.rb b/app/workers/pipeline_process_worker.rb index 8c067d05081..07dbf6a971e 100644 --- a/app/workers/pipeline_process_worker.rb +++ b/app/workers/pipeline_process_worker.rb @@ -1,5 +1,5 @@ class PipelineProcessWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/pipeline_schedule_worker.rb b/app/workers/pipeline_schedule_worker.rb index 7320db1065e..c49758878a4 100644 --- a/app/workers/pipeline_schedule_worker.rb +++ b/app/workers/pipeline_schedule_worker.rb @@ -1,5 +1,5 @@ class PipelineScheduleWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/pipeline_success_worker.rb b/app/workers/pipeline_success_worker.rb index cb8bb2ffe75..68c40a259e1 100644 --- a/app/workers/pipeline_success_worker.rb +++ b/app/workers/pipeline_success_worker.rb @@ -1,5 +1,5 @@ class PipelineSuccessWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/pipeline_update_worker.rb b/app/workers/pipeline_update_worker.rb index 5fa399dff4c..24a8a9fbed5 100644 --- a/app/workers/pipeline_update_worker.rb +++ b/app/workers/pipeline_update_worker.rb @@ -1,5 +1,5 @@ class PipelineUpdateWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index b8f8d3750d9..f2b2c4428d3 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -1,6 +1,5 @@ class PostReceive - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(gl_repository, identifier, changes) project, is_wiki = Gitlab::GlRepository.parse(gl_repository) diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb index c0c03848a40..52eebe475ec 100644 --- a/app/workers/process_commit_worker.rb +++ b/app/workers/process_commit_worker.rb @@ -5,8 +5,7 @@ # Consider using an extra worker if you need to add any extra (and potentially # slow) processing of commits. class ProcessCommitWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker # project_id - The ID of the project this commit belongs to. # user_id - The ID of the user that pushed the commit. diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index 505ff9e086e..f19bcbf946a 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -1,7 +1,6 @@ # Worker for updating any project specific caches. class ProjectCacheWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker LEASE_TIMEOUT = 15.minutes.to_i diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb index 3be7e686609..1ba854ca4cb 100644 --- a/app/workers/project_destroy_worker.rb +++ b/app/workers/project_destroy_worker.rb @@ -1,6 +1,5 @@ class ProjectDestroyWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include ExceptionBacktrace def perform(project_id, user_id, params) diff --git a/app/workers/project_export_worker.rb b/app/workers/project_export_worker.rb index f13ac9e5db2..c100852374a 100644 --- a/app/workers/project_export_worker.rb +++ b/app/workers/project_export_worker.rb @@ -1,6 +1,5 @@ class ProjectExportWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include ExceptionBacktrace sidekiq_options retry: 3 diff --git a/app/workers/project_migrate_hashed_storage_worker.rb b/app/workers/project_migrate_hashed_storage_worker.rb index 127aa6b9d7d..d01eb744e5d 100644 --- a/app/workers/project_migrate_hashed_storage_worker.rb +++ b/app/workers/project_migrate_hashed_storage_worker.rb @@ -1,6 +1,5 @@ class ProjectMigrateHashedStorageWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker LEASE_TIMEOUT = 30.seconds.to_i diff --git a/app/workers/project_service_worker.rb b/app/workers/project_service_worker.rb index 4883d848c53..75c4b8b3663 100644 --- a/app/workers/project_service_worker.rb +++ b/app/workers/project_service_worker.rb @@ -1,6 +1,5 @@ class ProjectServiceWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker sidekiq_options dead: false diff --git a/app/workers/propagate_service_template_worker.rb b/app/workers/propagate_service_template_worker.rb index 6b607451c7a..635a97c99af 100644 --- a/app/workers/propagate_service_template_worker.rb +++ b/app/workers/propagate_service_template_worker.rb @@ -1,7 +1,6 @@ # Worker for updating any project specific caches. class PropagateServiceTemplateWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker LEASE_TIMEOUT = 4.hours.to_i diff --git a/app/workers/prune_old_events_worker.rb b/app/workers/prune_old_events_worker.rb index 2b43bb19ad1..5ff62ab1369 100644 --- a/app/workers/prune_old_events_worker.rb +++ b/app/workers/prune_old_events_worker.rb @@ -1,5 +1,5 @@ class PruneOldEventsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/reactive_caching_worker.rb b/app/workers/reactive_caching_worker.rb index 18b8daf4e1e..ef3ddb9024b 100644 --- a/app/workers/reactive_caching_worker.rb +++ b/app/workers/reactive_caching_worker.rb @@ -1,6 +1,5 @@ class ReactiveCachingWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(class_name, id, *args) klass = begin diff --git a/app/workers/remove_expired_group_links_worker.rb b/app/workers/remove_expired_group_links_worker.rb index 2a619f83410..7e64c3070a8 100644 --- a/app/workers/remove_expired_group_links_worker.rb +++ b/app/workers/remove_expired_group_links_worker.rb @@ -1,5 +1,5 @@ class RemoveExpiredGroupLinksWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index 31f652e5f9b..d80b3b15840 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -1,5 +1,5 @@ class RemoveExpiredMembersWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/remove_old_web_hook_logs_worker.rb b/app/workers/remove_old_web_hook_logs_worker.rb index 555e1bb8691..87fed42d7ce 100644 --- a/app/workers/remove_old_web_hook_logs_worker.rb +++ b/app/workers/remove_old_web_hook_logs_worker.rb @@ -1,5 +1,5 @@ class RemoveOldWebHookLogsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue WEB_HOOK_LOG_LIFETIME = 2.days diff --git a/app/workers/remove_unreferenced_lfs_objects_worker.rb b/app/workers/remove_unreferenced_lfs_objects_worker.rb index b80f131d5f7..8daf079fc31 100644 --- a/app/workers/remove_unreferenced_lfs_objects_worker.rb +++ b/app/workers/remove_unreferenced_lfs_objects_worker.rb @@ -1,5 +1,5 @@ class RemoveUnreferencedLfsObjectsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/repository_archive_cache_worker.rb b/app/workers/repository_archive_cache_worker.rb index e47069df189..86a258cf94f 100644 --- a/app/workers/repository_archive_cache_worker.rb +++ b/app/workers/repository_archive_cache_worker.rb @@ -1,5 +1,5 @@ class RepositoryArchiveCacheWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/repository_check/batch_worker.rb b/app/workers/repository_check/batch_worker.rb index b94d83bd709..76688cf51c1 100644 --- a/app/workers/repository_check/batch_worker.rb +++ b/app/workers/repository_check/batch_worker.rb @@ -1,6 +1,6 @@ module RepositoryCheck class BatchWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue RUN_TIME = 3600 diff --git a/app/workers/repository_check/clear_worker.rb b/app/workers/repository_check/clear_worker.rb index 85bc9103538..97b89dc3db5 100644 --- a/app/workers/repository_check/clear_worker.rb +++ b/app/workers/repository_check/clear_worker.rb @@ -1,6 +1,6 @@ module RepositoryCheck class ClearWorker - include Sidekiq::Worker + include ApplicationWorker include RepositoryCheckQueue def perform diff --git a/app/workers/repository_check/single_repository_worker.rb b/app/workers/repository_check/single_repository_worker.rb index 3d4bee15f1c..4e3c691e8da 100644 --- a/app/workers/repository_check/single_repository_worker.rb +++ b/app/workers/repository_check/single_repository_worker.rb @@ -1,6 +1,6 @@ module RepositoryCheck class SingleRepositoryWorker - include Sidekiq::Worker + include ApplicationWorker include RepositoryCheckQueue def perform(project_id) diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 001c11b73e1..a07ef1705a1 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -1,9 +1,8 @@ class RepositoryForkWorker ForkError = Class.new(StandardError) - include Sidekiq::Worker + include ApplicationWorker include Gitlab::ShellAdapter - include DedicatedSidekiqQueue include ProjectStartImport sidekiq_options status_expiration: StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index 4e90b137b26..55715c83cb1 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -1,8 +1,7 @@ class RepositoryImportWorker ImportError = Class.new(StandardError) - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker include ExceptionBacktrace include ProjectStartImport diff --git a/app/workers/requests_profiles_worker.rb b/app/workers/requests_profiles_worker.rb index 703b025d76e..55c236e9e9d 100644 --- a/app/workers/requests_profiles_worker.rb +++ b/app/workers/requests_profiles_worker.rb @@ -1,5 +1,5 @@ class RequestsProfilesWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/schedule_update_user_activity_worker.rb b/app/workers/schedule_update_user_activity_worker.rb index 6c2c3e437f3..d9376577597 100644 --- a/app/workers/schedule_update_user_activity_worker.rb +++ b/app/workers/schedule_update_user_activity_worker.rb @@ -1,5 +1,5 @@ class ScheduleUpdateUserActivityWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform(batch_size = 500) diff --git a/app/workers/stage_update_worker.rb b/app/workers/stage_update_worker.rb index c301cea5ad6..69f2318d83b 100644 --- a/app/workers/stage_update_worker.rb +++ b/app/workers/stage_update_worker.rb @@ -1,5 +1,5 @@ class StageUpdateWorker - include Sidekiq::Worker + include ApplicationWorker include PipelineQueue enqueue_in group: :processing diff --git a/app/workers/storage_migrator_worker.rb b/app/workers/storage_migrator_worker.rb index b48ead799b9..f92421a667d 100644 --- a/app/workers/storage_migrator_worker.rb +++ b/app/workers/storage_migrator_worker.rb @@ -1,6 +1,5 @@ class StorageMigratorWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker BATCH_SIZE = 100 diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 367e227f680..fb26fa4c515 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -1,5 +1,5 @@ class StuckCiJobsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue EXCLUSIVE_LEASE_KEY = 'stuck_ci_builds_worker_lease'.freeze diff --git a/app/workers/stuck_import_jobs_worker.rb b/app/workers/stuck_import_jobs_worker.rb index f850e459cd9..e0e6d1418de 100644 --- a/app/workers/stuck_import_jobs_worker.rb +++ b/app/workers/stuck_import_jobs_worker.rb @@ -1,5 +1,5 @@ class StuckImportJobsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue IMPORT_JOBS_EXPIRATION = 15.hours.to_i diff --git a/app/workers/stuck_merge_jobs_worker.rb b/app/workers/stuck_merge_jobs_worker.rb index a396c0f27b2..36d2a2e6466 100644 --- a/app/workers/stuck_merge_jobs_worker.rb +++ b/app/workers/stuck_merge_jobs_worker.rb @@ -1,5 +1,5 @@ class StuckMergeJobsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/system_hook_push_worker.rb b/app/workers/system_hook_push_worker.rb index e43bbe35de9..ceeaaf8d189 100644 --- a/app/workers/system_hook_push_worker.rb +++ b/app/workers/system_hook_push_worker.rb @@ -1,6 +1,5 @@ class SystemHookPushWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(push_data, hook_id) SystemHooksService.new.execute_hooks(push_data, hook_id) diff --git a/app/workers/trending_projects_worker.rb b/app/workers/trending_projects_worker.rb index 0531630d13a..7eb65452a7d 100644 --- a/app/workers/trending_projects_worker.rb +++ b/app/workers/trending_projects_worker.rb @@ -1,5 +1,5 @@ class TrendingProjectsWorker - include Sidekiq::Worker + include ApplicationWorker include CronjobQueue def perform diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index afc47fc63d6..74bb9993275 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -1,6 +1,5 @@ class UpdateMergeRequestsWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker LOG_TIME_THRESHOLD = 90 # seconds diff --git a/app/workers/update_user_activity_worker.rb b/app/workers/update_user_activity_worker.rb index 31bbdb69edb..27ec5cd33fb 100644 --- a/app/workers/update_user_activity_worker.rb +++ b/app/workers/update_user_activity_worker.rb @@ -1,6 +1,5 @@ class UpdateUserActivityWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(pairs) pairs = cast_data(pairs) diff --git a/app/workers/upload_checksum_worker.rb b/app/workers/upload_checksum_worker.rb index 78931f1258f..9222760c031 100644 --- a/app/workers/upload_checksum_worker.rb +++ b/app/workers/upload_checksum_worker.rb @@ -1,6 +1,5 @@ class UploadChecksumWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker def perform(upload_id) upload = Upload.find(upload_id) diff --git a/app/workers/wait_for_cluster_creation_worker.rb b/app/workers/wait_for_cluster_creation_worker.rb index 241ed3901dc..19cdb279aaa 100644 --- a/app/workers/wait_for_cluster_creation_worker.rb +++ b/app/workers/wait_for_cluster_creation_worker.rb @@ -1,5 +1,5 @@ class WaitForClusterCreationWorker - include Sidekiq::Worker + include ApplicationWorker include ClusterQueue def perform(cluster_id) diff --git a/app/workers/web_hook_worker.rb b/app/workers/web_hook_worker.rb index 713c0228040..dfc3f33ad9d 100644 --- a/app/workers/web_hook_worker.rb +++ b/app/workers/web_hook_worker.rb @@ -1,6 +1,5 @@ class WebHookWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker sidekiq_options retry: 4, dead: false diff --git a/config/initializers/forbid_sidekiq_in_transactions.rb b/config/initializers/forbid_sidekiq_in_transactions.rb index a78711fe599..bedd57ede04 100644 --- a/config/initializers/forbid_sidekiq_in_transactions.rb +++ b/config/initializers/forbid_sidekiq_in_transactions.rb @@ -13,20 +13,19 @@ module Sidekiq module ClassMethods module NoSchedulingFromTransactions - NESTING = ::Rails.env.test? ? 1 : 0 - %i(perform_async perform_at perform_in).each do |name| define_method(name) do |*args| - return super(*args) if Sidekiq::Worker.skip_transaction_check - return super(*args) unless ActiveRecord::Base.connection.open_transactions > NESTING + if !Sidekiq::Worker.skip_transaction_check && AfterCommitQueue.inside_transaction? + raise <<-MSG.strip_heredoc + `#{self}.#{name}` cannot be called inside a transaction as this can lead to + race conditions when the worker runs before the transaction is committed and + tries to access a model that has not been saved yet. - raise <<-MSG.strip_heredoc - `#{self}.#{name}` cannot be called inside a transaction as this can lead to - race conditions when the worker runs before the transaction is committed and - tries to access a model that has not been saved yet. + Use an `after_commit` hook, or include `AfterCommitQueue` and use a `run_after_commit` block instead. + MSG + end - Use an `after_commit` hook, or include `AfterCommitQueue` and use a `run_after_commit` block instead. - MSG + super(*args) end end end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index a1cc9655319..ba4481ae602 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -64,13 +64,13 @@ end # The Sidekiq client API always adds the queue to the Sidekiq queue # list, but mail_room and gitlab-shell do not. This is only necessary # for monitoring. -config = YAML.load_file(Rails.root.join('config', 'sidekiq_queues.yml').to_s) - begin + queues = Gitlab::SidekiqConfig.worker_queues + Sidekiq.redis do |conn| conn.pipelined do - config[:queues].each do |queue| - conn.sadd('queues', queue[0]) + queues.each do |queue| + conn.sadd('queues', queue) end end end diff --git a/db/post_migrate/20170627101016_schedule_event_migrations.rb b/db/post_migrate/20170627101016_schedule_event_migrations.rb index 1f34375ff0d..1e020d05f78 100644 --- a/db/post_migrate/20170627101016_schedule_event_migrations.rb +++ b/db/post_migrate/20170627101016_schedule_event_migrations.rb @@ -25,14 +25,14 @@ class ScheduleEventMigrations < ActiveRecord::Migration # We push multiple jobs at a time to reduce the time spent in # Sidekiq/Redis operations. We're using this buffer based approach so we # don't need to run additional queries for every range. - BackgroundMigrationWorker.perform_bulk(jobs) + BackgroundMigrationWorker.bulk_perform_async(jobs) jobs.clear end jobs << ['MigrateEventsToPushEventPayloads', [min, max]] end - BackgroundMigrationWorker.perform_bulk(jobs) unless jobs.empty? + BackgroundMigrationWorker.bulk_perform_async(jobs) unless jobs.empty? end def down diff --git a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb index 01d56fbd490..467c584c2e0 100644 --- a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb +++ b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb @@ -19,7 +19,7 @@ class ScheduleCreateGpgKeySubkeysFromGpgKeys < ActiveRecord::Migration [MIGRATION, [id]] end - BackgroundMigrationWorker.perform_bulk(jobs) + BackgroundMigrationWorker.bulk_perform_async(jobs) end end diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 5452b0e7a2f..fd2b9d0e908 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -68,10 +68,10 @@ BackgroundMigrationWorker.perform_async('BackgroundMigrationClassName', [arg1, a ``` Usually it's better to enqueue jobs in bulk, for this you can use -`BackgroundMigrationWorker.perform_bulk`: +`BackgroundMigrationWorker.bulk_perform_async`: ```ruby -BackgroundMigrationWorker.perform_bulk( +BackgroundMigrationWorker.bulk_perform_async( [['BackgroundMigrationClassName', [1]], ['BackgroundMigrationClassName', [2]]] ) @@ -85,13 +85,13 @@ updates. Removals in turn can be handled by simply defining foreign keys with cascading deletes. If you would like to schedule jobs in bulk with a delay, you can use -`BackgroundMigrationWorker.perform_bulk_in`: +`BackgroundMigrationWorker.bulk_perform_in`: ```ruby jobs = [['BackgroundMigrationClassName', [1]], ['BackgroundMigrationClassName', [2]]] -BackgroundMigrationWorker.perform_bulk_in(5.minutes, jobs) +BackgroundMigrationWorker.bulk_perform_in(5.minutes, jobs) ``` ## Cleaning Up @@ -201,7 +201,7 @@ class ScheduleExtractServicesUrl < ActiveRecord::Migration ['ExtractServicesUrl', [id]] end - BackgroundMigrationWorker.perform_bulk(jobs) + BackgroundMigrationWorker.bulk_perform_async(jobs) end end diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 1e9fdbc65e2..085fb8f902c 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -3,6 +3,12 @@ This document outlines various guidelines that should be followed when adding or modifying Sidekiq workers. +## ApplicationWorker + +All workers should include `ApplicationWorker` instead of `Sidekiq::Worker`, +which adds some convenience methods and automatically sets the queue based on +the worker's name. + ## Default Queue Use of the "default" queue is not allowed. Every worker should use a queue that @@ -13,19 +19,10 @@ A list of all available queues can be found in `config/sidekiq_queues.yml`. ## Dedicated Queues -Most workers should use their own queue. To ease this process a worker can -include the `DedicatedSidekiqQueue` concern as follows: - -```ruby -class ProcessSomethingWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue -end -``` - -This will set the queue name based on the class' name, minus the `Worker` -suffix. In the above example this would lead to the queue being -`process_something`. +Most workers should use their own queue, which is automatically set based on the +worker class name. For a worker named `ProcessSomethingWorker`, the queue name +would be `process_something`. If you're not sure what a worker's queue name is, +you can find it using `SomeWorker.queue`. In some cases multiple workers do use the same queue. For example, the various workers for updating CI pipelines all use the `pipeline` queue. Adding workers @@ -39,7 +36,7 @@ tests should be placed in `spec/workers`. ## Removing or renaming queues -Try to avoid renaming or removing queues in minor and patch releases. -During online update instance can have pending jobs and removing the queue can -lead to those jobs being stuck forever. If you can't write migration for those -Sidekiq jobs, please consider doing rename or remove queue in major release only.
\ No newline at end of file +Try to avoid renaming or removing queues in minor and patch releases. +During online update instance can have pending jobs and removing the queue can +lead to those jobs being stuck forever. If you can't write migration for those +Sidekiq jobs, please consider doing rename or remove queue in major release only. diff --git a/lib/after_commit_queue.rb b/lib/after_commit_queue.rb index 4750a2c373a..db63c5038ae 100644 --- a/lib/after_commit_queue.rb +++ b/lib/after_commit_queue.rb @@ -6,12 +6,34 @@ module AfterCommitQueue after_rollback :_clear_after_commit_queue end - def run_after_commit(method = nil, &block) - _after_commit_queue << proc { self.send(method) } if method # rubocop:disable GitlabSecurity/PublicSend + def run_after_commit(&block) _after_commit_queue << block if block + + true + end + + def run_after_commit_or_now(&block) + if AfterCommitQueue.inside_transaction? + run_after_commit(&block) + else + instance_eval(&block) + end + true end + def self.open_transactions_baseline + if ::Rails.env.test? + return DatabaseCleaner.connections.count { |conn| conn.strategy.is_a?(DatabaseCleaner::ActiveRecord::Transaction) } + end + + 0 + end + + def self.inside_transaction? + ActiveRecord::Base.connection.open_transactions > open_transactions_baseline + end + protected def _run_after_commit_queue diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index c276c3566b4..3f65bc912de 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -703,14 +703,14 @@ into similar problems in the future (e.g. when new tables are created). # We push multiple jobs at a time to reduce the time spent in # Sidekiq/Redis operations. We're using this buffer based approach so we # don't need to run additional queries for every range. - BackgroundMigrationWorker.perform_bulk(jobs) + BackgroundMigrationWorker.bulk_perform_async(jobs) jobs.clear end jobs << [job_class_name, [start_id, end_id]] end - BackgroundMigrationWorker.perform_bulk(jobs) unless jobs.empty? + BackgroundMigrationWorker.bulk_perform_async(jobs) unless jobs.empty? end # Queues background migration jobs for an entire table, batched by ID range. diff --git a/lib/gitlab/sidekiq_config.rb b/lib/gitlab/sidekiq_config.rb new file mode 100644 index 00000000000..dc9886732b5 --- /dev/null +++ b/lib/gitlab/sidekiq_config.rb @@ -0,0 +1,50 @@ +require 'yaml' + +module Gitlab + module SidekiqConfig + def self.redis_queues + @redis_queues ||= Sidekiq::Queue.all.map(&:name) + end + + # This method is called by `bin/sidekiq-cluster` in EE, which runs outside + # of bundler/Rails context, so we cannot use any gem or Rails methods. + def self.config_queues(rails_path = Rails.root.to_s) + @config_queues ||= begin + config = YAML.load_file(File.join(rails_path, 'config', 'sidekiq_queues.yml')) + config[:queues].map(&:first) + end + end + + def self.cron_workers + @cron_workers ||= Settings.cron_jobs.map { |job_name, options| options['job_class'].constantize } + end + + def self.workers + @workers ||= find_workers(Rails.root.join('app', 'workers')) + end + + def self.default_queues + [ActionMailer::DeliveryJob.queue_name, 'default'] + end + + def self.worker_queues + @worker_queues ||= (workers.map(&:queue) + default_queues).uniq + end + + def self.find_workers(root) + concerns = root.join('concerns').to_s + + workers = Dir[root.join('**', '*.rb')] + .reject { |path| path.start_with?(concerns) } + + workers.map! do |path| + ns = Pathname.new(path).relative_path_from(root).to_s.gsub('.rb', '') + + ns.camelize.constantize + end + + # Skip concerns + workers.select { |w| w < Sidekiq::Worker } + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 3c8350b3aad..664ba0f7234 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -942,8 +942,8 @@ describe Gitlab::Database::MigrationHelpers do end it 'queues jobs in groups of buffer size 1' do - expect(BackgroundMigrationWorker).to receive(:perform_bulk).with([['FooJob', [id1, id2]]]) - expect(BackgroundMigrationWorker).to receive(:perform_bulk).with([['FooJob', [id3, id3]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id3, id3]]]) model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) end @@ -960,8 +960,8 @@ describe Gitlab::Database::MigrationHelpers do end it 'queues jobs in bulk all at once (big buffer size)' do - expect(BackgroundMigrationWorker).to receive(:perform_bulk).with([['FooJob', [id1, id2]], - ['FooJob', [id3, id3]]]) + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([['FooJob', [id1, id2]], + ['FooJob', [id3, id3]]]) model.bulk_queue_background_migration_jobs_by_range(User, 'FooJob', batch_size: 2) end diff --git a/spec/lib/gitlab/sidekiq_config_spec.rb b/spec/lib/gitlab/sidekiq_config_spec.rb new file mode 100644 index 00000000000..09f95be2213 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_config_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +describe Gitlab::SidekiqConfig do + describe '.workers' do + it 'includes all workers' do + workers = described_class.workers + + expect(workers).to include(PostReceive) + expect(workers).to include(MergeWorker) + end + end + + describe '.worker_queues' do + it 'includes all queues' do + queues = described_class.worker_queues + + expect(queues).to include('post_receive') + expect(queues).to include('merge') + expect(queues).to include('cronjob') + expect(queues).to include('mailers') + expect(queues).to include('default') + end + end +end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index a669429ce3e..21910e69d2e 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -146,7 +146,7 @@ describe WebHookService do let(:system_hook) { create(:system_hook) } it 'enqueue WebHookWorker' do - expect(Sidekiq::Client).to receive(:enqueue).with(WebHookWorker, project_hook.id, data, 'push_hooks') + expect(WebHookWorker).to receive(:perform_async).with(project_hook.id, data, 'push_hooks') described_class.new(project_hook, data, 'push_hooks').async_execute end diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb index 90ed1309d4a..0d6eb536c33 100644 --- a/spec/workers/authorized_projects_worker_spec.rb +++ b/spec/workers/authorized_projects_worker_spec.rb @@ -65,7 +65,6 @@ describe AuthorizedProjectsWorker do args_list = build_args_list(project.owner.id) push_bulk_args = { 'class' => described_class, - 'queue' => described_class.sidekiq_options['queue'], 'args' => args_list } diff --git a/spec/workers/background_migration_worker_spec.rb b/spec/workers/background_migration_worker_spec.rb index 4f6e3474634..1c54cf55fa0 100644 --- a/spec/workers/background_migration_worker_spec.rb +++ b/spec/workers/background_migration_worker_spec.rb @@ -10,35 +10,4 @@ describe BackgroundMigrationWorker, :sidekiq do described_class.new.perform('Foo', [10, 20]) end end - - describe '.perform_bulk' do - it 'enqueues background migrations in bulk' do - Sidekiq::Testing.fake! do - described_class.perform_bulk([['Foo', [1]], ['Foo', [2]]]) - - expect(described_class.jobs.count).to eq 2 - expect(described_class.jobs).to all(include('enqueued_at')) - end - end - end - - describe '.perform_bulk_in' do - context 'when delay is valid' do - it 'correctly schedules background migrations' do - Sidekiq::Testing.fake! do - described_class.perform_bulk_in(1.minute, [['Foo', [1]], ['Foo', [2]]]) - - expect(described_class.jobs.count).to eq 2 - expect(described_class.jobs).to all(include('at')) - end - end - end - - context 'when delay is invalid' do - it 'raises an ArgumentError exception' do - expect { described_class.perform_bulk_in(-60, [['Foo']]) } - .to raise_error(ArgumentError) - end - end - end end diff --git a/spec/workers/concerns/application_worker_spec.rb b/spec/workers/concerns/application_worker_spec.rb new file mode 100644 index 00000000000..0145563e0ed --- /dev/null +++ b/spec/workers/concerns/application_worker_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe ApplicationWorker do + let(:worker) do + Class.new do + def self.name + 'Gitlab::Foo::Bar::DummyWorker' + end + + include ApplicationWorker + end + end + + describe 'Sidekiq options' do + it 'sets the queue name based on the class name' do + expect(worker.sidekiq_options['queue']).to eq('foo_bar_dummy') + end + end + + describe '.queue' do + it 'returns the queue name' do + worker.sidekiq_options queue: :some_queue + + expect(worker.queue).to eq('some_queue') + end + end + + describe '.bulk_perform_async' do + it 'enqueues jobs in bulk' do + Sidekiq::Testing.fake! do + worker.bulk_perform_async([['Foo', [1]], ['Foo', [2]]]) + + expect(worker.jobs.count).to eq 2 + expect(worker.jobs).to all(include('enqueued_at')) + end + end + end + + describe '.bulk_perform_in' do + context 'when delay is valid' do + it 'correctly schedules jobs' do + Sidekiq::Testing.fake! do + worker.bulk_perform_in(1.minute, [['Foo', [1]], ['Foo', [2]]]) + + expect(worker.jobs.count).to eq 2 + expect(worker.jobs).to all(include('at')) + end + end + end + + context 'when delay is invalid' do + it 'raises an ArgumentError exception' do + expect { worker.bulk_perform_in(-60, [['Foo']]) } + .to raise_error(ArgumentError) + end + end + end +end diff --git a/spec/workers/concerns/cluster_queue_spec.rb b/spec/workers/concerns/cluster_queue_spec.rb index 1050651fa51..5049886b55c 100644 --- a/spec/workers/concerns/cluster_queue_spec.rb +++ b/spec/workers/concerns/cluster_queue_spec.rb @@ -3,7 +3,11 @@ require 'spec_helper' describe ClusterQueue do let(:worker) do Class.new do - include Sidekiq::Worker + def self.name + 'DummyWorker' + end + + include ApplicationWorker include ClusterQueue end end diff --git a/spec/workers/concerns/cronjob_queue_spec.rb b/spec/workers/concerns/cronjob_queue_spec.rb index 5d1336c21a6..3ae1c5f54d8 100644 --- a/spec/workers/concerns/cronjob_queue_spec.rb +++ b/spec/workers/concerns/cronjob_queue_spec.rb @@ -3,7 +3,11 @@ require 'spec_helper' describe CronjobQueue do let(:worker) do Class.new do - include Sidekiq::Worker + def self.name + 'DummyWorker' + end + + include ApplicationWorker include CronjobQueue end end diff --git a/spec/workers/concerns/dedicated_sidekiq_queue_spec.rb b/spec/workers/concerns/dedicated_sidekiq_queue_spec.rb deleted file mode 100644 index 512baec8b7e..00000000000 --- a/spec/workers/concerns/dedicated_sidekiq_queue_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'spec_helper' - -describe DedicatedSidekiqQueue do - let(:worker) do - Class.new do - def self.name - 'Foo::Bar::DummyWorker' - end - - include Sidekiq::Worker - include DedicatedSidekiqQueue - end - end - - describe 'queue names' do - it 'sets the queue name based on the class name' do - expect(worker.sidekiq_options['queue']).to eq('foo_bar_dummy') - end - end -end diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb index 3ccf06f2d7d..68cfe9d5545 100644 --- a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' describe Gitlab::GithubImport::ObjectImporter do let(:worker) do Class.new do + def self.name + 'DummyWorker' + end + include(Gitlab::GithubImport::ObjectImporter) def counter_name diff --git a/spec/workers/concerns/gitlab/github_import/queue_spec.rb b/spec/workers/concerns/gitlab/github_import/queue_spec.rb index 321ae3fe978..9c69ee32da1 100644 --- a/spec/workers/concerns/gitlab/github_import/queue_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/queue_spec.rb @@ -3,7 +3,11 @@ require 'spec_helper' describe Gitlab::GithubImport::Queue do it 'sets the Sidekiq options for the worker' do worker = Class.new do - include Sidekiq::Worker + def self.name + 'DummyWorker' + end + + include ApplicationWorker include Gitlab::GithubImport::Queue end diff --git a/spec/workers/concerns/pipeline_queue_spec.rb b/spec/workers/concerns/pipeline_queue_spec.rb index eac5a770e5f..dd911760948 100644 --- a/spec/workers/concerns/pipeline_queue_spec.rb +++ b/spec/workers/concerns/pipeline_queue_spec.rb @@ -3,7 +3,11 @@ require 'spec_helper' describe PipelineQueue do let(:worker) do Class.new do - include Sidekiq::Worker + def self.name + 'DummyWorker' + end + + include ApplicationWorker include PipelineQueue end end diff --git a/spec/workers/concerns/repository_check_queue_spec.rb b/spec/workers/concerns/repository_check_queue_spec.rb index 8868e969829..fdbbfcc90a5 100644 --- a/spec/workers/concerns/repository_check_queue_spec.rb +++ b/spec/workers/concerns/repository_check_queue_spec.rb @@ -3,7 +3,11 @@ require 'spec_helper' describe RepositoryCheckQueue do let(:worker) do Class.new do - include Sidekiq::Worker + def self.name + 'DummyWorker' + end + + include ApplicationWorker include RepositoryCheckQueue end end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 30908534eb3..7ee0a51a263 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -1,44 +1,21 @@ require 'spec_helper' describe 'Every Sidekiq worker' do - let(:workers) do - root = Rails.root.join('app', 'workers') - concerns = root.join('concerns').to_s - - workers = Dir[root.join('**', '*.rb')] - .reject { |path| path.start_with?(concerns) } - - workers.map do |path| - ns = Pathname.new(path).relative_path_from(root).to_s.gsub('.rb', '') - - ns.camelize.constantize - end + it 'includes ApplicationWorker' do + expect(Gitlab::SidekiqConfig.workers).to all(include(ApplicationWorker)) end it 'does not use the default queue' do - workers.each do |worker| - expect(worker.sidekiq_options['queue'].to_s).not_to eq('default') - end + expect(Gitlab::SidekiqConfig.workers.map(&:queue)).not_to include('default') end it 'uses the cronjob queue when the worker runs as a cronjob' do - cron_workers = Settings.cron_jobs - .map { |job_name, options| options['job_class'].constantize } - .to_set - - workers.each do |worker| - next unless cron_workers.include?(worker) - - expect(worker.sidekiq_options['queue'].to_s).to eq('cronjob') - end + expect(Gitlab::SidekiqConfig.cron_workers.map(&:queue)).to all(eq('cronjob')) end it 'defines the queue in the Sidekiq configuration file' do - config = YAML.load_file(Rails.root.join('config', 'sidekiq_queues.yml').to_s) - queue_names = config[:queues].map { |(queue, _)| queue }.to_set + config_queue_names = Gitlab::SidekiqConfig.config_queues.to_set - workers.each do |worker| - expect(queue_names).to include(worker.sidekiq_options['queue'].to_s) - end + expect(Gitlab::SidekiqConfig.worker_queues).to all(be_in(config_queue_names)) end end |