Race leaves email stuck in "Being Imported" (no sender/recipients, never retried)

[berlinerkind]

Race between parallel account fetch jobs leaves email permanently stuck in "Being Imported" (no sender/recipients, never retried)

Versions: EspoCRM 10.0.3 (latest), PHP 8.4.23, MariaDB 12.3.2, official Docker image.

Steps to reproduce

1. Create two users, each with an active personal Email Account (Personal Email Accounts) fetching from mailboxes on the same mail server.
2. Keep the scheduled job "Check Personal Email Accounts" at its default ~1-minute frequency; ensure both accounts' fetch jobs can run concurrently (default job pool).
3. Send one external email addressed to both users (user A in To, user B in Cc).
4. Wait for both fetch jobs to import the message in overlapping runs. It is a timing race — with a shared busy mailbox it occurs regularly (observed twice in 4 days in production); sending several multi-recipient messages at once raises the odds.
5. Check data/logs/espo-*.log for Import message error, and run:
   SELECT id, status FROM email WHERE deleted = 0 AND status = 'Being Imported';

Expected behavior

Both imports succeed (Message-ID dedup relates the second import to the existing email record), or the failed import is retried — the DB error explicitly says "try restarting transaction". At minimum, fetch_data.lastUID should not advance past a message whose import failed.

Actual behavior

• The losing import aborts mid-save:
  ERROR: (HY000) Import message error. EmailAccount <id>.
  PDOException: SQLSTATE[HY000]: General error: 1020 Record has changed since last read in table 'email_user'; try restarting transaction
• The exception is only logged; the UID is message is never retried.
• The email record is permanently half-written: status='Being Imported', from_email_address_id=NULL (while from_string is set), zero rows in email_emaentity_team links soft-deleted, no email_user row for the crashed account's user (that user cannot see the email).
• UI: detail view shows empty From/To/Cc. Re from the DB afterwards; only manual SQLrepair plus re-reading the IMAP headers restores the record.

Logs: Espo log lines above (data/logs); no web-server or browser-console errors; the failure is entirely in the scheduled job.

[yuri]

We've been fetching emails from multiple accounts in parallel for years, I don't recall any such incident.

Any help with the fix appreciated.

[yuri]

> At minimum, fetch_data.lastUID should not advance past a message whose import failed.

I don't think we should focus on it, but rather try to solve the race condition problem if possible. It's a moot point whether to advance or not. If retry, then may be deeper, but not at the fetcher level.

[yuri]

Related:

https://mariadb.com/docs/server/server-management/install-and-upgrade-mariadb/upgrading/mariadb-community-server-upgrade-paths/upgrading-from-mariadb-11-4-to-mariadb-11-8#changes-in-transaction-behavior

https://www.percona.com/blog/mariadbs-snapshot-isolation-a-fix-that-breaks-more-than-it-fixes/

[yuri]

In the Importer, we use SELECT FOR UPDATE to lock the email record before calling save, which updates email_user and other relationship tables. A parallel import of the same email should have waited before proceeding to save.

It could be that some other transaction, unrelated to email import, updates the email_user (automation?). In earlier MariaDB versions it was not a problem as 'innodb_snapshot_isolation' was disabled by default.

[yuri]

Possible fix: https://github.com/espocrm/espocrm/c...48b7399adef770

Could you apply this fix manually and let know if it works?

[berlinerkind]

Tried the fix on our instance (10.0.3, MariaDB 12.3.2). Applies cleanly.

Can confirm the MariaDB angle: innodb_snapshot_isolation is ON here (default since 11.6.2), which explains why nobody saw this for years. The 1020 simply didn't exist before, the losing statement just proceeded on the newer row version.

I couldn't make the retry actually fire though. I hammered it with two workers importing the same Message-ID in lock-step (a few hundred rounds, even with an artificial 200 ms sleep inside the final-save transaction) and never produced a single 1020. Reading the flow made clear why, and it also answers your "some other transaction, automation?" question from #5: the parallel import of the same email never reaches processFinalTransactionalSave at all. It returns through processDuplicate(), and those email_user writes: updateColumnsById() and the linkMultipleSaver calls, run autocommit, outside any transaction and without the email lock. So the SELECT FOR UPDATE never serializes the two imports against each other; the "other transaction" is the importer's own duplicate path.

