Skip to content

Fixed deadlock when recording the send of automation emails#29460

Merged
cmraible merged 1 commit into
mainfrom
evanhahn-ny-1481-fix-deadlock-errors-in-automations-logs
Jul 21, 2026
Merged

Fixed deadlock when recording the send of automation emails#29460
cmraible merged 1 commit into
mainfrom
evanhahn-ny-1481-fix-deadlock-errors-in-automations-logs

Conversation

@EvanHahn

@EvanHahn EvanHahn commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

closes https://linear.app/ghost/issue/NY-1481
ref e9ef527

What

When we record the send of an automation email, we do two things:

  1. Insert an automated_email_recipients row
  2. Increment automation_action_revisions.email_sent_count

This patch switches the order.

Why

To fix a race condition.

We recently saw an error like this (reformatted slightly):

Failed to record automated email recipient for step abc123:

update `automation_action_revisions`
set `email_sent_count` = COALESCE(`email_sent_count`, 0) + 1
where `id` = 'abc123'

Deadlock found when trying to get lock; try restarting transaction

Why did this happen?

Before I explain why this happened, you need to a couple of things about shared and exclusive locks in MySQL:

  • Shared locks lock rows for reading.
  • Exclusive locks lock rows for updates.
  • If you try to get an exclusive lock on a row, you wait for existing locks to be released first.
  • You can get stuck waiting for a lock, which causes deadlock errors like this.

This bug can happen when tracking the send for the same revision simultaneously. Here's a scenario:

  1. Transaction A is opened. It wants to insert Recipient X and increment Revision R.
  2. Transaction B is opened. It wants to insert Recipient Y and increment Revision R.
  3. Transaction A goes to insert Recipient X. Because its automation_action_revisions row is a foreign key reference, MySQL acquires a shared lock on Revision R to prevent it from being deleted.
  4. Transaction B goes to insert Recipient Y. Same thing: a shared lock is acquired on Revision R.
  5. Transaction A goes to increment Revision R's email_sent_count. Before it can do that, it requests an exclusive lock, waiting on Transaction B's shared lock.
  6. Transaction B does the same. It can't get its lock because it's waiting on the shared lock from Transaction A.

A deadlock occurs!

The fix is straightforward: switch the order. Now the following will happen:

  1. Transaction A is opened. It wants to insert Recipient X and increment Revision R.
  2. Transaction B is opened. It wants to insert Recipient Y and increment Revision R.
  3. Transaction A acquires an exclusive lock on Revision R.
  4. Transaction B requests an exclusive lock on Revision R, but is blocked.
  5. Transaction A finishes.
  6. Transaction B is unblocked and finishes.

Thoughts

I admit this solution feels brittle, but it's hard to test without significantly muddying the waters. If you have suggestions for good ways to test this, let me know. But I might wanna do that in a follow-up.

closes https://linear.app/ghost/issue/NY-1481
ref e9ef527

What
----

When we record the send of an automation email, we do two things:

1. Insert an `automated_email_recipients` row
2. Increment `automation_action_revisions.email_sent_count`

This patch switches the order.

Why
---

To fix a race condition.

We recently saw an error like this (reformatted slightly):

```
Failed to record automated email recipient for step abc123:

update `automation_action_revisions`
set `email_sent_count` = COALESCE(`email_sent_count`, 0) + 1
where `id` = 'abc123'

Deadlock found when trying to get lock; try restarting transaction
```

Why did this happen?

Before I explain why this happened, you need to a couple of things about
[shared and exclusive locks in MySQL][0]:

- **Shared locks** lock rows for reading.
- **Exclusive locks** lock rows for updates.
- If you try to get an exclusive lock on a row, you wait for existing
  locks to be released first.
- You can get stuck waiting for a lock, which causes deadlock errors
  like this.

This bug can happen when tracking the send for the same revision
simultaneously. Here's a scenario:

1. Transaction A is opened. It wants to insert Recipient X and increment
   Revision R.
2. Transaction B is opened. It wants to insert Recipient Y and increment
   Revision R.
3. Transaction A goes to insert Recipient X. *Because its
   `automation_action_revisions` row is a foreign key reference*, MySQL
   acquires a *shared lock* on Revision R to prevent it from being
   deleted.
4. Transaction B goes to insert Recipient Y. Same thing: a *shared
   lock* is acquired on Revision R.
5. Transaction A goes to increment Revision R's `email_sent_count`.
   Before it can do that, it requests an *exclusive lock*, waiting on
   Transaction B's shared lock.
6. Transaction B does the same. It can't get its lock because it's
   waiting on the shared lock from Transaction A.

A deadlock occurs!

The fix is straightforward: switch the order. Now the following will
happen:

1. Transaction A is opened. It wants to insert Recipient X and increment
   Revision R.
2. Transaction B is opened. It wants to insert Recipient Y and increment
   Revision R.
3. Transaction A acquires an exclusive lock on Revision R.
4. Transaction B requests an exclusive lock on Revision R, but is
   blocked.
5. Transaction A finishes.
6. Transaction B is unblocked and finishes.

[0]: https://dev.mysql.com/doc/refman/8.0/en/innodb-locking.html#innodb-shared-exclusive-locks
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

recordEmailSent now increments automation_action_revisions.email_sent_count before inserting the corresponding automated_email_recipients row. The previous counter-increment block after the recipient insertion was removed.

Possibly related PRs

  • TryGhost/Ghost#29399: Modifies the same recordEmailSent transaction logic and sent-count update ordering.

Suggested reviewers: troyciesco

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: fixing a deadlock while recording automation email sends.
Description check ✅ Passed The description accurately explains the database operation reorder and its purpose, matching the pull request changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch evanhahn-ny-1481-fix-deadlock-errors-in-automations-logs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 131d9df

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 40s View ↗
nx run ghost:test:integration ✅ Succeeded 3m 14s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 56s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 43s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 20s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 30s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 21s View ↗
nx run @tryghost/admin:build ✅ Succeeded 4s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-20 19:43:50 UTC

@EvanHahn
EvanHahn requested a review from cmraible July 20, 2026 19:34
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.48%. Comparing base (9fd4070) to head (131d9df).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29460      +/-   ##
==========================================
- Coverage   74.53%   74.48%   -0.05%     
==========================================
  Files        1602     1602              
  Lines      140399   140406       +7     
  Branches    17060    17056       -4     
==========================================
- Hits       104646   104586      -60     
- Misses      34733    34766      +33     
- Partials     1020     1054      +34     
Flag Coverage Δ
e2e-tests 76.56% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cmraible cmraible left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shared locks lock rows for reading.
Exclusive locks lock rows for updates.
If you try to get an exclusive lock on a row, you wait for existing locks to be released first.
You can get stuck waiting for a lock, which causes deadlock errors like this.

TIL, thanks for sharing!

Fix makes sense to me. I agree that adding a test would be nice, but also quite tricky since it's timing / concurrency dependent. LGTM!

@cmraible
cmraible merged commit 8599b35 into main Jul 21, 2026
50 checks passed
@cmraible
cmraible deleted the evanhahn-ny-1481-fix-deadlock-errors-in-automations-logs branch July 21, 2026 15:37
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.

2 participants