Skip to content

translate_and_adapt: no 500 when two requests create the same article - #909

Merged
mircealungu merged 2 commits into
masterfrom
fix/translate-and-adapt-duplicate-article
Oct 25, 2026
Merged

mircealungu merged 2 commits into
masterfrom
fix/translate-and-adapt-duplicate-article

Conversation

@mircealungu

@mircealungu mircealungu commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

1. What happened

FMD alert, 22 Oct 11:24:04:

IntegrityError: (1062, "Duplicate entry '6974326' for key 'url_id'")
[SQL: INSERT INTO article (title, ..., url_id, ...)] [parameters: ('Bau von Trumps 76-Meter-Triumphbogen in ...', ...)]
  zeeguu/api/endpoints/article_upload.py, line 281, in article_upload_translate_and_adapt

A user asked Zeeguu to translate and adapt an uploaded article ("Bau von Trumps 76-Meter-Triumphbogen ..."). Two requests for the same upload arrived close together, most likely a double tap or a retry while the first one was still waiting for the LLM. Both requests called the LLM. The first one saved the translated article. The second one tried to save a second article at the same URL, the database refused it (article.url_id is unique), and the user got a 500.

2. Why it happened

POST /article_upload/<id>/translate_and_adapt, zeeguu/api/endpoints/article_upload.py on master:

  1. Build the key (line 210). The translated article gets its own URL, made from the upload's URL:
    translated_url_key = (
        f"{upload.url.as_string()}"
        f"#translated-from-{source_language}-to-{target_language}-{user_level}"
    )
  2. Look for an existing translation (line 215). If one exists, return it:
    existing_article = Article.find(translated_url_key)
    if existing_article:
        ...
        return json_result(uai)
  3. Call the LLM (line 234), which takes several seconds:
    result = service.translate_and_adapt(...)
  4. Insert the article (lines 255–282): Url.find_or_create, Source.find_or_create, build Article(...), then
    db_session.add(translated_article)
    db_session.commit()        # <- the 1062 comes from here
  5. Create its fragments (lines 284–285), commit, and make the personal copy (line 287).

The check in step 2 and the insert in step 4 are seconds apart because of the LLM call. Request A and request B both do step 2 before either of them reaches step 4, so both see "no article yet". A inserts and commits. B then inserts a second article with the same url_id, and the unique key rejects it.

3. The fix

3a. The insert goes through race_safe (endpoint)

Step 4 now uses the shared race-safe helper from #897 instead of add + commit:

translated_article, created = find_or_create_and_report(
    db_session, Article, dict(url=translated_url), make_translated_article
)
  • The helper first SELECTs the article at translated_url. If it is already there, it returns it.
  • Otherwise it inserts the article built by make_translated_article() inside a savepoint.
  • If the insert fails with IntegrityError (the other request won), it rolls back only that savepoint and fetches the winner's article with a locking read (LOCK IN SHARE MODE). A plain re-read would not see it under REPEATABLE READ (see Share one race-safe find_or_create helper (locking re-read) across 15 models #897).

So the loser now gets the winner's article and continues as if it had found it in step 2: personal copy, user_article_info, 200.

The LLM call stays where it was, before the insert. Both requests still pay for the LLM (see follow-ups); the goal here is only to get rid of the 500.

make_translated_article() contains exactly what the old code did to build the article (cefr_level, source_upload_id, img_url). The one thing that moved is Url.find_or_create(db_session, upload.image_url): it now runs before the helper rather than inside the builder, because Url.find_or_create commits, and a commit must not happen inside the helper's savepoint.

3b. Fragments: only for an article this request created

If the loser also ran create_article_fragments, it would add a second set of fragments to the winner's article, made from its own (different) LLM text. So fragments must be created only when this request inserted the article.

The helper did not say whether it created the row. I added a small function next to it, find_or_create_and_report, which returns (row, created). find_or_create is now a two-line wrapper around it and still returns just the row:

def find_or_create(session, cls, filters: dict, make, commit=False):
    row, _ = find_or_create_and_report(session, cls, filters, make, commit)
    return row

The endpoint then does:

if created:
    translated_article.create_article_fragments(db_session)
db_session.commit()

Why this and not a check in the endpoint such as "does the article already have fragments?": that check would itself be racy. On master the winner commits the article first and only then creates the fragments and commits again. A loser arriving in between would see an article with no fragments yet and create its own set. Only the insert itself knows who won.

3c. Article and fragments in one commit

