Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make operation to deduplicate messages atomic #14

Merged
merged 3 commits into from
Mar 23, 2017

Conversation

k0kubun
Copy link
Collaborator

@k0kubun k0kubun commented Mar 18, 2017

Background

In Barbeque::MessageHandler::JobExecution, we execute SELECT and INSERT queries for job_execution without a transaction. We didn't introduce a transaction to avoid performance regression. So it is possible that a single job is executed twice.

But I noticed that we can create a record and detect duplication in a single INSERT query because we have message_id unique index.

Changes

So I changed Barbeque::MessageHandler::JobExecution to use such a query. I also applied it to Barbeque::MessageHandler::JobExecution.

I believe this PR resolves message duplication by SQS's at-least-once delivery.

@sorah
Copy link
Member

sorah commented Mar 23, 2017

Looking for voice from other people 🔈

job_retry = Barbeque::JobRetry.find_or_initialize_by(message_id: @message.id)
job_retry.update!(job_execution: job_execution)
begin
job_retry = Barbeque::JobRetry.create(message_id: @message.id, job_execution: job_execution)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but can we retry the job more than twice?

Copy link
Collaborator Author

@k0kubun k0kubun Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Retry" in barbeque means re-enqueuing a message having type: "JobRetry" to SQS. Thus it can be possible.

If we don't dedup this, there may be the situation that barbeque-console says it is retried once but actually it was retried twice. In that situation, one of their logs will be lost.

Copy link
Collaborator Author

@k0kubun k0kubun Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, only when deduping both JobExecution and JobRetry, we can assume that the same message isn't consumed by multiple workers. People who write code of a job won't be necessary to care about exclusive control.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Retry" in barbeque means re-enqueuing a message having type: "JobRetry" to SQS.

Aha, now I can know how this change will work. Looks good.

@k0kubun k0kubun merged commit d0d3472 into cookpad:master Mar 23, 2017
@k0kubun k0kubun deleted the exactly-once-delivery branch March 23, 2017 13:28
@taiki45
Copy link

taiki45 commented Mar 24, 2017

@k0kubun Can you release this? If not, I'll take.

@k0kubun
Copy link
Collaborator Author

k0kubun commented Mar 24, 2017

Done. Released as v0.2.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants