element-hq / element-hq/synapse
Fix unique constraint exceptions on remote media redownload
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
This issue has been migrated from [#12661](https://github.com/matrix-org/synapse/pull/12661).
---
Related: https://github.com/matrix-org/synapse/issues/12660
This fixes a very specific problem, with the following conditions;
- The custom storage provider does not `store_remote`
- The storage provider has not stored the media
- The media was previously downloaded (and the file info registered in the database)
- The local `remote_content` folder was wiped
Synapse then tries to redownload the file, generating a new `file_info`, but the previous file info was already stored in the database under a unique key constraint, creating the following error;
Error
```
2022-05-06 20:10:53,419 - synapse.rest.media.v1.media_repository - 361 - WARNING - GET-1070 - Ignoring _download_remote_file exception
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/synapse/rest/media/v1/media_repository.py", line 348, in _get_remote_media_impl
media_info = await self._download_remote_file(
File "/usr/local/lib/python3.9/site-packages/synapse/rest/media/v1/media_repository.py", line 474, in _download_remote_file
await self.store.store_cached_remote_media(
File "/usr/local/lib/python3.9/site-packages/synapse/storage/databases/main/media_repository.py", line 471, in store_cached_remote_media
await self.db_pool.simple_insert(
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 966, in simple_insert
await self.runInteraction(desc, self.simple_insert_txn, table, values)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 797, in runInteraction
return await delay_cancellation(defer.ensureDeferred(_runInteraction()))
File "/usr/local/lib/python3.9/site-packages/twisted/internet/defer.py", line 1656, in _inlineCallbacks
result = current_context.run(
File "/usr/local/lib/python3.9/site-packages/twisted/python/failure.py", line 489, in throwExceptionIntoGenerator
return g.throw(self.type, self.value, self.tb)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 769, in _runInteraction
result = await self.runWithConnection(
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 892, in runWithConnection
return await make_deferred_yieldable(
File "/usr/local/lib/python3.9/site-packages/twisted/python/threadpool.py", line 244, in inContext
result = inContext.theWork() # type: ignore[attr-defined]
File "/usr/local/lib/python3.9/site-packages/twisted/python/threadpool.py", line 260, in
inContext.theWork = lambda: context.call( # type: ignore[attr-defined]
File "/usr/local/lib/python3.9/site-packages/twisted/python/context.py", line 117, in callWithContext
return self.currentContext().callWithContext(ctx, func, *args, **kw)
File "/usr/local/lib/python3.9/site-packages/twisted/python/context.py", line 82, in callWithContext
return func(*args, **kw)
File "/usr/local/lib/python3.9/site-packages/twisted/enterprise/adbapi.py", line 282, in _runWithConnection
result = func(conn, *args, **kw)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 885, in inner_func
return func(db_conn, *args, **kwargs)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 633, in new_transaction
r = func(cursor, *args, **kwargs)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 980, in simple_insert_txn
txn.execute(sql, vals)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 334, in execute
self._do_execute(self.txn.execute, sql, *args)
File "/usr/local/lib/python3.9/site-packages/synapse/storage/database.py", line 367, in _do_execute
return func(sql, *args)
psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "remote_media_cache_media_origin_media_id_key"
DETAIL: Key (media_origin, media_id)=(neko.dev, wLFwLqbnyvrstuomVXdKMqyJ) already exists.
```
This error gets surpressed due to a logic error described in #12660, where synapse only tests if file_info is available before moving on and ignoring the error, before running into a `fetch_media` call later on, which returns no responder, which then results in a 404.
This PR attempts to fix that issue, and also add a warning log whenever such an error is ignored. (If I recall correctly, this means it'll also be captured by sentry)
### Pull Request Checklist
* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog file](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
- Use markdown where necessary, mostly for `code blocks`.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
* [x] Pull request includes a [sign off](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off)
* [x] [Code style](https://matrix-org.github.io/synapse/latest/code_style.html) is correct
(run the [linters](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
`Signed-off-by: Jonathan de Jong `
Contributor guide
Research direction
Start by reading synapse/rest/media/v1/media_repository.py around _get_remote_media_impl and _download_remote_file, then synapse/storage/databases/main/media_repository.py around store_cached_remote_media. Reproduce the listed custom-provider and wiped-folder conditions, and verify that a remote redownload no longer fails with a unique-constraint error or ends in a 404.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100