diff --git a/changelog.d/18794.misc b/changelog.d/18794.misc new file mode 100644 index 0000000000..b01a686633 --- /dev/null +++ b/changelog.d/18794.misc @@ -0,0 +1 @@ +Update tests to ensure all database tables are emptied when purging a room. diff --git a/synapse/storage/databases/main/purge_events.py b/synapse/storage/databases/main/purge_events.py index a11f522f03..d4642a1309 100644 --- a/synapse/storage/databases/main/purge_events.py +++ b/synapse/storage/databases/main/purge_events.py @@ -33,6 +33,73 @@ from synapse.types import RoomStreamToken logger = logging.getLogger(__name__) +purge_room_tables_with_event_id_index = ( + "event_auth", + "event_edges", + "event_json", + "event_push_actions_staging", + "event_relations", + "event_to_state_groups", + "event_auth_chains", + "event_auth_chain_to_calculate", + "redactions", + "rejections", + "state_events", +) +""" +Tables which lack an index on `room_id` but have one on `event_id` +""" + +purge_room_tables_with_room_id_column = ( + "current_state_events", + "destination_rooms", + "event_backward_extremities", + "event_forward_extremities", + "event_push_actions", + "event_search", + "event_failed_pull_attempts", + # Note: the partial state tables have foreign keys between each other, and to + # `events` and `rooms`. We need to delete from them in the right order. + "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", + # Note: the sliding_sync_ tables have foreign keys to the `events` table + # so must be deleted first. + "sliding_sync_joined_rooms", + "sliding_sync_membership_snapshots", + "events", + "federation_inbound_events_staging", + "receipts_graph", + "receipts_linearized", + "room_aliases", + "room_depth", + "room_stats_state", + "room_stats_current", + "room_stats_earliest_token", + "stream_ordering_to_exterm", + "users_in_public_rooms", + "users_who_share_private_rooms", + # no useful index, but let's clear them anyway + "appservice_room_list", + "e2e_room_keys", + "event_push_summary", + "pusher_throttle", + "room_account_data", + "room_tags", + # "rooms" happens last, to keep the foreign keys in the other tables + # happy + "rooms", +) +""" +The tables with a `room_id` column regardless of whether they have a useful index on +`room_id`. +""" + + class PurgeEventsStore(StateGroupWorkerStore, CacheInvalidationWorkerStore): async def purge_history( self, room_id: str, token: str, delete_local_events: bool @@ -398,20 +465,8 @@ class PurgeEventsStore(StateGroupWorkerStore, CacheInvalidationWorkerStore): referenced_chain_id_tuples, ) - # Now we delete tables which lack an index on room_id but have one on event_id - for table in ( - "event_auth", - "event_edges", - "event_json", - "event_push_actions_staging", - "event_relations", - "event_to_state_groups", - "event_auth_chains", - "event_auth_chain_to_calculate", - "redactions", - "rejections", - "state_events", - ): + # Now we delete tables which lack an index on `room_id` but have one on `event_id` + for table in purge_room_tables_with_event_id_index: logger.info("[purge] removing from %s", table) txn.execute( @@ -424,51 +479,9 @@ class PurgeEventsStore(StateGroupWorkerStore, CacheInvalidationWorkerStore): (room_id,), ) - # next, the tables with an index on room_id (or no useful index) - for table in ( - "current_state_events", - "destination_rooms", - "event_backward_extremities", - "event_forward_extremities", - "event_push_actions", - "event_search", - "event_failed_pull_attempts", - # Note: the partial state tables have foreign keys between each other, and to - # `events` and `rooms`. We need to delete from them in the right order. - "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", - # Note: the sliding_sync_ tables have foreign keys to the `events` table - # so must be deleted first. - "sliding_sync_joined_rooms", - "sliding_sync_membership_snapshots", - "events", - "federation_inbound_events_staging", - "receipts_graph", - "receipts_linearized", - "room_aliases", - "room_depth", - "room_stats_state", - "room_stats_current", - "room_stats_earliest_token", - "stream_ordering_to_exterm", - "users_in_public_rooms", - "users_who_share_private_rooms", - # no useful index, but let's clear them anyway - "appservice_room_list", - "e2e_room_keys", - "event_push_summary", - "pusher_throttle", - "room_account_data", - "room_tags", - # "rooms" happens last, to keep the foreign keys in the other tables - # happy - "rooms", - ): + # next, the tables with a `room_id` column regardless of whether they have a + # useful index on `room_id` + for table in purge_room_tables_with_room_id_column: logger.info("[purge] removing from %s", table) txn.execute("DELETE FROM %s WHERE room_id=?" % (table,), (room_id,)) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 6454f18571..b98c53891c 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -40,6 +40,10 @@ from synapse.handlers.pagination import ( ) from synapse.rest.client import directory, events, knock, login, room, sync from synapse.server import HomeServer +from synapse.storage.databases.main.purge_events import ( + purge_room_tables_with_event_id_index, + purge_room_tables_with_room_id_column, +) from synapse.types import UserID from synapse.util import Clock from synapse.util.task_scheduler import TaskScheduler @@ -547,7 +551,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): def _is_purged(self, room_id: str) -> None: """Test that the following tables have been purged of all rows related to the room.""" - for table in PURGE_TABLES: + for table in purge_room_tables_with_room_id_column: count = self.get_success( self.store.db_pool.simple_select_one_onecol( table=table, @@ -556,7 +560,21 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): desc="test_purge_room", ) ) + self.assertEqual(count, 0, msg=f"Rows not purged in {table}") + for table in purge_room_tables_with_event_id_index: + rows = self.get_success( + self.store.db_pool.execute( + "find_event_count_for_table", + f""" + SELECT COUNT(*) FROM {table} WHERE event_id IN ( + SELECT event_id FROM events WHERE room_id=? + ) + """, + room_id, + ) + ) + count = rows[0][0] self.assertEqual(count, 0, msg=f"Rows not purged in {table}") def _assert_peek(self, room_id: str, expect_code: int) -> None: @@ -1229,7 +1247,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): def _is_purged(self, room_id: str) -> None: """Test that the following tables have been purged of all rows related to the room.""" - for table in PURGE_TABLES: + for table in purge_room_tables_with_room_id_column: count = self.get_success( self.store.db_pool.simple_select_one_onecol( table=table, @@ -1238,7 +1256,21 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase): desc="test_purge_room", ) ) + self.assertEqual(count, 0, msg=f"Rows not purged in {table}") + for table in purge_room_tables_with_event_id_index: + rows = self.get_success( + self.store.db_pool.execute( + "find_event_count_for_table", + f""" + SELECT COUNT(*) FROM {table} WHERE event_id IN ( + SELECT event_id FROM events WHERE room_id=? + ) + """, + room_id, + ) + ) + count = rows[0][0] self.assertEqual(count, 0, msg=f"Rows not purged in {table}") def _assert_peek(self, room_id: str, expect_code: int) -> None: @@ -3177,35 +3209,3 @@ class BlockRoomTestCase(unittest.HomeserverTestCase): """Block a room in database""" self.get_success(self._store.block_room(room_id, self.other_user)) self._is_blocked(room_id, expect=True) - - -PURGE_TABLES = [ - "current_state_events", - "event_backward_extremities", - "event_forward_extremities", - "event_json", - "event_push_actions", - "event_search", - "events", - "receipts_graph", - "receipts_linearized", - "room_aliases", - "room_depth", - "room_memberships", - "room_stats_state", - "room_stats_current", - "room_stats_earliest_token", - "rooms", - "stream_ordering_to_exterm", - "users_in_public_rooms", - "users_who_share_private_rooms", - "appservice_room_list", - "e2e_room_keys", - "event_push_summary", - "pusher_throttle", - "room_account_data", - "room_tags", - "state_groups", - "state_groups_state", - "federation_inbound_events_staging", -]