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

Add membership column to current_state_events table #5706

Merged
merged 7 commits into from
Jul 19, 2019
Merged

Add membership column to current_state_events table #5706

merged 7 commits into from
Jul 19, 2019

Conversation

erikjohnston
Copy link
Member

Staring at pg_stat_statements on matrix.org's synapse shows a lot of IO goes on queries that are of the form current_state_events INNER JOIN room_memberships. This PR folds in the membership column into the current_state_events table so that we don't have to do the join. This should hopefully mean a lot less IO, since current_state_events table can fit in the disk cache.

The commits should be separately reviewable.

@richvdh richvdh self-requested a review July 18, 2019 09:19
@codecov
Copy link

codecov bot commented Jul 18, 2019 •
edited
Loading

Codecov Report

Merging #5706 into develop will decrease coverage by 0.08%.
The diff coverage is 78.04%.

@@             Coverage Diff             @@
##           develop    #5706      +/-   ##
===========================================
- Coverage    63.42%   63.34%   -0.09%     
===========================================
  Files          331      331              
  Lines        36163    36176      +13     
  Branches      5960     5960              
===========================================
- Hits         22938    22915      -23     
- Misses       11597    11627      +30     
- Partials      1628     1634       +6

@erikjohnston erikjohnston force-pushed the erikj/add_memberships_to_current_state branch from 9f75cdc to eabf0a4 Compare July 18, 2019 13:03
@erikjohnston erikjohnston force-pushed the erikj/add_memberships_to_current_state branch from eabf0a4 to 89c8859 Compare July 18, 2019 13:16
richvdh
richvdh reviewed Jul 19, 2019
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

couple of nits, generally lgtm

next we're going to need an index which includes membership?

synapse/storage/schema/delta/56/current_state_events_membership.sql Show resolved Hide resolved
synapse/storage/prepare_database.py
@@ -27,7 +27,7 @@

# Remember to update this number every time a change is made to database
# schema files, so the users will be informed on server restarts.
SCHEMA_VERSION = 55
SCHEMA_VERSION = 56
Copy link
Member

Choose a reason for hiding this comment

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

what's the thinking behind the schema bump?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mainly that this stops people from rolling back, which will cause membership column to not be correctly updated.

