Commit e3abc1c7 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Mark ProcessCommitWorker as idempotent

The ProcessCommitWorker processed 1 million "duplicate"
jobs the past 7 days, and spent 120 hours on that.

Duplicate jobs are jobs that get scheduled when there
is already a job in the queue for the same worker with
the same arguments.

If the job was (marked as) idempotent,
we would be able to deduplicate those jobs automatically
when they get scheduled.
parent 04fe25d6
...@@ -1234,7 +1234,7 @@ ...@@ -1234,7 +1234,7 @@
:urgency: :high :urgency: :high
:resource_boundary: :unknown :resource_boundary: :unknown
:weight: 3 :weight: 3
:idempotent: :idempotent: true
- :name: project_cache - :name: project_cache
:feature_category: :source_code_management :feature_category: :source_code_management
:has_external_dependencies: :has_external_dependencies:
......
...@@ -7,13 +7,15 @@ ...@@ -7,13 +7,15 @@
# result of this the workload of this worker should be kept to a bare minimum. # result of this the workload of this worker should be kept to a bare minimum.
# Consider using an extra worker if you need to add any extra (and potentially # Consider using an extra worker if you need to add any extra (and potentially
# slow) processing of commits. # slow) processing of commits.
class ProcessCommitWorker # rubocop:disable Scalability/IdempotentWorker class ProcessCommitWorker
include ApplicationWorker include ApplicationWorker
feature_category :source_code_management feature_category :source_code_management
urgency :high urgency :high
weight 3 weight 3
idempotent!
# project_id - The ID of the project this commit belongs to. # project_id - The ID of the project this commit belongs to.
# user_id - The ID of the user that pushed the commit. # user_id - The ID of the user that pushed the commit.
# commit_hash - Hash containing commit details to use for constructing a # commit_hash - Hash containing commit details to use for constructing a
......
...@@ -291,7 +291,7 @@ describe Git::BranchPushService, services: true do ...@@ -291,7 +291,7 @@ describe Git::BranchPushService, services: true do
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
end end
it "defaults to the pushing user if the commit's author is not known", :sidekiq_might_not_need_inline do it "defaults to the pushing user if the commit's author is not known", :sidekiq_inline, :use_clean_rails_redis_caching do
allow(commit).to receive_messages( allow(commit).to receive_messages(
author_name: 'unknown name', author_name: 'unknown name',
author_email: 'unknown@email.com' author_email: 'unknown@email.com'
...@@ -336,7 +336,7 @@ describe Git::BranchPushService, services: true do ...@@ -336,7 +336,7 @@ describe Git::BranchPushService, services: true do
end end
context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do
it 'sets the metric for referenced issues', :sidekiq_might_not_need_inline do it 'sets the metric for referenced issues', :sidekiq_inline, :use_clean_rails_redis_caching do
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_like_time(commit_time) expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_like_time(commit_time)
...@@ -397,7 +397,7 @@ describe Git::BranchPushService, services: true do ...@@ -397,7 +397,7 @@ describe Git::BranchPushService, services: true do
allow(project).to receive(:default_branch).and_return('not-master') allow(project).to receive(:default_branch).and_return('not-master')
end end
it "creates cross-reference notes", :sidekiq_might_not_need_inline do it "creates cross-reference notes", :sidekiq_inline, :use_clean_rails_redis_caching do
expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author) expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author)
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
end end
...@@ -438,7 +438,7 @@ describe Git::BranchPushService, services: true do ...@@ -438,7 +438,7 @@ describe Git::BranchPushService, services: true do
context "mentioning an issue" do context "mentioning an issue" do
let(:message) { "this is some work.\n\nrelated to JIRA-1" } let(:message) { "this is some work.\n\nrelated to JIRA-1" }
it "initiates one api call to jira server to mention the issue", :sidekiq_might_not_need_inline do it "initiates one api call to jira server to mention the issue", :sidekiq_inline, :use_clean_rails_redis_caching do
execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with(
......
...@@ -22,16 +22,26 @@ describe ProcessCommitWorker do ...@@ -22,16 +22,26 @@ describe ProcessCommitWorker do
worker.perform(project.id, -1, commit.to_hash) worker.perform(project.id, -1, commit.to_hash)
end end
it 'processes the commit message' do include_examples 'an idempotent worker' do
expect(worker).to receive(:process_commit_message).and_call_original subject do
perform_multiple([project.id, user.id, commit.to_hash], worker: worker)
end
worker.perform(project.id, user.id, commit.to_hash) it 'processes the commit message' do
end expect(worker).to receive(:process_commit_message)
.exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES)
.and_call_original
it 'updates the issue metrics' do subject
expect(worker).to receive(:update_issue_metrics).and_call_original end
worker.perform(project.id, user.id, commit.to_hash) it 'updates the issue metrics' do
expect(worker).to receive(:update_issue_metrics)
.exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES)
.and_call_original
subject
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment