From 0046d7278bd8e350dcef40b95a05e116e6e66d90 Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Wed, 16 Apr 2025 09:34:58 +0200
Subject: [PATCH 1/2] Fix ExternalIDReuse exception for concurrent transactions
 (#18342)

---
 changelog.d/18342.bugfix                      |  1 +
 .../storage/databases/main/registration.py    | 29 +++++++++++++++----
 2 files changed, 24 insertions(+), 6 deletions(-)
 create mode 100644 changelog.d/18342.bugfix

diff --git a/changelog.d/18342.bugfix b/changelog.d/18342.bugfix
new file mode 100644
index 000000000..6fa2fa679
--- /dev/null
+++ b/changelog.d/18342.bugfix
@@ -0,0 +1 @@
+Fix `ExternalIDReuse` exception after migrating to MAS on workers with a high traffic.
diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py
index eadbf4901..c43f31353 100644
--- a/synapse/storage/databases/main/registration.py
+++ b/synapse/storage/databases/main/registration.py
@@ -763,16 +763,33 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
             txn, self.get_user_by_external_id, (auth_provider, external_id)
         )
 
-        self.db_pool.simple_insert_txn(
+        # This INSERT ... ON CONFLICT DO NOTHING statement will cause a
+        # 'could not serialize access due to concurrent update'
+        # if the row is added concurrently by another transaction.
+        # This is exactly what we want, as it makes the transaction get retried
+        # in a new snapshot where we can check for a genuine conflict.
+        was_inserted = self.db_pool.simple_upsert_txn(
             txn,
             table="user_external_ids",
-            values={
-                "auth_provider": auth_provider,
-                "external_id": external_id,
-                "user_id": user_id,
-            },
+            keyvalues={"auth_provider": auth_provider, "external_id": external_id},
+            values={},
+            insertion_values={"user_id": user_id},
         )
 
+        if not was_inserted:
+            existing_id = self.db_pool.simple_select_one_onecol_txn(
+                txn,
+                table="user_external_ids",
+                keyvalues={"auth_provider": auth_provider, "user_id": user_id},
+                retcol="external_id",
+                allow_none=True,
+            )
+
+            if existing_id != external_id:
+                raise ExternalIDReuseException(
+                    f"{user_id!r} has external id {existing_id!r} for {auth_provider} but trying to add {external_id!r}"
+                )
+
     async def remove_user_external_id(
         self, auth_provider: str, external_id: str, user_id: str
     ) -> None:

From c16a981f22dd559b56caa94a46392c206be9a265 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erikj@element.io>
Date: Wed, 16 Apr 2025 14:14:56 +0100
Subject: [PATCH 2/2] Fix query for room participation (#18345)

Follow on from #18068

Currently the subquery in `UPDATE` is pointless, as it will still just
update all `room_membership` rows. Instead, we should look at the
current membership event ID (which is easily retrieved from
`local_current_membership`). We also add a `AND NOT participant` to noop
the `UPDATE` when the `participant` flag is already set.

cc @H-Shay
---
 changelog.d/18345.bugfix                     |  1 +
 synapse/storage/databases/main/roommember.py | 20 ++++++++------------
 2 files changed, 9 insertions(+), 12 deletions(-)
 create mode 100644 changelog.d/18345.bugfix

diff --git a/changelog.d/18345.bugfix b/changelog.d/18345.bugfix
new file mode 100644
index 000000000..c8a001d4a
--- /dev/null
+++ b/changelog.d/18345.bugfix
@@ -0,0 +1 @@
+Fix minor performance regression caused by tracking of room participation. Regressed in v1.128.0.
diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py
index a0a6dcd04..dfa7dd48d 100644
--- a/synapse/storage/databases/main/roommember.py
+++ b/synapse/storage/databases/main/roommember.py
@@ -1622,14 +1622,11 @@ class RoomMemberWorkerStore(EventsWorkerStore, CacheInvalidationWorkerStore):
             sql = """
                 UPDATE room_memberships
                 SET participant = true
-                WHERE (user_id, room_id) IN (
-                    SELECT user_id, room_id
-                    FROM room_memberships
-                    WHERE user_id = ?
-                    AND room_id = ?
-                    ORDER BY event_stream_ordering DESC
-                    LIMIT 1
+                WHERE event_id IN (
+                    SELECT event_id FROM local_current_membership
+                    WHERE user_id = ? AND room_id = ?
                 )
+                AND NOT participant
             """
             txn.execute(sql, (user_id, room_id))
 
@@ -1651,11 +1648,10 @@ class RoomMemberWorkerStore(EventsWorkerStore, CacheInvalidationWorkerStore):
         ) -> bool:
             sql = """
                 SELECT participant
-                FROM room_memberships
-                WHERE user_id = ?
-                AND room_id = ?
-                ORDER BY event_stream_ordering DESC
-                LIMIT 1
+                FROM local_current_membership AS l
+                INNER JOIN room_memberships AS r USING (event_id)
+                WHERE l.user_id = ?
+                AND l.room_id = ?
             """
             txn.execute(sql, (user_id, room_id))
             res = txn.fetchone()