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

Don't pull out the full state when calculating push actions #13078

Merged

erikjohnston merged 56 commits into develop from erikj/less_push_state

Jul 11, 2022

Member

erikjohnston commented Jun 16, 2022 •

edited

Loading

Fixes #12653

erikjohnston and others added 9 commits

June 8, 2022 16:29


          Don't pull out stuff for push

9f3afd2


          Reduce state that push rules pull from DB

fe6175a


          Ignore display name push

24c3974


          fetch display name

944f627


          filter visibility of push event

05005c4


          add filter_events_for_client_with_context and move _check_visibility …

c16260e

…to visibility.py


          Merge branch 'develop' into erikj/less_push_state

8260c30


          refactor filter_events_for_client to use _check_visibility

d1378f5


          fix typo in cache.py

6c83781

erikjohnston force-pushed the erikj/less_push_state branch 2 times, most recently from 3ce63fd to 2e4be71 Compare

June 28, 2022 08:30

erikjohnston and others added 5 commits

June 28, 2022 09:39


          Don't pull out stuff for push

fb0a78c


          Reduce state that push rules pull from DB

83ff92d


          Fetch display names from DB.

4966b03


          filter visibility of push event

21951c3


          Newsfile

f7ace02

erikjohnston force-pushed the erikj/less_push_state branch from 2e4be71 to f7ace02 Compare

June 28, 2022 08:39

erikjohnston commented

View reviewed changes

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

H-Shay added 3 commits

June 29, 2022 14:30


          Merge branch 'erikj/less_push_state' of https://github.com/matrix-org…

d186237

…/synapse into erikj/less_push_state


          remove rulesforroom and use room-specific profiles

4a0836e


          alter _get_users_in_room_with_profiles to take an optional list of us…

6f9dd6f

…er ids

erikjohnston commented

View reviewed changes

synapse/storage/_base.py Outdated Show resolved Hide resolved

erikjohnston commented

View reviewed changes

synapse/storage/databases/main/roommember.py Show resolved Hide resolved

erikjohnston commented

View reviewed changes

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

erikjohnston commented

View reviewed changes

synapse/push/bulk_push_rule_evaluator.py Show resolved Hide resolved

erikjohnston commented

View reviewed changes

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

erikjohnston commented

View reviewed changes

synapse/storage/databases/main/roommember.py Outdated Show resolved Hide resolved

Member Author

erikjohnston commented Jul 5, 2022

Thanks Shay, this is really starting to look good! Had a quick look through what is currently there (not sure if it was ready for my review or not!), and it's mostly comments around getting the caching correct (which is always the really fiddly and brittle part). LMK if you want to jump on a call to discuss, exactly how to do caching isn't exactly obvious.

t3chguy mentioned this pull request

Notif panel is still showing moderator-deleted messages element-hq/element-web#22685

Closed

erikjohnston commented

View reviewed changes

synapse/visibility.py Outdated Show resolved Hide resolved


          Merge branch 'erikj/visibility_multiple_users' into erikj/less_push_s…

c30ee0e

…tate

erikjohnston changed the base branch from develop to erikj/visibility_multiple_users

July 11, 2022 10:16

erikjohnston marked this pull request as ready for review

July 11, 2022 10:19

erikjohnston requested a review from a team as a code owner

July 11, 2022 10:19

DMRobertson self-assigned this

Base automatically changed from erikj/visibility_multiple_users to develop

July 11, 2022 13:14


          Merge branch 'develop' into erikj/less_push_state

18e818c

DMRobertson reviewed

View reviewed changes

Contributor

DMRobertson left a comment

TL;DR: nothing looks wrong; it looks simpler; it seems sane. Nitpicks and general reviewer arsey remains.

synapse/storage/databases/main/cache.py Outdated Show resolved Hide resolved

synapse/storage/_base.py Outdated Show resolved Hide resolved

synapse/storage/databases/main/roommember.py Outdated Show resolved Hide resolved

synapse/storage/databases/main/roommember.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

synapse/push/bulk_push_rule_evaluator.py Outdated Show resolved Hide resolved

erikjohnston added 9 commits

July 11, 2022 18:53


          Fix invalidation

d370472


          Rewrap lines

94c5419


          Use Collection

ebf01b9


          Update docstring

47cfc15


          Remove pointless inclusion of join event

615d053


          Merge get_rules functions

1e26f18


          Remove pointless copy

763dd3b


          Fix logging

858ebe8


          Add note about efficiency

fadc16a

erikjohnston requested a review from DMRobertson

July 11, 2022 18:14

erikjohnston added 3 commits

July 11, 2022 19:16


          Mark get_local_users_in_room as iterable cache

f9be434


          Merge remote-tracking branch 'origin/develop' into erikj/less_push_state

9c4e507


          Up the query counts in tests

6429b27

DMRobertson approved these changes

View reviewed changes

Contributor

DMRobertson left a comment

Sytest failure looks to be matrix-org/sytest#1125.

LGTM otherwise; I'm happy if CI is happy.

erikjohnston enabled auto-merge (squash)

July 11, 2022 20:03

erikjohnston merged commit e5716b6 into develop

erikjohnston deleted the erikj/less_push_state branch

July 11, 2022 20:08

davidmehren mentioned this pull request

Performance Regression in 1.63 #13331

Closed

erikjohnston mentioned this pull request

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

Merged

erikjohnston added a commit that referenced this pull request


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

b4ae3b0

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

Shudogg reviewed

View reviewed changes

Shudogg left a comment

Msg

richvdh mentioned this pull request

Remove unused @lru_cache decorator #13595

Merged

4 tasks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet