element-hq / element-hq/synapse

Race condition with replication means that publishing room aliases lacks read-after-write consistency between workers

Open
#14,210 0 comments 0 reactions 0 assignees View on GitHub
A-Testing A-Workers O-Uncommon S-Tolerable T-Defect Z-Read-After-Write
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

This issue has been migrated from [#14210](https://github.com/matrix-org/synapse/issues/14210).

---

Consider the following sequence of events:

1. Alice creates a room without any aliases.
2. Alice lists aliases for that room.
3. Alice sets an alias for that room.
4. Alice lists aliases for that room.

If the alias writes occur on a separate worker to the reads, this is vulnerable to a classic worker cache invalidation race:

- (2) succeeds because the reader has no cached alias information for the room. It queries the database (which is written before (1) completes) and caches the result.
- (3) succeeds on the writer, which fires off a message telling readers to invalidate their caches.
- :warning: If request (4) arrives before the reader has received and processed the invalidation, the reader will return the (now stale) data in its cache. This means Alice has failed to read her own write.

I don't think actual humans edit and then immediately list aliases that often, so I suggest we don't worry about fixing this. (i.e. I think this only manifests as test flakes). But I wanted to write this up as a reference. (It would be nice to have a catalogue of known races like this).

### History:

See issues labeled with https://github.com/matrix-org/synapse/labels/Z-Read-After-Write

And previous related history specifically around aliases:

- https://github.com/matrix-org/sytest/pull/1053 moved requests off the main worker in sytest
- https://github.com/matrix-org/sytest/issues/1055 this causes a broken/flakey test
- https://github.com/matrix-org/sytest/pull/1056 introduces retry logic to work around this
- https://github.com/matrix-org/complement/pull/266 sytest ported to complement
- https://github.com/matrix-org/synapse/issues/12638 We start testing complement with workers
- Unknown: something happens to cause that test to start failing in worker mode on complement. Possibly https://github.com/matrix-org/synapse/pull/14165? Unconfirmed.
- https://github.com/matrix-org/synapse/issues/14183 we notice the failures.
- https://github.com/matrix-org/complement/pull/521 readds the retry logic

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.