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

Mainline matrix-org-hotfixes changes to RoomMemberHandler.update_membership #11995

Closed
babolivier opened this issue Feb 15, 2022 · 2 comments · Fixed by #11996
Closed

Mainline matrix-org-hotfixes changes to RoomMemberHandler.update_membership #11995

babolivier opened this issue Feb 15, 2022 · 2 comments · Fixed by #11996

Comments

@babolivier
Copy link
Contributor

babolivier commented Feb 15, 2022

Specifically this bit:

as_id = object()
if requester.app_service:
as_id = requester.app_service.id
then = self.clock.time_msec()
with (await self.member_limiter.queue(as_id)):
diff = self.clock.time_msec() - then
if diff > 80 * 1000:
# haproxy would have timed the request out anyway...
raise SynapseError(504, "took to long to process")
with (await self.member_linearizer.queue(key)):
diff = self.clock.time_msec() - then
if diff > 80 * 1000:
# haproxy would have timed the request out anyway...
raise SynapseError(504, "took to long to process")
result = await self.update_membership_locked(
requester,
target,
room_id,
action,
txn_id=txn_id,
remote_room_hosts=remote_room_hosts,
third_party_signed=third_party_signed,
ratelimit=ratelimit,
content=content,
new_room=new_room,
require_consent=require_consent,
outlier=outlier,
historical=historical,
prev_event_ids=prev_event_ids,
auth_event_ids=auth_event_ids,
)

It's been causing merge conflicts when merging release branches into matrix-org-hotfixes a few times lately due to new releases adding kwargs to update_membership_locked, e.g.:

diff --cc synapse/handlers/room_member.py
index 8ee1a6470,bf1a47efb..000000000
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@@ -483,43 -500,25 +501,65 @@@ class RoomMemberHandler(metaclass=abc.A
  
          key = (room_id,)
  
++<<<<<<< HEAD
 +        as_id = object()
 +        if requester.app_service:
 +            as_id = requester.app_service.id
 +
 +        then = self.clock.time_msec()
 +
 +        with (await self.member_limiter.queue(as_id)):
 +            diff = self.clock.time_msec() - then
 +
 +            if diff > 80 * 1000:
 +                # haproxy would have timed the request out anyway...
 +                raise SynapseError(504, "took to long to process")
 +
 +            with (await self.member_linearizer.queue(key)):
 +                diff = self.clock.time_msec() - then
 +
 +                if diff > 80 * 1000:
 +                    # haproxy would have timed the request out anyway...
 +                    raise SynapseError(504, "took to long to process")
 +
 +                result = await self.update_membership_locked(
 +                    requester,
 +                    target,
 +                    room_id,
 +                    action,
 +                    txn_id=txn_id,
 +                    remote_room_hosts=remote_room_hosts,
 +                    third_party_signed=third_party_signed,
 +                    ratelimit=ratelimit,
 +                    content=content,
 +                    new_room=new_room,
 +                    require_consent=require_consent,
 +                    outlier=outlier,
 +                    historical=historical,
 +                    prev_event_ids=prev_event_ids,
 +                    auth_event_ids=auth_event_ids,
 +                )
++=======
+         with (await self.member_linearizer.queue(key)):
+             result = await self.update_membership_locked(
+                 requester,
+                 target,
+                 room_id,
+                 action,
+                 txn_id=txn_id,
+                 remote_room_hosts=remote_room_hosts,
+                 third_party_signed=third_party_signed,
+                 ratelimit=ratelimit,
+                 content=content,
+                 new_room=new_room,
+                 require_consent=require_consent,
+                 outlier=outlier,
+                 historical=historical,
+                 allow_no_prev_events=allow_no_prev_events,
+                 prev_event_ids=prev_event_ids,
+                 auth_event_ids=auth_event_ids,
+             )
++>>>>>>> release-v1.53
  
          return result

When merging the 1.53 release branch today. Resolving the conflict isn't difficult (just add the new kwargs to the update_membership_locked call) but it's not the first time we're running into this.

@babolivier
Copy link
Contributor Author

Looks like the commit causing issues is e5537cf (and I have a feeling ca21957 also doesn't help)

@babolivier
Copy link
Contributor Author

Also see #4826

babolivier added a commit that referenced this issue Feb 16, 2022
Initially introduced in matrix-org-hotfixes by e5537cf (and tweaked by later commits).

Fixes #11995

See also #4826
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 a pull request may close this issue.

1 participant