Skip to content

Commit

Permalink
Fix bug where device lists would break sync (#17292)
Browse files Browse the repository at this point in the history
If the stream ID in the unconverted table is ahead of the device lists
ID gen, then it can break all /sync requests that had an ID from ahead
of the table.

The fix is to make sure we add the unconverted table to the list of
tables we check at start up.

Broke in #17229
  • Loading branch information
erikjohnston authored Jun 10, 2024
1 parent b84e313 commit 8c4937b
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog.d/17292.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where `/sync` could get stuck due to edge case in device lists handling. Introduced in v1.109.0rc1.
28 changes: 19 additions & 9 deletions synapse/storage/databases/main/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ def __init__(
("device_lists_outbound_pokes", "instance_name", "stream_id"),
("device_lists_changes_in_room", "instance_name", "stream_id"),
("device_lists_remote_pending", "instance_name", "stream_id"),
(
"device_lists_changes_converted_stream_position",
"instance_name",
"stream_id",
),
],
sequence_name="device_lists_sequence",
writers=["master"],
Expand Down Expand Up @@ -2394,15 +2399,16 @@ async def get_device_change_last_converted_pos(self) -> Tuple[int, str]:
`FALSE` have not been converted.
"""

return cast(
Tuple[int, str],
await self.db_pool.simple_select_one(
table="device_lists_changes_converted_stream_position",
keyvalues={},
retcols=["stream_id", "room_id"],
desc="get_device_change_last_converted_pos",
),
# There should be only one row in this table, though we want to
# future-proof ourselves for when we have multiple rows (one for each
# instance). So to handle that case we take the minimum of all rows.
rows = await self.db_pool.simple_select_list(
table="device_lists_changes_converted_stream_position",
keyvalues={},
retcols=["stream_id", "room_id"],
desc="get_device_change_last_converted_pos",
)
return cast(Tuple[int, str], min(rows))

async def set_device_change_last_converted_pos(
self,
Expand All @@ -2417,6 +2423,10 @@ async def set_device_change_last_converted_pos(
await self.db_pool.simple_update_one(
table="device_lists_changes_converted_stream_position",
keyvalues={},
updatevalues={"stream_id": stream_id, "room_id": room_id},
updatevalues={
"stream_id": stream_id,
"instance_name": self._instance_name,
"room_id": room_id,
},
desc="set_device_change_last_converted_pos",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--
-- This file is licensed under the Affero General Public License (AGPL) version 3.
--
-- Copyright (C) 2024 New Vector, Ltd
--
-- This program is free software: you can redistribute it and/or modify
-- it under the terms of the GNU Affero General Public License as
-- published by the Free Software Foundation, either version 3 of the
-- License, or (at your option) any later version.
--
-- See the GNU Affero General Public License for more details:
-- <https://www.gnu.org/licenses/agpl-3.0.html>.

-- Add `instance_name` columns to stream tables to allow them to be used with
-- `MultiWriterIdGenerator`
ALTER TABLE device_lists_changes_converted_stream_position ADD COLUMN instance_name TEXT;

0 comments on commit 8c4937b

Please sign in to comment.