On master the article and its fragments were two commits (lines 282 and 285). Now they are one, after the if created: block. There are two reasons:

  1. Whoever sees the article also sees its fragments. Before, anyone reading in the gap between the two commits (the loser, or a third request taking the step-2 path) got an article with no fragments.
  2. The loser's commit ends its stale snapshot. The loser's last read before the collision took its REPEATABLE READ snapshot, and that snapshot is older than the winner's commit. The locking read sees the winner's article row. A plain query for its fragments would still read the old snapshot and find none, and user_article_info(..., with_content=True) would then tokenize zero fragments and write that into the tokenization cache. The commit starts a fresh snapshot, so the loser sees the fragments. On local MySQL (below): before its commit the loser saw 0 fragments, after the commit it saw 2.

A side effect: if create_article_fragments raises, the article is no longer left committed without fragments. Nothing is saved.

4. Why this is safe

  • The 16 existing callers of race_safe.find_or_create are unchanged. find_or_create has the same signature and return value. Its body is the same code, now living in find_or_create_and_report: the only differences are the , False / , True added to the three returns, and the wrapper drops that flag. commit=True still commits only a row it created. The 5 existing helper tests pass unchanged.
  • The endpoint, normal case (no race): same rows as before: one article with the same fields, fragments, personal copy. The only differences are the single commit and that img_url is looked up a few lines earlier.
  • Url.find_or_create and Source.find_or_create in this path. I checked whether they can fail in the same race:
    • Url.find_or_create(db_session, translated_url_key) can collide on its unique (path, domain) key when both requests create the URL. It catches the IntegrityError, does session.rollback() and reads the row again. The rollback also starts a fresh snapshot, so the re-read finds the winner's URL. This endpoint has nothing pending at that point, so the full rollback loses nothing. The alert confirms this: the loser got past it and failed on the article insert, with the shared url_id 6974326. Not broken here, so I left it alone.
    • Source.find_or_create(db_session, result["content"], ...): the two LLM outputs differ, so each request creates its own source; the source table has no unique key that they would share. Not broken here.
    • Url.find_or_create(db_session, upload.image_url): both requests find the same existing URL, and it collides the same way as above if not.
  • Other unique keys on article. The model also declares img_url_id and source_id as unique=True. If prod has those keys and an insert collides on one of them (rather than on url_id), this PR does not recover it: no article matches the url, so there is no winner to return. That was a 500 on master too. With this PR it surfaces as the original IntegrityError naming the key (see 6b), not a misleading NoResultFound. Whether prod has these keys at all is unknown; check with:
    SHOW INDEX FROM article WHERE Column_name IN ('img_url_id','source_id');

5. How it was tested

  • New endpoint test, zeeguu/api/test/test_article_upload_translate_race.py. The LLM is mocked and returns a different text on each call, as the real one does. The first request creates the article. The second request is made to miss both lookups (step 2's Article.find, and the helper's first SELECT), as on MySQL where it reads an older snapshot. So it really calls the LLM and really inserts at the same URL. It checks that:

    • the second request returns 200 with the winner's article (same id, the winner's title);
    • there is still one article for the upload;
    • the winner's fragment count is unchanged.

    On master this test fails with UNIQUE constraint failed: article.url_id, the same error as the alert.

  • Two new helper tests in zeeguu/core/test/test_race_safe.py: created is True when the call inserts, False when it finds the row, and False when it loses the race.

  • Local MySQL (zeeguu_test, REPEATABLE-READ), a scratch script with two sessions. Request A takes its snapshot; request B creates the article plus fragments at the translated URL and commits; then A runs the insert step. All rows created (url, domain, sources, texts, articles, fragments) were deleted afterwards.

    • master code: IntegrityError (1062, "Duplicate entry '6703913' for key 'article.url_id'")
    • this PR: helper returned article 4828048 created: False, fragments A sees: before its commit 0, after 2, articles at url: 1
  • Full suite: 1343 passed, 18 skipped (20 subtests passed).

6. Review fixes (second commit)

