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

Don't include appservice users when calculating push rules #13332

Merged
merged 4 commits into from
Jul 20, 2022

Conversation

erikjohnston
Copy link
Member

This can cause a lot of extra load on servers with lots of appservice users. Introduced in #13078

@erikjohnston erikjohnston added the X-Regression Something broke which worked on a previous release label Jul 20, 2022
Copy link
Contributor

@MatMaul MatMaul left a comment

Choose a reason for hiding this comment

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

LGTM.

@clokep
Copy link
Member

clokep commented Jul 20, 2022

Is this something we should have a unit test for?

@erikjohnston
Copy link
Member Author

Testing is cheating!

(yes we should)

@erikjohnston erikjohnston marked this pull request as ready for review July 20, 2022 10:31
@erikjohnston erikjohnston requested a review from a team as a code owner July 20, 2022 10:31
@erikjohnston
Copy link
Member Author

I've added a dodgy test, and confirmed it breaks on v1.63.0

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 otherwise. Shame we (I?) missed this in review.

Do we want to do a 1.63.1 release with this fix?

changelog.d/13332.bugfix Outdated Show resolved Hide resolved
Co-authored-by: David Robertson <davidr@element.io>
@erikjohnston
Copy link
Member Author

Do we want to do a 1.63.1 release with this fix?

I think so, yes

@erikjohnston erikjohnston merged commit b4ae3b0 into release-v1.63 Jul 20, 2022
@erikjohnston erikjohnston deleted the erikj/fix_appservices branch July 20, 2022 11:06
@clokep
Copy link
Member

clokep commented Jul 20, 2022

I've added a dodgy test, and confirmed it breaks on v1.63.0

Thank you!

@DMRobertson DMRobertson linked an issue Jul 20, 2022 that may be closed by this pull request
Fizzadar pushed a commit to beeper/synapse-legacy-fork that referenced this pull request Aug 23, 2022
Synapse 1.63.1 (2022-07-20)
===========================

Bugfixes
--------

- Fix a bug introduced in Synapse 1.63.0 where push actions were incorrectly calculated for appservice users. This caused performance issues on servers with large numbers of appservices. ([\matrix-org#13332](matrix-org#13332))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
X-Regression Something broke which worked on a previous release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance Regression in 1.63
4 participants