Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix could not serialize access due to concurrent DELETE from presence_stream #15826

Merged

Conversation

realtyem
Copy link
Contributor

@realtyem realtyem commented Jun 25, 2023

I considered(and tested) using a Linearizer for this, I liked it's queueing behavior. However on further investigation, I found out that this type of situation has occurred before in Synapse history and was simply changing the isolation level to be slightly less strict than the default of REPEATABLE_READ. Using READ_COMMITTED allows the DELETE transaction to continue, even if it means the DELETE not seeing the concurrent transaction taking place to add a lower Stream ID(that this DELETE would otherwise have removed). Since either function will run again shortly, it will then subsequently remove the lower Stream ID entry and all will be well. The Postgres docs suggest that depending on the order of the transaction, it may still go ahead and DELETE the row anyways instead of ignoring it, which seems to have been the intended behavior.

... DELETE,... commands behave the same as SELECT in terms of searching for target rows: they will only find target rows that were committed as of the command start time. However, such a target row might have already been updated (or deleted or locked) by another concurrent transaction by the time it is found. In this case, the would-be updater will wait for the first updating transaction to commit or roll back (if it is still in progress). If the first updater rolls back, then its effects are negated and the second updater can proceed with updating the originally found row. If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition. If so, the second updater proceeds with its operation using the updated version of the row.

Related docs and PRs/Issues:

Note: This has no effect for SQLite usage, as attempt_to_set_isolation_level()SQLite version says everything is set to SERIALIZABLE regardless.

Pull Request Checklist

Signed-off-by: Jason Little realtyem@gmail.com

EDIT: Added some pretty pictures
Example 1
Example 2

@realtyem realtyem force-pushed the fix-concurrent-presence-table-delete branch from cefd370 to fe28de3 Compare June 25, 2023 08:52
@realtyem realtyem marked this pull request as ready for review June 25, 2023 10:26
@realtyem realtyem requested a review from a team as a code owner June 25, 2023 10:26
@realtyem realtyem marked this pull request as draft July 2, 2023 21:02
@realtyem
Copy link
Contributor Author

realtyem commented Jul 2, 2023

Actually, let's hold off on this for now. I think I may have found the place that was causing this behavior. While I investigate how hard this will be to untangle, I'll leave this in draft.

@realtyem realtyem marked this pull request as ready for review July 4, 2023 09:05
@realtyem
Copy link
Contributor Author

realtyem commented Jul 4, 2023

Let's go ahead with this, the angle I was looking at will not solve this on it's own.

Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

can't see why this would pose any problems given the txn that is being run

@reivilibre reivilibre merged commit 4cf9f92 into matrix-org:develop Jul 5, 2023
yingziwu added a commit to yingziwu/synapse that referenced this pull request Jul 19, 2023
This release
 - raises the minimum supported version of Python to 3.8, as Python 3.7 is now [end-of-life](https://devguide.python.org/versions/), and
 - removes deprecated config options related to worker deployment.

See [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#upgrading-to-v1880) for more information.

- Revert "Stop writing to column `user_id` of tables `profiles` and `user_filters`", which was introduced in Synapse 1.88.0rc1. ([\matrix-org#15953](matrix-org#15953))

- Add `not_user_type` param to the [list accounts admin API](https://matrix-org.github.io/synapse/v1.88/admin_api/user_admin_api.html#list-accounts). ([\matrix-org#15844](matrix-org#15844))

- Pin `pydantic` to `^=1.7.4` to avoid backwards-incompatible API changes from the 2.0.0 release.
  Contributed by @PaarthShah. ([\matrix-org#15862](matrix-org#15862))
- Correctly resize thumbnails with pillow version >=10. ([\matrix-org#15876](matrix-org#15876))

- Fixed header levels on the [Admin API "Users"](https://matrix-org.github.io/synapse/v1.87/admin_api/user_admin_api.html) documentation page. Contributed by @sumnerevans at @beeper. ([\matrix-org#15852](matrix-org#15852))
- Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. ([\matrix-org#15872](matrix-org#15872))

- **Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options.** See the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#removal-of-worker_replication_-settings) for more details. ([\matrix-org#15860](matrix-org#15860))
- Remove support for Python 3.7 and hence for Debian Buster. ([\matrix-org#15851](matrix-org#15851), [\matrix-org#15892](matrix-org#15892), [\matrix-org#15893](matrix-org#15893), [\matrix-org#15917](matrix-org#15917))

- Add foreign key constraint to `event_forward_extremities`. ([\matrix-org#15751](matrix-org#15751), [\matrix-org#15907](matrix-org#15907))
- Add read/write style cross-worker locks. ([\matrix-org#15782](matrix-org#15782))
- Stop writing to column `user_id` of tables `profiles` and `user_filters`. ([\matrix-org#15787](matrix-org#15787))
- Use lower isolation level when cleaning old presence stream data to avoid serialization errors. ([\matrix-org#15826](matrix-org#15826))
- Add tracing to media `/upload` code paths. ([\matrix-org#15850](matrix-org#15850), [\matrix-org#15888](matrix-org#15888))
- Add a timeout that aborts any Postgres statement taking more than 1 hour. ([\matrix-org#15853](matrix-org#15853))
- Fix the `devenv up` configuration which was ignoring the config overrides. ([\matrix-org#15854](matrix-org#15854))
- Optimised cleanup of old entries in `device_lists_stream`. ([\matrix-org#15861](matrix-org#15861))
- Update the Matrix clients link in the _It works! Synapse is running_ landing page. ([\matrix-org#15874](matrix-org#15874))
- Fix building Synapse with the nightly Rust compiler. ([\matrix-org#15906](matrix-org#15906))
- Add `Server` to Access-Control-Expose-Headers header. ([\matrix-org#15908](matrix-org#15908))

* Bump authlib from 1.2.0 to 1.2.1. ([\matrix-org#15864](matrix-org#15864))
* Bump importlib-metadata from 6.6.0 to 6.7.0. ([\matrix-org#15865](matrix-org#15865))
* Bump lxml from 4.9.2 to 4.9.3. ([\matrix-org#15897](matrix-org#15897))
* Bump regex from 1.8.4 to 1.9.1. ([\matrix-org#15902](matrix-org#15902))
* Bump ruff from 0.0.275 to 0.0.277. ([\matrix-org#15900](matrix-org#15900))
* Bump sentry-sdk from 1.25.1 to 1.26.0. ([\matrix-org#15867](matrix-org#15867))
* Bump serde_json from 1.0.99 to 1.0.100. ([\matrix-org#15901](matrix-org#15901))
* Bump types-pyopenssl from 23.2.0.0 to 23.2.0.1. ([\matrix-org#15866](matrix-org#15866))
Fizzadar added a commit to beeper/synapse-legacy-fork that referenced this pull request Jul 27, 2023
This release
 - raises the minimum supported version of Python to 3.8, as Python 3.7 is now [end-of-life](https://devguide.python.org/versions/), and
 - removes deprecated config options related to worker deployment.

See [the upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#upgrading-to-v1880) for more information.

- Revert "Stop writing to column `user_id` of tables `profiles` and `user_filters`", which was introduced in Synapse 1.88.0rc1. ([\matrix-org#15953](matrix-org#15953))

- Add `not_user_type` param to the [list accounts admin API](https://matrix-org.github.io/synapse/v1.88/admin_api/user_admin_api.html#list-accounts). ([\matrix-org#15844](matrix-org#15844))

- Pin `pydantic` to `^=1.7.4` to avoid backwards-incompatible API changes from the 2.0.0 release.
  Contributed by @PaarthShah. ([\matrix-org#15862](matrix-org#15862))
- Correctly resize thumbnails with pillow version >=10. ([\matrix-org#15876](matrix-org#15876))

- Fixed header levels on the [Admin API "Users"](https://matrix-org.github.io/synapse/v1.87/admin_api/user_admin_api.html) documentation page. Contributed by @sumnerevans at @beeper. ([\matrix-org#15852](matrix-org#15852))
- Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options. ([\matrix-org#15872](matrix-org#15872))

- **Remove deprecated `worker_replication_host`, `worker_replication_http_port` and `worker_replication_http_tls` configuration options.** See the [upgrade notes](https://github.com/matrix-org/synapse/blob/release-v1.88/docs/upgrade.md#removal-of-worker_replication_-settings) for more details. ([\matrix-org#15860](matrix-org#15860))
- Remove support for Python 3.7 and hence for Debian Buster. ([\matrix-org#15851](matrix-org#15851), [\matrix-org#15892](matrix-org#15892), [\matrix-org#15893](matrix-org#15893), [\matrix-org#15917](matrix-org#15917))

- Add foreign key constraint to `event_forward_extremities`. ([\matrix-org#15751](matrix-org#15751), [\matrix-org#15907](matrix-org#15907))
- Add read/write style cross-worker locks. ([\matrix-org#15782](matrix-org#15782))
- Stop writing to column `user_id` of tables `profiles` and `user_filters`. ([\matrix-org#15787](matrix-org#15787))
- Use lower isolation level when cleaning old presence stream data to avoid serialization errors. ([\matrix-org#15826](matrix-org#15826))
- Add tracing to media `/upload` code paths. ([\matrix-org#15850](matrix-org#15850), [\matrix-org#15888](matrix-org#15888))
- Add a timeout that aborts any Postgres statement taking more than 1 hour. ([\matrix-org#15853](matrix-org#15853))
- Fix the `devenv up` configuration which was ignoring the config overrides. ([\matrix-org#15854](matrix-org#15854))
- Optimised cleanup of old entries in `device_lists_stream`. ([\matrix-org#15861](matrix-org#15861))
- Update the Matrix clients link in the _It works! Synapse is running_ landing page. ([\matrix-org#15874](matrix-org#15874))
- Fix building Synapse with the nightly Rust compiler. ([\matrix-org#15906](matrix-org#15906))
- Add `Server` to Access-Control-Expose-Headers header. ([\matrix-org#15908](matrix-org#15908))

* Bump authlib from 1.2.0 to 1.2.1. ([\matrix-org#15864](matrix-org#15864))
* Bump importlib-metadata from 6.6.0 to 6.7.0. ([\matrix-org#15865](matrix-org#15865))
* Bump lxml from 4.9.2 to 4.9.3. ([\matrix-org#15897](matrix-org#15897))
* Bump regex from 1.8.4 to 1.9.1. ([\matrix-org#15902](matrix-org#15902))
* Bump ruff from 0.0.275 to 0.0.277. ([\matrix-org#15900](matrix-org#15900))
* Bump sentry-sdk from 1.25.1 to 1.26.0. ([\matrix-org#15867](matrix-org#15867))
* Bump serde_json from 1.0.99 to 1.0.100. ([\matrix-org#15901](matrix-org#15901))
* Bump types-pyopenssl from 23.2.0.0 to 23.2.0.1. ([\matrix-org#15866](matrix-org#15866))

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE8SRSDO7gYkSP4chELS76LzL74EcFAmS2ncYACgkQLS76LzL7
# 4EfaiA/9HUdk1tIlnQyIZDAT0d9RhmaL+KL1Fkk4wpi16QrtJ7gqLTrq1BXDxOYM
# 2bycL0yHN9gPu3f7TI0ic4p17T/vud8Fd7FoPJTkUZ7dFHsEPICtNmIp6ZkpuDYW
# Sv8QKuEeMxe98XCKxiI+zctu8wtNsrnu2RECD0zUqf5rMgbabuYnpSge2CqKftuf
# CfmYN161QjnONavQTk4iYSFmJpRZwvwoAlpMPsqkMIrhId2ko2SkPj1HBPrAFrFs
# Fq/PaVZQRZk15wnQzrLrlRAEfq/quoOSDnJSlUvMPWjsQUVP8Ug149L9oUIiwhqq
# zQtuL9dl5xGoUWMiWOP8937gVeA/lsJpcVPka3G0g3mIR8ukbHUOm2fZReV30xp8
# 81xpu8KwzDR+/Oo3INYsqoOiQ/t7Myg8sTgiJBMParuRmfqnsbdUWG8pEeUMzDYY
# 4+yzRrHo9KnHcAMEFT94rqVhgl7OQh2Fx9zduW7YOVk0wo0EBMl/rcfEsvvl//l0
# sSykqGx1XIr3Cdynp1PjCYJsYdLJzG73aSVh5qPY5sftsHIQ8DiiKX+UlSqlguMW
# 1ndkCuz3fK/+9CFGbBiixe/RKFxn0CVp3cBU6cCVzyLJerZpXrOkyMmSuZtOuIaH
# cLLGK2bQSbOegPtg4qjpL537xmk94tUiSGhEdt7M4wsHm4uRv0M=
# =QYwZ
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue Jul 18 15:12:22 2023 BST
# gpg:                using RSA key F124520CEEE062448FE1C8442D2EFA2F32FBE047
# gpg: Can't check signature: No public key

# Conflicts:
#	.github/workflows/latest_deps.yml
#	.github/workflows/release-artifacts.yml
#	.github/workflows/tests.yml
#	.github/workflows/twisted_trunk.yml
#	docker/Dockerfile
#	poetry.lock
#	synapse/api/auth/base.py
#	synapse/config/experimental.py
#	synapse/handlers/pagination.py
#	synapse/handlers/room_batch.py
#	synapse/push/bulk_push_rule_evaluator.py
#	synapse/rest/admin/users.py
#	synapse/rest/client/room_batch.py
#	synapse/storage/databases/main/__init__.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could not serialize access due to concurrent DELETE from presence_stream
2 participants