diff --git a/app/workers/ci/create_downstream_pipeline_worker.rb b/app/workers/ci/create_downstream_pipeline_worker.rb index 9f5ff45b8a6597375160839d63fe198998d0081d..fcfb077d4b9c4a1b026c8e983d4cebde75918d88 100644 --- a/app/workers/ci/create_downstream_pipeline_worker.rb +++ b/app/workers/ci/create_downstream_pipeline_worker.rb @@ -10,6 +10,8 @@ class CreateDownstreamPipelineWorker # rubocop:disable Scalability/IdempotentWor urgency :high def perform(bridge_id) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464668') + ::Ci::Bridge.find_by_id(bridge_id).try do |bridge| result = ::Ci::CreateDownstreamPipelineService .new(bridge.project, bridge.user) diff --git a/app/workers/create_pipeline_worker.rb b/app/workers/create_pipeline_worker.rb index 3c442760d3830fc6b3b4fc1219353f6b8a643041..f183a222138269b88976590e58446703d2b06638 100644 --- a/app/workers/create_pipeline_worker.rb +++ b/app/workers/create_pipeline_worker.rb @@ -15,6 +15,8 @@ class CreatePipelineWorker # rubocop:disable Scalability/IdempotentWorker loggable_arguments 2, 3, 4 def perform(project_id, user_id, ref, source, params = {}) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464671') + project = Project.find(project_id) user = User.find(user_id) params = params.deep_symbolize_keys diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb index 4634ea8ff4ffb430cb412216c402c6e66321b56a..097dfc060d9a6abd74f1e53462969bcd8ab47853 100644 --- a/app/workers/delete_user_worker.rb +++ b/app/workers/delete_user_worker.rb @@ -11,6 +11,8 @@ class DeleteUserWorker # rubocop:disable Scalability/IdempotentWorker loggable_arguments 2 def perform(current_user_id, delete_user_id, options = {}) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464672') + delete_user = User.find_by_id(delete_user_id) return unless delete_user.present? diff --git a/app/workers/group_destroy_worker.rb b/app/workers/group_destroy_worker.rb index d8dd0b6d7b3bd7815822947b2ecf1689486a1907..28e0735f5d6a16468098a582346b3733fc09de40 100644 --- a/app/workers/group_destroy_worker.rb +++ b/app/workers/group_destroy_worker.rb @@ -14,6 +14,8 @@ class GroupDestroyWorker deduplicate :until_executed, ttl: 2.hours def perform(group_id, user_id) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464673') + begin group = Group.find(group_id) rescue ActiveRecord::RecordNotFound diff --git a/app/workers/group_import_worker.rb b/app/workers/group_import_worker.rb index 198c6274166ee3119de1b400ca5650d5ab61bed6..a6521e7804be28b8face10219b4951ea7a9ca69a 100644 --- a/app/workers/group_import_worker.rb +++ b/app/workers/group_import_worker.rb @@ -9,6 +9,8 @@ class GroupImportWorker # rubocop:disable Scalability/IdempotentWorker feature_category :importers def perform(user_id, group_id) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464675') + current_user = User.find(user_id) group = Group.find(group_id) group_import_state = group.import_state diff --git a/app/workers/mail_scheduler/notification_service_worker.rb b/app/workers/mail_scheduler/notification_service_worker.rb index 8206a17021bbc10797b7eb99e12e10bbcc11c06a..7f3066f883ef9e352c4f5582cfb7f0d75aa693ca 100644 --- a/app/workers/mail_scheduler/notification_service_worker.rb +++ b/app/workers/mail_scheduler/notification_service_worker.rb @@ -16,6 +16,8 @@ class NotificationServiceWorker # rubocop:disable Scalability/IdempotentWorker loggable_arguments 0 def perform(meth, *args) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464670') + check_arguments!(args) if NotificationService.permitted_actions.exclude?(meth.to_sym) diff --git a/app/workers/merge_requests/create_pipeline_worker.rb b/app/workers/merge_requests/create_pipeline_worker.rb index e92e557284c070653fb5443108ffe2a4ca75f9f4..12f114f49691fddbf708603801e3bff669b8cd13 100644 --- a/app/workers/merge_requests/create_pipeline_worker.rb +++ b/app/workers/merge_requests/create_pipeline_worker.rb @@ -16,6 +16,8 @@ class CreatePipelineWorker idempotent! def perform(project_id, user_id, merge_request_id, params = {}) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464679') + project = Project.find_by_id(project_id) return unless project diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 29f0c0bbbf4fa5764387ea0b6537c82d2b6459e4..2979a77e19f2779fb256a884c9c3fac18137626f 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -16,6 +16,8 @@ class MergeWorker # rubocop:disable Scalability/IdempotentWorker deduplicate :until_executed, including_scheduled: true def perform(merge_request_id, current_user_id, params) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464676') + begin current_user = User.find(current_user_id) merge_request = MergeRequest.find(merge_request_id) diff --git a/app/workers/new_merge_request_worker.rb b/app/workers/new_merge_request_worker.rb index e2e738f79a5792efd9aa6affa1746a6f80a71036..a11f26088b38e08502324b9daeaa5c98c7d95d4a 100644 --- a/app/workers/new_merge_request_worker.rb +++ b/app/workers/new_merge_request_worker.rb @@ -17,6 +17,8 @@ class NewMergeRequestWorker # rubocop:disable Scalability/IdempotentWorker weight 2 def perform(merge_request_id, user_id) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/337182') + return unless objects_found?(merge_request_id, user_id) return if issuable.prepared? diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb index 181eebe56e83e856156779a174196cfbcd6aff43..565dfab9930f5195ff47ff76f45b8c4d76106dae 100644 --- a/app/workers/project_destroy_worker.rb +++ b/app/workers/project_destroy_worker.rb @@ -14,6 +14,8 @@ class ProjectDestroyWorker deduplicate :until_executed, ttl: 2.hours def perform(project_id, user_id, params) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/333366') + project = Project.find(project_id) user = User.find(user_id) diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index c16071e8e31c05561734a2c344c16a79f2cf0489..94bbd812b68764dab13eaae5efef692d1d301949 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -15,6 +15,8 @@ class RepositoryImportWorker # rubocop:disable Scalability/IdempotentWorker worker_resource_boundary :memory def perform(project_id) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464677') + @project = Project.find_by_id(project_id) return if project.nil? || !start_import? diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index caf46c1ac4ef2dca3aafb915af91ea7e5c4daca4..d61b6e601b87a70b61b69c1c0ff7661f1677d842 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -14,6 +14,8 @@ class UpdateMergeRequestsWorker # rubocop:disable Scalability/IdempotentWorker loggable_arguments 2, 3, 4 def perform(project_id, user_id, oldrev, newrev, ref, params = {}) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/24907') + project = Project.find_by_id(project_id) return unless project diff --git a/ee/app/workers/merge_trains/refresh_worker.rb b/ee/app/workers/merge_trains/refresh_worker.rb index 521701c8b32dbf24f32d5692f381737fdaedc486..f1442d192df378a3a4058051abe9b9cd268d1533 100644 --- a/ee/app/workers/merge_trains/refresh_worker.rb +++ b/ee/app/workers/merge_trains/refresh_worker.rb @@ -17,6 +17,8 @@ class RefreshWorker idempotent! def perform(target_project_id, target_branch) + Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/464678') + ::MergeTrains::RefreshService .new(target_project_id, target_branch) .execute diff --git a/lib/gitlab/query_limiting/sidekiq_middleware.rb b/lib/gitlab/query_limiting/sidekiq_middleware.rb new file mode 100644 index 0000000000000000000000000000000000000000..cfeeecce3b7a33813ffd95e4b1bc07a5177308dc --- /dev/null +++ b/lib/gitlab/query_limiting/sidekiq_middleware.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module QueryLimiting + # Middleware for reporting (or raising) when a Sidekiq worker performs more than a + # certain amount of database queries. + class SidekiqMiddleware + def call(worker, _job, _queue) + transaction, retval = ::Gitlab::QueryLimiting::Transaction.run do + yield + end + + transaction.action = action_name(worker) + transaction.act_upon_results + + retval + end + + private + + def action_name(worker) + worker.class.name + end + end + end +end diff --git a/lib/gitlab/query_limiting/transaction.rb b/lib/gitlab/query_limiting/transaction.rb index 728b0da5049b009b11b8459783e996cbeb90c665..2b410f37d2a8b744f6a97f74e0ae018683875f3f 100644 --- a/lib/gitlab/query_limiting/transaction.rb +++ b/lib/gitlab/query_limiting/transaction.rb @@ -39,12 +39,14 @@ def self.current # # retval # => 10 def self.run + previous_transaction = current + transaction = new Thread.current[THREAD_KEY] = transaction [transaction, yield] ensure - Thread.current[THREAD_KEY] = nil + Thread.current[THREAD_KEY] = previous_transaction end def initialize diff --git a/spec/lib/gitlab/query_limiting/sidekiq_middleware_spec.rb b/spec/lib/gitlab/query_limiting/sidekiq_middleware_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3a1c8bdac94425af73c60f1cb8eeed7a0de11614 --- /dev/null +++ b/spec/lib/gitlab/query_limiting/sidekiq_middleware_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::QueryLimiting::SidekiqMiddleware, feature_category: :database do + describe '#call' do + let(:worker_class) do + Class.new do + def self.name + 'TestWorker' + end + + include ApplicationWorker + end + end + + let(:worker) { worker_class.new } + let(:job) { {} } + let(:queue) { :test } + + it 'runs the middleware with query limiting in place' do + expect_next_instance_of(Gitlab::QueryLimiting::Transaction) do |instance| + expect(instance).to receive(:action=).with('TestWorker') + expect(instance).to receive(:act_upon_results) + end + + middleware = described_class.new + + middleware.call(worker, job, queue) { nil } + end + + it 'yields block' do + middleware = described_class.new + + expect { |b| middleware.call(worker, job, queue, &b) }.to yield_control.once + end + + it 'returns value of block' do + middleware = described_class.new + + return_value = middleware.call(worker, job, queue) do + { value: 11 } + end + + expect(return_value).to eq({ value: 11 }) + end + end +end diff --git a/spec/lib/gitlab/query_limiting/transaction_spec.rb b/spec/lib/gitlab/query_limiting/transaction_spec.rb index 3ce8b08c77dc6adc5be76c8f4d78516868c9ca14..4c653a261fe6d20bb1ba7140fcaa094a17d76c03 100644 --- a/spec/lib/gitlab/query_limiting/transaction_spec.rb +++ b/spec/lib/gitlab/query_limiting/transaction_spec.rb @@ -36,6 +36,18 @@ expect(Thread.current[described_class::THREAD_KEY]).to be_nil end + + it 'restores the previous transaction upon completion' do + original_transaction = described_class.new + Thread.current[described_class::THREAD_KEY] = original_transaction + + new_transaction, _ret = described_class.run do + 10 + end + + expect(new_transaction).not_to eq(original_transaction) + expect(Thread.current[described_class::THREAD_KEY]).to eq(original_transaction) + end end describe '#act_upon_results' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 63dbf36b9e92c3c12bbe741a735b73c68da56355..9e1b7983121ccac5b506ce35a044eeac6a36597b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -436,7 +436,8 @@ arguments_logger: false, # We're not logging the regular messages for inline jobs skip_jobs: false # We're not skipping jobs for inline tests ).call(chain) - chain.add DisableQueryLimit + + chain.insert_after ::Gitlab::SidekiqMiddleware::RequestStoreMiddleware, ::Gitlab::QueryLimiting::SidekiqMiddleware chain.insert_after ::Gitlab::SidekiqMiddleware::RequestStoreMiddleware, IsolatedRequestStore example.run diff --git a/spec/support/sidekiq_middleware.rb b/spec/support/sidekiq_middleware.rb index cbd6163d46b25a061d76d3b93e99d9ff77ae7dd9..ae3680b11d533d4ea914a4a1dd1f80b0cde975bc 100644 --- a/spec/support/sidekiq_middleware.rb +++ b/spec/support/sidekiq_middleware.rb @@ -13,19 +13,6 @@ def with_sidekiq_server_middleware(&block) end # rubocop:enable RSpec/ModifySidekiqMiddleware -# If Sidekiq::Testing.inline! is used, SQL transactions done inside -# Sidekiq worker are included in the SQL query limit (in a real -# deployment sidekiq worker is executed separately). To avoid increasing -# SQL limit counter, query limiting is disabled during Sidekiq block -class DisableQueryLimit - def call(worker_instance, msg, queue) - ::Gitlab::QueryLimiting.disable!('https://mock-issue') - yield - ensure - ::Gitlab::QueryLimiting.enable! - end -end - # When running `Sidekiq::Testing.inline!` each job is using a request-store. # This middleware makes sure the values don't leak into eachother. class IsolatedRequestStore