From 7098d47f29d38daa5089bc1dbf0c60e99c5cafeb Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 30 Aug 2024 08:54:07 +0100 Subject: [PATCH] Sliding sync: Fix bg update again (v3) (#17634) Follow-up to https://github.com/element-hq/synapse/pull/17631 and https://github.com/element-hq/synapse/pull/17632 to fix-up https://github.com/element-hq/synapse/pull/17599 --------- Co-authored-by: Eric Eastwood --- changelog.d/17634.misc | 1 + .../databases/main/events_bg_updates.py | 31 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 changelog.d/17634.misc diff --git a/changelog.d/17634.misc b/changelog.d/17634.misc new file mode 100644 index 000000000..756918e2b --- /dev/null +++ b/changelog.d/17634.misc @@ -0,0 +1 @@ +Pre-populate room data used in experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint for quick filtering/sorting. diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 49ca985c4..2cb3f1d01 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1961,9 +1961,30 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS return 0 def _find_previous_membership_txn( - txn: LoggingTransaction, room_id: str, user_id: str, stream_ordering: int + txn: LoggingTransaction, room_id: str, user_id: str, event_id: str ) -> Tuple[str, str]: # Find the previous invite/knock event before the leave event + # + # Here are some notes on how we landed on this query: + # + # We're using `topological_ordering` instead of `stream_ordering` because + # somehow it's possible to have your `leave` event backfilled with a + # negative `stream_ordering` and your previous `invite` event with a + # positive `stream_ordering` so we wouldn't have a chance of finding the + # previous membership with a naive `event_stream_ordering < ?` comparison. + # + # Also be careful because `room_memberships.event_stream_ordering` is + # nullable and not always filled in. You would need to join on `events` to + # rely on `events.stream_ordering` instead. Even though the + # `events.stream_ordering` also doesn't have a `NOT NULL` constraint, it + # doesn't have any rows where this is the case (checked on `matrix.org`). + # The fact the `events.stream_ordering` is a nullable column is a holdover + # from a rename of the column. + # + # You might also consider using the `event_auth` table to find the previous + # membership, but there are cases where somehow a membership event doesn't + # point back to the previous membership event in the auth events (unknown + # cause). txn.execute( """ SELECT event_id, membership @@ -1972,14 +1993,14 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS WHERE room_id = ? AND m.user_id = ? - AND e.stream_ordering < ? - ORDER BY e.stream_ordering DESC + AND e.event_id != ? + ORDER BY e.topological_ordering DESC LIMIT 1 """, ( room_id, user_id, - stream_ordering, + event_id, ), ) row = txn.fetchone() @@ -2106,7 +2127,7 @@ class EventsBackgroundUpdatesStore(StreamWorkerStore, StateDeltasStore, SQLBaseS _find_previous_membership_txn, room_id, user_id, - membership_event_stream_ordering, + membership_event_id, ) )