diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index 2ead2e2a11323025d4e62cd6d61709d7608ddd2c..1fa8cceaf3ebb4f124e57347ab92b0fcbeccb8f5 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -4,6 +4,9 @@ module Git class BranchHooksService < ::Git::BaseHooksService extend ::Gitlab::Utils::Override + JIRA_SYNC_BATCH_SIZE = 20 + JIRA_SYNC_BATCH_DELAY = 10.seconds + def execute execute_branch_hooks @@ -157,13 +160,34 @@ def enqueue_jira_connect_sync_messages return unless project.jira_subscription_exists? branch_to_sync = branch_name if Atlassian::JiraIssueKeyExtractors::Branch.has_keys?(project, branch_name) - commits_to_sync = limited_commits.select { |commit| Atlassian::JiraIssueKeyExtractor.has_keys?(commit.safe_message) }.map(&:sha) - - if branch_to_sync || commits_to_sync.any? - JiraConnect::SyncBranchWorker.perform_async(project.id, branch_to_sync, commits_to_sync, Atlassian::JiraConnect::Client.generate_update_sequence_id) + commits_to_sync = filtered_commit_shas + + return if branch_to_sync.nil? && commits_to_sync.empty? + + if commits_to_sync.any? && Feature.enabled?(:batch_delay_jira_branch_sync_worker, project) + commits_to_sync.each_slice(JIRA_SYNC_BATCH_SIZE).with_index do |commits, i| + JiraConnect::SyncBranchWorker.perform_in( + JIRA_SYNC_BATCH_DELAY * i, + project.id, + branch_to_sync, + commits, + Atlassian::JiraConnect::Client.generate_update_sequence_id + ) + end + else + JiraConnect::SyncBranchWorker.perform_async( + project.id, + branch_to_sync, + commits_to_sync, + Atlassian::JiraConnect::Client.generate_update_sequence_id + ) end end + def filtered_commit_shas + limited_commits.select { |commit| Atlassian::JiraIssueKeyExtractor.has_keys?(commit.safe_message) }.map(&:sha) + end + def signature_types [ ::CommitSignatures::GpgSignature, diff --git a/config/feature_flags/development/batch_delay_jira_branch_sync_worker.yml b/config/feature_flags/development/batch_delay_jira_branch_sync_worker.yml new file mode 100644 index 0000000000000000000000000000000000000000..f10403e98e9d448d7b127e524a1be2ec05174544 --- /dev/null +++ b/config/feature_flags/development/batch_delay_jira_branch_sync_worker.yml @@ -0,0 +1,8 @@ +--- +name: batch_delay_jira_branch_sync_worker +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120866 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/411865 +milestone: '16.1' +type: development +group: group::source code +default_enabled: false diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb index aa534777f3e7992276a35d9b758d96307dd4251b..5e43426b9dd962669e4a40f71c05e041fff33768 100644 --- a/spec/services/git/branch_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -14,15 +14,17 @@ let(:branch) { 'master' } let(:ref) { "refs/heads/#{branch}" } let(:push_options) { nil } + let(:service) do + described_class + .new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }, push_options: push_options) + end before do project.add_maintainer(user) end subject(:execute_service) do - described_class - .new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }, push_options: push_options) - .execute + service.execute end describe 'Push branches' do @@ -683,14 +685,44 @@ let(:commits_to_sync) { [] } shared_examples 'enqueues Jira sync worker' do - specify :aggregate_failures do - Sidekiq::Testing.fake! do - expect(JiraConnect::SyncBranchWorker) - .to receive(:perform_async) - .with(project.id, branch_to_sync, commits_to_sync, kind_of(Numeric)) - .and_call_original + context "batch_delay_jira_branch_sync_worker feature flag is enabled" do + before do + stub_feature_flags(batch_delay_jira_branch_sync_worker: true) + end + + specify :aggregate_failures do + Sidekiq::Testing.fake! do + if commits_to_sync.any? + expect(JiraConnect::SyncBranchWorker) + .to receive(:perform_in) + .with(kind_of(Numeric), project.id, branch_to_sync, commits_to_sync, kind_of(Numeric)) + .and_call_original + else + expect(JiraConnect::SyncBranchWorker) + .to receive(:perform_async) + .with(project.id, branch_to_sync, commits_to_sync, kind_of(Numeric)) + .and_call_original + end + + expect { subject }.to change(JiraConnect::SyncBranchWorker.jobs, :size).by(1) + end + end + end - expect { subject }.to change(JiraConnect::SyncBranchWorker.jobs, :size).by(1) + context "batch_delay_jira_branch_sync_worker feature flag is disabled" do + before do + stub_feature_flags(batch_delay_jira_branch_sync_worker: false) + end + + specify :aggregate_failures do + Sidekiq::Testing.fake! do + expect(JiraConnect::SyncBranchWorker) + .to receive(:perform_async) + .with(project.id, branch_to_sync, commits_to_sync, kind_of(Numeric)) + .and_call_original + + expect { subject }.to change(JiraConnect::SyncBranchWorker.jobs, :size).by(1) + end end end end @@ -723,6 +755,29 @@ end it_behaves_like 'enqueues Jira sync worker' + + describe 'batch requests' do + let(:commits_to_sync) { [sample_commit.id, another_sample_commit.id] } + + it 'enqueues multiple jobs' do + # We have to stub this as we only have two valid commits to use + stub_const('Git::BranchHooksService::JIRA_SYNC_BATCH_SIZE', 1) + + expect_any_instance_of(Git::BranchHooksService).to receive(:filtered_commit_shas).and_return(commits_to_sync) + + expect(JiraConnect::SyncBranchWorker) + .to receive(:perform_in) + .with(0.seconds, project.id, branch_to_sync, [commits_to_sync.first], kind_of(Numeric)) + .and_call_original + + expect(JiraConnect::SyncBranchWorker) + .to receive(:perform_in) + .with(10.seconds, project.id, branch_to_sync, [commits_to_sync.last], kind_of(Numeric)) + .and_call_original + + subject + end + end end context 'branch name and commit message does not contain Jira issue key' do