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

Use READ COMMITTED isolation level when purging rooms #12942

Merged
merged 12 commits into from
Jul 18, 2022
Merged

Use READ COMMITTED isolation level when purging rooms #12942

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cbeefd9
Use READ COMITTED isolation level when purging rooms
Fizzadar Jun 1, 2022
da89a5b
Add changelog file
Fizzadar Jun 1, 2022
8a1c5b7
Linting fix import order
Fizzadar Jun 1, 2022
b9cf39e
Add contribution info to changelog
Fizzadar Jun 4, 2022
6e7a288
Add comment about isolation level in purge
Fizzadar Jul 8, 2022
fc32d56
Add select for update at start of room purge txn
Fizzadar Jul 8, 2022
d5af031
Only do select for update query on purge with postgres
Fizzadar Jul 11, 2022
cedbe2a
Delete rooms using two transactions
Fizzadar Jul 13, 2022
c83db92
Simplify purge room by just re-running the same txn
Fizzadar Jul 16, 2022
110ad87
Put back missing table deletes
Fizzadar Jul 16, 2022
f895ace
Include any state groups from the second purge txn
Fizzadar Jul 18, 2022
78d5c10
Merge branch 'develop' into purge-room-isolation-level
erikjohnston Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions synapse/storage/databases/main/purge_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,31 @@ async def purge_room(self, room_id: str) -> List[int]:
Returns:
The list of state groups to delete.
"""
return await self.db_pool.runInteraction(
state_groups_to_delete = await self.db_pool.runInteraction(
"purge_room",
self._purge_room_txn,
room_id=room_id,
# This is safe because we don't care if room data is updated during the transaction
# This is safe because we don't care if room data is updated during the transaction, note
# we run a second transaction to cleanup tables we may write to during this transaction.
isolation_level=IsolationLevel.READ_COMMITTED,
Fizzadar marked this conversation as resolved.
Show resolved Hide resolved
)

def _purge_room_second_pass_txn(txn: LoggingTransaction, room_id: str) -> None:
for table in (
"event_push_actions",
"stream_ordering_to_exterm",
Copy link
Member

Choose a reason for hiding this comment

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

Are these the only one's we care about? I think it might just be easier to re-run the same _purge_room_txn again a second time, as it should be quick if we've deleted the majority of stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦, so much simpler! c83db92

):
logger.info("[purge] removing %s from %s", room_id, table)
txn.execute("DELETE FROM %s WHERE room_id=?" % (table,), (room_id,))

await self.db_pool.runInteraction(
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to add the return value here to state_groups_to_delete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot! f895ace

"purge_room_second_pass",
_purge_room_second_pass_txn,
room_id=room_id,
)

return state_groups_to_delete

def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]:
Fizzadar marked this conversation as resolved.
Show resolved Hide resolved
# This collides with event persistence so we cannot write new events and metadata into
# a room while deleting it or this transaction will fail.
Expand Down Expand Up @@ -402,7 +419,6 @@ def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]:
"destination_rooms",
"event_backward_extremities",
"event_forward_extremities",
"event_push_actions",
"event_search",
"partial_state_events",
"events",
Expand All @@ -419,7 +435,6 @@ def _purge_room_txn(self, txn: LoggingTransaction, room_id: str) -> List[int]:
"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
Expand Down