6a. Translated URL too long → None (was a regression in the first commit)

  1. Url.find_or_create returns None when the path is over 255 chars (url.py:99-101).
  2. The #translated-from-<src>-to-<target>-<level> suffix adds about 28 chars, so an upload whose path is 228–255 chars gets translated_url = None.
  3. Master then inserted Article(None, ...), and that worked. The first commit of this PR instead called the helper with dict(url=None). That runs url_id IS NULL ... one_or_none(), which either returns an unrelated article with no url (created=False: no fragments, and the user gets a personal copy of someone else's article) or raises MultipleResultsFound (500).
  4. Fix: when translated_url is None, skip the helper and insert directly, with created = True, as master did. Such articles get no race protection, the same as on master.
  5. Test: test_an_upload_whose_translated_url_is_too_long_gets_its_own_article translates two different uploads with 250-char paths. Each must get its own article, with fragments. Without the fix, the second request got the first one's article.

6b. race_safe: a collision on another unique key re-raises the original error

  1. The helper catches an IntegrityError from any unique key, and then looks the winner up by filters only.
  2. If the collision was on a key filters doesn't cover, no row matches, and .one() raised NoResultFound, which hid the real constraint.
  3. Fix: the locking read now uses .one_or_none(). If it finds nothing, the helper re-raises the original IntegrityError. The savepoint has already undone only this insert, so the caller's session is as it was before the call.
  4. When the row is found (every case the 16 existing callers rely on), nothing changes: same query, same row returned. MultipleResultsFound behaves as before.
  5. Test: test_collision_on_another_unique_key_raises_the_integrity_error collides on (bookmark, article) while filtering by an id that matches nothing. It expects an IntegrityError and checks that the caller's pending change survives.

Tests after the review fixes

  • Helper + endpoint race tests: 10 passed. Both new tests fail on the first commit.
  • Full suite: 1345 passed, 18 skipped (20 subtests passed).

How to verify in prod

All read-only.

How often it happened. API logs (journalctl keeps about 9 days):

ssh zeeguu@$ZORGIP "sudo journalctl -t zeeguu-api --since='10 days ago' | grep 'translate_and_adapt' | grep -c '\" 500 '"
ssh zeeguu@$ZORGIP "sudo journalctl -t zeeguu-api --since='10 days ago' | grep -c \"Duplicate entry .* for key 'url_id'\""

The second command counts every url_id duplicate, not only this endpoint's, so read it next to the first. In FMD, endpoint endpoints.article_upload_translate_and_adapt shows its 500s for a longer period.

The article from the alert (it was created by the winner, so it should be there once):

SELECT a.id, a.title, a.source_upload_id, a.cefr_level,
       (SELECT COUNT(*) FROM article_fragment f WHERE f.article_id = a.id) AS fragments
FROM article a
WHERE a.url_id = 6974326;

No translated article with a doubled set of fragments (there should be none from this path before or after; a doubled set would show up as two fragments with the same order):

SELECT f.article_id, f.`order`, COUNT(*) AS n
FROM article_fragment f
JOIN article a ON a.id = f.article_id
WHERE a.source_upload_id IS NOT NULL
GROUP BY f.article_id, f.`order`
HAVING n > 1
LIMIT 20;

After deploy: the two journalctl lines above should stop finding new hits for this endpoint, and no new FMD alert should come with Duplicate entry ... for key 'url_id' from article_upload_translate_and_adapt.

Follow-ups (not in this PR)

  • Double LLM spend. Both requests still call the LLM; the loser's output is thrown away. Avoiding that needs a "translation in progress" marker or a lock taken before the LLM call.
  • Orphan source rows. The loser's Source.find_or_create commits a source (+ source_text) for its own LLM text, which no article then uses. That's one orphan row pair per lost race.
  • Duplicate personal copy. For the same user, winner and loser can both pass PersonalCopy.exists_for before either inserts. The model declares no unique key on (user_id, article_id), so that would add a second row rather than a 500. I haven't checked prod for such pairs.
  • The sibling endpoint has the same race. POST /translate_and_adapt_article in zeeguu/api/endpoints/article.py (~lines 614–695) calls translate_and_adapt, then does add + commit (690–691) and a separate fragments commit (694–695). It should get its own PR, ideally sharing one "create translated article" function with this endpoint.
  • Trade-off of the single commit. create_article_fragments now runs in the same transaction as the insert, which reads with the snapshot taken before the LLM call. NewText.find_or_create(commit=False) can therefore miss a paragraph text that another request committed in the meantime. It then inserts a duplicate content_hash, and the whole translation is rolled back (500). Before, the fragments ran after a commit, with a fresher snapshot. This needs two different articles containing an identical new paragraph within seconds of each other, so it is rare.
  • Url.find_or_create still uses the older commit + full rollback() retry loop. It works for this endpoint, but it would throw away pending work in a caller that has some. It could move onto race_safe in a separate PR.

🤖 Generated with Claude Code

Two requests for the same upload (double tap, retry while the first waits
on the LLM) both missed Article.find and both inserted an article at the
same url; the second hit the unique key on article.url_id (1062) and
returned a 500.

The insert now goes through race_safe, so the loser gets the winner's
article back. A new find_or_create_and_report (find_or_create is now a
wrapper around it) also says whether this call created the row, so
fragments are only created by the request that created the article. The
article and its fragments go in one commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

ArchLens detected architectural changes in the following views:
diff

… race_safe re-raises other-key collisions

Url.find_or_create returns None for a path over 255 chars, and the
"#translated-from-..." suffix can push an upload's url over. The helper
would then look up `url_id IS NULL` and return an unrelated article (or
raise MultipleResultsFound). With no url, insert directly as master did.

race_safe: when the insert collides but no row matches `filters`, the
collision was on another unique key. Re-raise that IntegrityError instead
of a NoResultFound that hides it. Unchanged when the row is found.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mircealungu
mircealungu merged commit 6aa7c44 into master Oct 25, 2026
2 checks passed
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.

1 participant