From 3b38cf23923fec31aaeae8c804caa530b1dd441b Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Thu, 30 Mar 2023 18:37:28 +0200 Subject: [PATCH 1/4] Revert "Revert "Reintroduce membership tables event stream ordering (#15128)" (#15347)" This reverts commit 5350b5d04da8aca80b60a6b4970020d13d789501. --- changelog.d/15128.misc | 1 + synapse/storage/databases/main/events.py | 23 ++++-- .../storage/databases/main/purge_events.py | 6 +- synapse/storage/schema/__init__.py | 14 +++- ...embership_tables_event_stream_ordering.sql | 20 +++++ ...p_tables_event_stream_ordering_triggers.py | 79 +++++++++++++++++++ 6 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 changelog.d/15128.misc create mode 100644 synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql create mode 100644 synapse/storage/schema/main/delta/74/02membership_tables_event_stream_ordering_triggers.py diff --git a/changelog.d/15128.misc b/changelog.d/15128.misc new file mode 100644 index 000000000000..c09911e48d2f --- /dev/null +++ b/changelog.d/15128.misc @@ -0,0 +1 @@ +Add denormalised event stream ordering column to membership state tables for future use. Contributed by Nick @ Beeper (@fizzadar). diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 9c1e506da66c..ccd9f9d14189 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1127,11 +1127,15 @@ def _update_current_state_txn( # been inserted into room_memberships. txn.execute_batch( """INSERT INTO current_state_events - (room_id, type, state_key, event_id, membership) - VALUES (?, ?, ?, ?, (SELECT membership FROM room_memberships WHERE event_id = ?)) + (room_id, type, state_key, event_id, membership, event_stream_ordering) + VALUES ( + ?, ?, ?, ?, + (SELECT membership FROM room_memberships WHERE event_id = ?), + (SELECT stream_ordering FROM events WHERE event_id = ?) + ) """, [ - (room_id, key[0], key[1], ev_id, ev_id) + (room_id, key[0], key[1], ev_id, ev_id, ev_id) for key, ev_id in to_insert.items() ], ) @@ -1158,11 +1162,15 @@ def _update_current_state_txn( if to_insert: txn.execute_batch( """INSERT INTO local_current_membership - (room_id, user_id, event_id, membership) - VALUES (?, ?, ?, (SELECT membership FROM room_memberships WHERE event_id = ?)) + (room_id, user_id, event_id, membership, event_stream_ordering) + VALUES ( + ?, ?, ?, + (SELECT membership FROM room_memberships WHERE event_id = ?), + (SELECT stream_ordering FROM events WHERE event_id = ?) + ) """, [ - (room_id, key[1], ev_id, ev_id) + (room_id, key[1], ev_id, ev_id, ev_id) for key, ev_id in to_insert.items() if key[0] == EventTypes.Member and self.is_mine_id(key[1]) ], @@ -1768,6 +1776,7 @@ def _store_room_members_txn( table="room_memberships", keys=( "event_id", + "event_stream_ordering", "user_id", "sender", "room_id", @@ -1778,6 +1787,7 @@ def _store_room_members_txn( values=[ ( event.event_id, + event.internal_metadata.stream_ordering, event.state_key, event.user_id, event.room_id, @@ -1810,6 +1820,7 @@ def _store_room_members_txn( keyvalues={"room_id": event.room_id, "user_id": event.state_key}, values={ "event_id": event.event_id, + "event_stream_ordering": event.internal_metadata.stream_ordering, "membership": event.membership, }, ) diff --git a/synapse/storage/databases/main/purge_events.py b/synapse/storage/databases/main/purge_events.py index 7a7c0d9c753d..efbd3e75d99e 100644 --- a/synapse/storage/databases/main/purge_events.py +++ b/synapse/storage/databases/main/purge_events.py @@ -428,14 +428,16 @@ def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]: "partial_state_events", "partial_state_rooms_servers", "partial_state_rooms", + # Note: the _membership(s) tables have foreign keys to the `events` table + # so must be deleted first. + "local_current_membership", + "room_memberships", "events", "federation_inbound_events_staging", - "local_current_membership", "receipts_graph", "receipts_linearized", "room_aliases", "room_depth", - "room_memberships", "room_stats_state", "room_stats_current", "room_stats_earliest_token", diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index d3103a6c7a05..a28f2b997cef 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -SCHEMA_VERSION = 74 # remember to update the list below when updating +SCHEMA_VERSION = 75 # remember to update the list below when updating """Represents the expectations made by the codebase about the database schema This should be incremented whenever the codebase changes its requirements on the @@ -91,13 +91,19 @@ - A query on `event_stream_ordering` column has now been disambiguated (i.e. the codebase can handle the `current_state_events`, `local_current_memberships` and `room_memberships` tables having an `event_stream_ordering` column). + +Changes in SCHEMA_VERSION = 75: + - The `event_stream_ordering` column in membership tables (`current_state_events`, + `local_current_membership` & `room_memberships`) is now being populated for new + rows. When the background job to populate historical rows lands this will + become the compat schema version. """ SCHEMA_COMPAT_VERSION = ( - # The threads_id column must exist for event_push_actions, event_push_summary, - # receipts_linearized, and receipts_graph. - 73 + # Queries against `event_stream_ordering` columns in membership tables must + # be disambiguated. + 74 ) """Limit on how far the synapse codebase can be rolled back without breaking db compat diff --git a/synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql b/synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql new file mode 100644 index 000000000000..e2608f3a2e83 --- /dev/null +++ b/synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql @@ -0,0 +1,20 @@ +/* Copyright 2022 Beeper + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- Each of these are denormalised copies of `stream_ordering` from the corresponding row in` events` which +-- we use to improve database performance by reduring JOINs. +ALTER TABLE current_state_events ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); +ALTER TABLE local_current_membership ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); +ALTER TABLE room_memberships ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); diff --git a/synapse/storage/schema/main/delta/74/02membership_tables_event_stream_ordering_triggers.py b/synapse/storage/schema/main/delta/74/02membership_tables_event_stream_ordering_triggers.py new file mode 100644 index 000000000000..e32e9083b359 --- /dev/null +++ b/synapse/storage/schema/main/delta/74/02membership_tables_event_stream_ordering_triggers.py @@ -0,0 +1,79 @@ +# Copyright 2022 Beeper +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +""" +This migration adds triggers to the room membership tables to enforce consistency. +Triggers cannot be expressed in .sql files, so we have to use a separate file. +""" +from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine +from synapse.storage.types import Cursor + + +def run_create(cur: Cursor, database_engine: BaseDatabaseEngine, *args, **kwargs): + # Complain if the `event_stream_ordering` in membership tables doesn't match + # the `stream_ordering` row with the same `event_id` in `events`. + if isinstance(database_engine, Sqlite3Engine): + for table in ( + "current_state_events", + "local_current_membership", + "room_memberships", + ): + cur.execute( + f""" + CREATE TRIGGER IF NOT EXISTS {table}_bad_event_stream_ordering + BEFORE INSERT ON {table} + FOR EACH ROW + BEGIN + SELECT RAISE(ABORT, 'Incorrect event_stream_ordering in {table}') + WHERE EXISTS ( + SELECT 1 FROM events + WHERE events.event_id = NEW.event_id + AND events.stream_ordering != NEW.event_stream_ordering + ); + END; + """ + ) + elif isinstance(database_engine, PostgresEngine): + cur.execute( + """ + CREATE OR REPLACE FUNCTION check_event_stream_ordering() RETURNS trigger AS $BODY$ + BEGIN + IF EXISTS ( + SELECT 1 FROM events + WHERE events.event_id = NEW.event_id + AND events.stream_ordering != NEW.event_stream_ordering + ) THEN + RAISE EXCEPTION 'Incorrect event_stream_ordering'; + END IF; + RETURN NEW; + END; + $BODY$ LANGUAGE plpgsql; + """ + ) + + for table in ( + "current_state_events", + "local_current_membership", + "room_memberships", + ): + cur.execute( + f""" + CREATE TRIGGER check_event_stream_ordering BEFORE INSERT OR UPDATE ON {table} + FOR EACH ROW + EXECUTE PROCEDURE check_event_stream_ordering() + """ + ) + else: + raise NotImplementedError("Unknown database engine") From ddc12c343b8509ab46d20fe419d0cec5f2f7f69a Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Thu, 30 Mar 2023 18:44:50 +0200 Subject: [PATCH 2/4] Reintroduce migrations using `NOT VALID` foreign key constraints --- ...03_membership_tables_event_stream_ordering.sql} | 14 +++++++++++--- ...rship_tables_event_stream_ordering_triggers.py} | 0 2 files changed, 11 insertions(+), 3 deletions(-) rename synapse/storage/schema/main/delta/74/{01membership_tables_event_stream_ordering.sql => 03_membership_tables_event_stream_ordering.sql} (57%) rename synapse/storage/schema/main/delta/74/{02membership_tables_event_stream_ordering_triggers.py => 04_membership_tables_event_stream_ordering_triggers.py} (100%) diff --git a/synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql b/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql similarity index 57% rename from synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql rename to synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql index e2608f3a2e83..9c217e4accfb 100644 --- a/synapse/storage/schema/main/delta/74/01membership_tables_event_stream_ordering.sql +++ b/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql @@ -15,6 +15,14 @@ -- Each of these are denormalised copies of `stream_ordering` from the corresponding row in` events` which -- we use to improve database performance by reduring JOINs. -ALTER TABLE current_state_events ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); -ALTER TABLE local_current_membership ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); -ALTER TABLE room_memberships ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); +-- NOTE: these are set to NOT VALID to prevent locks while adding the column on large existing tables, +-- which will be validated in a later migration. For all new/updated rows the FKEY will be checked. + +ALTER TABLE current_state_events ADD COLUMN event_stream_ordering BIGINT; +ALTER TABLE current_state_events ADD CONSTRAINT event_stream_ordering_fkey FOREIGN KEY (event_stream_ordering) REFERENCES events(stream_ordering) NOT VALID; + +ALTER TABLE local_current_membership ADD COLUMN event_stream_ordering BIGINT; +ALTER TABLE local_current_membership ADD CONSTRAINT event_stream_ordering_fkey FOREIGN KEY (event_stream_ordering) REFERENCES events(stream_ordering) NOT VALID; + +ALTER TABLE room_memberships ADD COLUMN event_stream_ordering BIGINT; +ALTER TABLE room_memberships ADD CONSTRAINT event_stream_ordering_fkey FOREIGN KEY (event_stream_ordering) REFERENCES events(stream_ordering) NOT VALID; diff --git a/synapse/storage/schema/main/delta/74/02membership_tables_event_stream_ordering_triggers.py b/synapse/storage/schema/main/delta/74/04_membership_tables_event_stream_ordering_triggers.py similarity index 100% rename from synapse/storage/schema/main/delta/74/02membership_tables_event_stream_ordering_triggers.py rename to synapse/storage/schema/main/delta/74/04_membership_tables_event_stream_ordering_triggers.py From 64a59ddd1aba36af2260a7a02fd8cf61f1c3c5a6 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Thu, 30 Mar 2023 18:52:14 +0200 Subject: [PATCH 3/4] Update newsfile --- changelog.d/{15128.misc => 15356.misc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{15128.misc => 15356.misc} (100%) diff --git a/changelog.d/15128.misc b/changelog.d/15356.misc similarity index 100% rename from changelog.d/15128.misc rename to changelog.d/15356.misc From 4b27edfe825e54d049213f3331cded354016edf5 Mon Sep 17 00:00:00 2001 From: Nick Barrett Date: Thu, 30 Mar 2023 19:37:30 +0200 Subject: [PATCH 4/4] Support sqlite migrations --- ...tables_event_stream_ordering.sql.postgres} | 1 + ...ip_tables_event_stream_ordering.sql.sqlite | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) rename synapse/storage/schema/main/delta/74/{03_membership_tables_event_stream_ordering.sql => 03_membership_tables_event_stream_ordering.sql.postgres} (99%) create mode 100644 synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.sqlite diff --git a/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql b/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.postgres similarity index 99% rename from synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql rename to synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.postgres index 9c217e4accfb..ceb750a9fa51 100644 --- a/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql +++ b/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.postgres @@ -15,6 +15,7 @@ -- Each of these are denormalised copies of `stream_ordering` from the corresponding row in` events` which -- we use to improve database performance by reduring JOINs. + -- NOTE: these are set to NOT VALID to prevent locks while adding the column on large existing tables, -- which will be validated in a later migration. For all new/updated rows the FKEY will be checked. diff --git a/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.sqlite b/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.sqlite new file mode 100644 index 000000000000..6f6283fdb769 --- /dev/null +++ b/synapse/storage/schema/main/delta/74/03_membership_tables_event_stream_ordering.sql.sqlite @@ -0,0 +1,23 @@ +/* Copyright 2022 Beeper + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- Each of these are denormalised copies of `stream_ordering` from the corresponding row in` events` which +-- we use to improve database performance by reduring JOINs. + +-- NOTE: sqlite does not support ADD CONSTRAINT so we add the new columns with FK constraint as-is + +ALTER TABLE current_state_events ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); +ALTER TABLE local_current_membership ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering); +ALTER TABLE room_memberships ADD COLUMN event_stream_ordering BIGINT REFERENCES events(stream_ordering);