/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Speed up filtering of a single event in push #2080

Merged
merged 3 commits into from Apr 28, 2017

Conversation

Reviewers

@NegativeMjark NegativeMjark

Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@erikjohnston @NegativeMjark @richvdh
@erikjohnston
Owner

erikjohnston commented Mar 29, 2017

The filtering we do with events we're calculating the push rules for is much simpler than the general case as we know the users are currently joined in the room. This changes the filter_events_for_clients_context to take advantage of that fact (which was only used from push)

@erikjohnston erikjohnston assigned richvdh and unassigned richvdh Mar 29, 2017

@erikjohnston
Owner

erikjohnston commented Mar 30, 2017

(Actually, I think I will change this to work differently so we can cache things better)

@erikjohnston erikjohnston assigned NegativeMjark and NegativeMjark and unassigned NegativeMjark Apr 12, 2017

synapse/storage/push_rule.py
@@ -184,11 +185,23 @@ def _bulk_get_push_rules_for_room(self, room_id, state_group, current_state_ids,
if uid in local_users_in_room:
user_ids.add(uid)
+ forgotten = yield self.who_forgot_in_room(
+ event.room_id, on_invalidate=cache_context.invalidate,
+ )
@NegativeMjark

NegativeMjark Apr 20, 2017

Contributor

Why do we need to check if the user has forgotten the room? I guess it's because the user might have left the room and forgotten it but we are still getting new messages with them joined to the room over federation?
Should we be checking whether the user is currently in the room instead?

I'd really like to have some comments here to explain what we are doing and why.

@erikjohnston

erikjohnston Apr 27, 2017

Owner

To be honest I just copied from the filtering code. I'm happy to drop it if we think we can?

@NegativeMjark

NegativeMjark Apr 27, 2017

Contributor

I think you are safe to ignore the forgotten stuff if we are only looking for rooms that the user is joined/invited to. I think you can only forget a room that you have left?

@erikjohnston

erikjohnston Apr 27, 2017

Owner

¯_(ツ)_/¯

erikjohnston added some commits Mar 29, 2017

@erikjohnston
Speed up filtering of a single event in push
421fdf7
@erikjohnston
We don't care about forgotten rooms

Some checks were not successful

2 failing and 4 successful checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
continuous-integration/travis-ci/push — The Travis CI build failed
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2935 origin/erikj/filter_speed succeeded in 6 min 11 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
ad8b316
@erikjohnston
Remove unused import

All checks have passed

8 successful checks
@matrixbot
Sytest Dendron (Commit) — Build #2036 origin/erikj/filter_speed succeeded in 9 min 44 sec
Details
@matrixbot
Sytest Dendron (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #2868 origin/erikj/filter_speed succeeded in 7 min 14 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #2936 origin/erikj/filter_speed succeeded in 6 min 51 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
ab37bef

@erikjohnston erikjohnston merged commit 3033261 into develop Apr 28, 2017
8 checks passed

8 checks passed

Sytest Dendron (Commit) Build #2036 origin/erikj/filter_speed succeeded in 9 min 44 sec
Details
Sytest Dendron (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #2868 origin/erikj/filter_speed succeeded in 7 min 14 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #2936 origin/erikj/filter_speed succeeded in 6 min 51 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

psaavedra added a commit to psaavedra/synapse that referenced this pull request May 19, 2017

@psaavedra
Merge tag 'v0.21.0' into v0.21.0_no_federate_by_default 
Changes in synapse v0.21.0 (2017-05-18)
=======================================

No changes since v0.21.0-rc3

Changes in synapse v0.21.0-rc3 (2017-05-17)
===========================================

Features:

* Add per user rate-limiting overrides (PR #2208)
* Add config option to limit maximum number of events requested by ``/sync``
  and ``/messages`` (PR #2221) Thanks to @psaavedra!

Changes:

* Various small performance fixes (PR #2201, #2202, #2224, #2226, #2227, #2228,
  #2229)
* Update username availability checker API (PR #2209, #2213)
* When purging, don't de-delta state groups we're about to delete (PR #2214)
* Documentation to check synapse version (PR #2215) Thanks to @hamber-dick!
* Add an index to event_search to speed up purge history API (PR #2218)

Bug fixes:

* Fix API to allow clients to upload one-time-keys with new sigs (PR #2206)

Changes in synapse v0.21.0-rc2 (2017-05-08)
===========================================

Changes:

* Always mark remotes as up if we receive a signed request from them (PR #2190)

Bug fixes:

* Fix bug where users got pushed for rooms they had muted (PR #2200)

Changes in synapse v0.21.0-rc1 (2017-05-08)
===========================================

Features:

* Add username availability checker API (PR #2183)
* Add read marker API (PR #2120)

Changes:

* Enable guest access for the 3pl/3pid APIs (PR #1986)
* Add setting to support TURN for guests (PR #2011)
* Various performance improvements (PR #2075, #2076, #2080, #2083, #2108,
  #2158, #2176, #2185)
* Make synctl a bit more user friendly (PR #2078, #2127) Thanks @APwhitehat!
* Replace HTTP replication with TCP replication (PR #2082, #2097, #2098,
  #2099, #2103, #2014, #2016, #2115, #2116, #2117)
* Support authenticated SMTP (PR #2102) Thanks @DanielDent!
* Add a counter metric for successfully-sent transactions (PR #2121)
* Propagate errors sensibly from proxied IS requests (PR #2147)
* Add more granular event send metrics (PR #2178)

Bug fixes:

* Fix nuke-room script to work with current schema (PR #1927) Thanks
  @zuckschwerdt!
* Fix db port script to not assume postgres tables are in the public schema
  (PR #2024) Thanks @jerrykan!
* Fix getting latest device IP for user with no devices (PR #2118)
* Fix rejection of invites to unreachable servers (PR #2145)
* Fix code for reporting old verify keys in synapse (PR #2156)
* Fix invite state to always include all events (PR #2163)
* Fix bug where synapse would always fetch state for any missing event (PR #2170)
* Fix a leak with timed out HTTP connections (PR #2180)
* Fix bug where we didn't time out HTTP requests to ASes  (PR #2192)

Docs:

* Clarify doc for SQLite to PostgreSQL port (PR #1961) Thanks @benhylau!
* Fix typo in synctl help (PR #2107) Thanks @HarHarLinks!
* ``web_client_location`` documentation fix (PR #2131) Thanks @matthewjwolff!
* Update README.rst with FreeBSD changes (PR #2132) Thanks @feld!
* Clarify setting up metrics (PR #2149) Thanks @encks!
badd30b

@erikjohnston erikjohnston deleted the erikj/filter_speed branch Oct 26, 2017

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