synapse/storage/roommember.py Outdated Show resolved Hide resolved
synapse/storage/roommember.py
if pending_update:
self._clock.call_later(
15.0,
run_as_background_process,
Copy link
Member

Choose a reason for hiding this comment

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

\o/

@erikjohnston
Update comment for new column
ebc5ed1
@erikjohnston
LoggingTransaction accepts None for callback lists.
bd2e1a2 
Its a bit disingenuousto give LoggingTransaction lists to append
callbacks to if we're not going to run the callbacks.
@erikjohnston erikjohnston requested a review from richvdh July 19, 2019 12:57
@erikjohnston
Copy link
Member Author

next we're going to need an index which includes membership?

Maybe, though I don't think its the end of the world for postgres to scan all membership events in current state tbh.

@erikjohnston
Copy link
Member Author

Although I guess we have the index:

    "current_state_events_member_index" btree (state_key) WHERE type = 'm.room.member'::text

which might be good to add a membership to, perhaps.

richvdh
richvdh approved these changes Jul 19, 2019
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

@erikjohnston erikjohnston merged commit 5c07c97 into develop Jul 19, 2019
This was referenced Jul 23, 2019
Merged
Merged
babolivier added a commit that referenced this pull request Aug 15, 2019
@babolivier
Merge tag 'v1.3.0'
6382914 
Synapse 1.3.0 (2019-08-15)
==========================

Bugfixes
--------

- Fix 500 Internal Server Error on `publicRooms` when the public room list was
  cached. ([\#5851](#5851))

Synapse 1.3.0rc1 (2019-08-13)
==========================

Features
--------

- Use `M_USER_DEACTIVATED` instead of `M_UNKNOWN` for errcode when a deactivated user attempts to login. ([\#5686](#5686))
- Add sd_notify hooks to ease systemd integration and allows usage of Type=Notify. ([\#5732](#5732))
- Synapse will no longer serve any media repo admin endpoints when `enable_media_repo` is set to False in the configuration. If a media repo worker is used, the admin APIs relating to the media repo will be served from it instead. ([\#5754](#5754), [\#5848](#5848))
- Synapse can now be configured to not join remote rooms of a given "complexity" (currently, state events) over federation. This option can be used to prevent adverse performance on resource-constrained homeservers. ([\#5783](#5783))
- Allow defining HTML templates to serve the user on account renewal attempt when using the account validity feature. ([\#5807](#5807))

Bugfixes
--------

- Fix UISIs during homeserver outage. ([\#5693](#5693), [\#5789](#5789))
- Fix stack overflow in server key lookup code. ([\#5724](#5724))
- start.sh no longer uses deprecated cli option. ([\#5725](#5725))
- Log when we receive an event receipt from an unexpected origin. ([\#5743](#5743))
- Fix debian packaging scripts to correctly build sid packages. ([\#5775](#5775))
- Correctly handle redactions of redactions. ([\#5788](#5788))
- Return 404 instead of 403 when accessing /rooms/{roomId}/event/{eventId} for an event without the appropriate permissions. ([\#5798](#5798))
- Fix check that tombstone is a state event in push rules. ([\#5804](#5804))
- Fix error when trying to login as a deactivated user when using a worker to handle login. ([\#5806](#5806))
- Fix bug where user `/sync` stream could get wedged in rare circumstances. ([\#5825](#5825))
- The purge_remote_media.sh script was fixed. ([\#5839](#5839))

Deprecations and Removals
-------------------------

- Synapse now no longer accepts the `-v`/`--verbose`, `-f`/`--log-file`, or `--log-config` command line flags, and removes the deprecated `verbose` and `log_file` configuration file options. Users of these options should migrate their options into the dedicated log configuration. ([\#5678](#5678), [\#5729](#5729))
- Remove non-functional 'expire_access_token' setting. ([\#5782](#5782))

Internal Changes
----------------

- Make Jaeger fully configurable. ([\#5694](#5694))
- Add precautionary measures to prevent future abuse of `window.opener` in default welcome page. ([\#5695](#5695))
- Reduce database IO usage by optimising queries for current membership. ([\#5706](#5706), [\#5738](#5738), [\#5746](#5746), [\#5752](#5752), [\#5770](#5770), [\#5774](#5774), [\#5792](#5792), [\#5793](#5793))
- Improve caching when fetching `get_filtered_current_state_ids`. ([\#5713](#5713))
- Don't accept opentracing data from clients. ([\#5715](#5715))
- Speed up PostgreSQL unit tests in CI. ([\#5717](#5717))
- Update the coding style document. ([\#5719](#5719))
- Improve database query performance when recording retry intervals for remote hosts. ([\#5720](#5720))
- Add a set of opentracing utils. ([\#5722](#5722))
- Cache result of get_version_string to reduce overhead of `/version` federation requests. ([\#5730](#5730))
- Return 'user_type' in admin API user endpoints results. ([\#5731](#5731))
- Don't package the sytest test blacklist file. ([\#5733](#5733))
- Replace uses of returnValue with plain return, as returnValue is not needed on Python 3. ([\#5736](#5736))
- Blacklist some flakey tests in worker mode. ([\#5740](#5740))
- Fix some error cases in the caching layer. ([\#5749](#5749))
- Add a prometheus metric for pending cache lookups. ([\#5750](#5750))
- Stop trying to fetch events with event_id=None. ([\#5753](#5753))
- Convert RedactionTestCase to modern test style. ([\#5768](#5768))
- Allow looping calls to be given arguments. ([\#5780](#5780))
- Set the logs emitted when checking typing and presence timeouts to DEBUG level, not INFO. ([\#5785](#5785))
- Remove DelayedCall debugging from the test suite, as it is no longer required in the vast majority of Synapse's tests. ([\#5787](#5787))
- Remove some spurious exceptions from the logs where we failed to talk to a remote server. ([\#5790](#5790))
- Improve performance when making `.well-known` requests by sharing the SSL options between requests. ([\#5794](#5794))
- Disable codecov GitHub comments on PRs. ([\#5796](#5796))
- Don't allow clients to send tombstone events that reference the room it's sent in. ([\#5801](#5801))
- Deny redactions of events sent in a different room. ([\#5802](#5802))
- Deny sending well known state types as non-state events. ([\#5805](#5805))
- Handle incorrectly encoded query params correctly by returning a 400. ([\#5808](#5808))
- Handle pusher being deleted during processing rather than logging an exception. ([\#5809](#5809))
- Return 502 not 500 when failing to reach any remote server. ([\#5810](#5810))
- Reduce global pauses in the events stream caused by expensive state resolution during persistence. ([\#5826](#5826))
- Add a lower bound to well-known lookup cache time to avoid repeated lookups. ([\#5836](#5836))
- Whitelist history visbility sytests in worker mode tests. ([\#5843](#5843))
@erikjohnston erikjohnston deleted the erikj/add_memberships_to_current_state branch January 9, 2020 15:50
anoadragon453 added a commit that referenced this pull request Feb 19, 2020
@anoadragon453
Merge pull request #5706 from matrix-org/erikj/add_memberships_to_cur…
04d4c31 
…rent_state
@richvdh richvdh mentioned this pull request May 1, 2020
Merged
richvdh added a commit that referenced this pull request May 1, 2020
@richvdh
Wait for current_state_events_membership before populate_stats_proces…
97ef147 
…s_rooms (#7387)

populate_stats_process_rooms was added in #5971 / v1.4.0; current_state_events_membership was added in #5706 / v1.3.0.

Fixes #7380.
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Jun 16, 2020
@richvdh @phil-flex
Wait for current_state_events_membership before populate_stats_proces…
aea0541 
…s_rooms (matrix-org#7387)

populate_stats_process_rooms was added in matrix-org#5971 / v1.4.0; current_state_events_membership was added in matrix-org#5706 / v1.3.0.

Fixes matrix-org#7380.
@Fizzadar Fizzadar mentioned this pull request Sep 7, 2022
Merged
4 tasks
MadLittleMods added a commit that referenced this pull request Jun 6, 2023
@MadLittleMods
No need for the extra join since membership is built-in to `current_s…
a23749e 
…tate_events`

`current_state_events` was added in #5706
and #13745
@MadLittleMods MadLittleMods mentioned this pull request Jun 6, 2023
Merged
4 tasks
MadLittleMods added a commit that referenced this pull request Jun 7, 2023
@MadLittleMods
No need for the extra join since membership is built-in to `current…
9d911b0 
…_state_events` (#15731)

This helps with the upstream `is_host_joined()` and `is_host_invited()` functions.

`membership` was added to `current_state_events` in #5706 and forced in #13745
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@richvdh richvdh richvdh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erikjohnston @richvdh