Skip to content

Commit 0046d72

Browse files
authored
Fix ExternalIDReuse exception for concurrent transactions (#18342)
1 parent 2c7a61e commit 0046d72

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

changelog.d/18342.bugfix

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix `ExternalIDReuse` exception after migrating to MAS on workers with a high traffic.

synapse/storage/databases/main/registration.py

+23-6
Original file line numberDiff line numberDiff line change
@@ -763,16 +763,33 @@ def _record_user_external_id_txn(
763763
txn, self.get_user_by_external_id, (auth_provider, external_id)
764764
)
765765

766-
self.db_pool.simple_insert_txn(
766+
# This INSERT ... ON CONFLICT DO NOTHING statement will cause a
767+
# 'could not serialize access due to concurrent update'
768+
# if the row is added concurrently by another transaction.
769+
# This is exactly what we want, as it makes the transaction get retried
770+
# in a new snapshot where we can check for a genuine conflict.
771+
was_inserted = self.db_pool.simple_upsert_txn(
767772
txn,
768773
table="user_external_ids",
769-
values={
770-
"auth_provider": auth_provider,
771-
"external_id": external_id,
772-
"user_id": user_id,
773-
},
774+
keyvalues={"auth_provider": auth_provider, "external_id": external_id},
775+
values={},
776+
insertion_values={"user_id": user_id},
774777
)
775778

779+
if not was_inserted:
780+
existing_id = self.db_pool.simple_select_one_onecol_txn(
781+
txn,
782+
table="user_external_ids",
783+
keyvalues={"auth_provider": auth_provider, "user_id": user_id},
784+
retcol="external_id",
785+
allow_none=True,
786+
)
787+
788+
if existing_id != external_id:
789+
raise ExternalIDReuseException(
790+
f"{user_id!r} has external id {existing_id!r} for {auth_provider} but trying to add {external_id!r}"
791+
)
792+
776793
async def remove_user_external_id(
777794
self, auth_provider: str, external_id: str, user_id: str
778795
) -> None:

0 commit comments

Comments
 (0)