That makes the crash mechanism: the creator is inside saveEntity in the final-save transaction, its read view gets created by the first non-locking read there, and a duplicate-path commit from the parallel worker lands between that snapshot read and the locking write on the same email_user rows → 1020, creator dies, its row is the stuck one. That window is microseconds wide, which fits both my failure to hit it synthetically and it only firing about twice in four days here despite constant parallel fetching. Your retry wraps exactly that spot, so for the stuck-row symptom the fix looks right. I just can't prove it outside production.

One gap it might leave: the same collision in the other direction. If the duplicate-path UPDATE blocks on the creator's open final-save transaction and gets aborted with 1020 when the creator commits (snapshot isolation kills waiters?), there's no retry on that path that account's user silently never gets the placement, lastUID advances, and there's no stuck row to notice. I haven't observed it, but under snapshot isolation it should be possible. Might be worth extending the retry to cover processDuplicate's link writes too.

Two small things in the commit itself:
- The loop doesn't return after a successful save, so every import now runs the transactional save twice
- If both attempts hit 1020, the loop just falls through. The error is swallowed and lastUID advances anyway. Probably want a rethrow (or at least a log line) after the retries are exhausted

No regressions otherwise in the stress runs (dedup, recipients, placements all correct). We reverted for now. Happy to re-test a follow-up.

[yuri]

Correct. I fixed it by adding a break.

Though I refactored the code since, to unit tests simpler, there's no one commit to apply the change.

Two files:

- https://github.com/espocrm/espocrm/b...ltImporter.php
- https://github.com/espocrm/espocrm/b...EmailSaver.php

[berlinerkind]

The break plus the rethrow covers both points, and EmailSaver is a nicer home for it anyway.

One thing I'm less sure about: processDuplicate's email_user writes (updateColumnsById and the linkMultipleSaver calls) still run outside the retry. If I read it right, one of those could also die with a 1020 after waiting on a creator's open transaction — unretried and silent, the user just never gets the placement. I never managed to trigger it, so this may well be a non-issue; just flagging it so you can decide.

I'll take the refactored version with the next release and report back if stuck rows reappear. Until then we're thinking about innodb_snapshot_isolation=OFF as a stopgap. As far as I understand it that just restores the old behavior the importer ran on for years, but I'm not a DB specialist: any reason not to?

[yuri]

I think processDuplicate update can fail only if there's a deadlock, which is not likely. Otherwise, we should wrap into retries almost everything, I guess.

[berlinerkind]

You're right, I take that back after I tested it. A plain autocommit UPDATE survives the lock wait fine on 12.3.2: only a transaction with an earlier non-locking read gets the 1020, which is exactly what the retry wraps. Looking forward to the release.

[yuri]

I think we'll release it in the next minor version, 10.1. In a few month.

[berlinerkind]

Thank you!

Unrelated: can you point me at the right thread/process for contributions? I have a few customizations running in production here I'd be happy to upstream. Most need cleanup and overall better generalisation, so I'd rather ask what's of interest before polishing anything:

- Dark mode for the email reader and composer
- Recurring meetings, iCal-style (RRULE/EXDATE, RECURRENCE-ID, etc.)
- ICS / iTIP handling, incl. conference-URL extraction into a clickable location
- Country as ISO 2-letter (no free text)
- User availability and quick-booking dashlet (via API)

Which, if any, would you want to see? The first and fourth are small and self-contained, I can open PRs for those directly. The rest isn't in upstream layout yet, so tell me what shape you'd accept and I'll port it next time.

[yuri]

Currently, we are too busy with other stuff, can't review feature PRs. Not sure when I'll get some spare time.

Free form country field is mapped to ISO codes. I don't think we need to do changes in that logic, as we rely on it in some places, and it would be a major breaking change. What we might do in future, is a parameter to enable validating a country value against the registered country list.

Dark mode for emails, maybe needed. Though isn't it already dark for Email composer? I'm not sure if I understood what it is. Converting imported email's HTML into dark mode is what I considered reasonable when was implementing the dark mode, it is what some email clients do. Though it might be not trivial.

Recurring events is what is planned for future.