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

Fix room creation being rate limited too aggressively since Synapse v1.69.0. #14314

Merged
merged 4 commits into from
Oct 28, 2022

Conversation

reivilibre
Copy link
Contributor

@reivilibre reivilibre commented Oct 27, 2022

Fixes #14312.

I make special effort to match the old limits, though I've tried to do it in a saner way: we apply the 2 units of ratelimiting upfront atomically rather than perhaps doing some work only to fail persisting the creator's membership event.
It doesn't seem to make sense to ratelimit half-way through creating a room — it's much better to avoid the wasted work and avoid half-creating a room (possibly leading to a janky state).

I did match the old limits, but now decided to be twice as lenient — Element Web's space creation screen always did fall over even with the old limits if you were fast enough. Given that we're making room creation more performant, it seems reasonable to apply the same cost as one event send — you're not really hurting anyone by creating empty rooms.

Test included for good measure.

@reivilibre reivilibre changed the base branch from develop to release-v1.70 October 27, 2022 17:41
@reivilibre reivilibre marked this pull request as ready for review October 27, 2022 17:41
@reivilibre reivilibre requested a review from a team as a code owner October 27, 2022 17:41
@reivilibre reivilibre marked this pull request as draft October 27, 2022 17:44
@reivilibre reivilibre removed the request for review from a team October 27, 2022 17:45
@clokep
Copy link
Member

clokep commented Oct 27, 2022

(Kind of related to #8895 I think?)

@reivilibre
Copy link
Contributor Author

(Kind of related to #8895 I think?)

Tangentially in that rate limiting is an extra cause of #8895.

…efore

Notably, the UI in Element Web was still broken after restoring to prior behaviour.

After discussion, we agreed that it would be sensible to increase the limit.
@reivilibre reivilibre marked this pull request as ready for review October 28, 2022 09:57
@reivilibre reivilibre requested a review from a team October 28, 2022 10:14
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM!

@DMRobertson
Copy link
Contributor

Uh oh, why is complement sad?

@DMRobertson
Copy link
Contributor

Uh oh, why is complement sad?

Because it's testing matrix-org/complement#525, the fix for which is on develop but not the release branch. Also there was a known flake (#14103). LGTmerge.

@reivilibre reivilibre merged commit 6a6e1e8 into release-v1.70 Oct 28, 2022
@reivilibre reivilibre deleted the rei/fixreg_space_creat_rate_limit branch October 28, 2022 10:53
chagai95 pushed a commit to chagai95/synapse that referenced this pull request Nov 5, 2022
Synapse 1.70.1 (2022-10-28)
===========================

Bugfixes
--------

- Fix a bug introduced in Synapse 1.70.0rc1 where the access tokens sent to application services as headers were malformed. Application services which were obtaining access tokens from query parameters were not affected. ([\matrix-org#14301](matrix-org#14301))
- Fix room creation being rate limited too aggressively since Synapse v1.69.0. ([\matrix-org#14314](matrix-org#14314))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants