From 06953bc193530780a7686b2aee9632a4ed1d604f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 6 Jun 2024 17:10:58 +0100 Subject: [PATCH 1/5] Always return OTK counts (#17275) Broke in https://github.com/element-hq/synapse/pull/17215 --- changelog.d/17275.bugfix | 1 + synapse/handlers/sync.py | 33 +++++++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 changelog.d/17275.bugfix diff --git a/changelog.d/17275.bugfix b/changelog.d/17275.bugfix new file mode 100644 index 000000000..eb522bb99 --- /dev/null +++ b/changelog.d/17275.bugfix @@ -0,0 +1 @@ +Fix bug where OTKs were not always included in `/sync` response when using workers. diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 1d7d9dfdd..6389c51b1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -285,7 +285,11 @@ class SyncResult: ) @staticmethod - def empty(next_batch: StreamToken) -> "SyncResult": + def empty( + next_batch: StreamToken, + device_one_time_keys_count: JsonMapping, + device_unused_fallback_key_types: List[str], + ) -> "SyncResult": "Return a new empty result" return SyncResult( next_batch=next_batch, @@ -297,8 +301,8 @@ class SyncResult: archived=[], to_device=[], device_lists=DeviceListUpdates(), - device_one_time_keys_count={}, - device_unused_fallback_key_types=[], + device_one_time_keys_count=device_one_time_keys_count, + device_unused_fallback_key_types=device_unused_fallback_key_types, ) @@ -523,7 +527,28 @@ class SyncHandler: logger.warning( "Timed out waiting for worker to catch up. Returning empty response" ) - return SyncResult.empty(since_token) + device_id = sync_config.device_id + one_time_keys_count: JsonMapping = {} + unused_fallback_key_types: List[str] = [] + if device_id: + user_id = sync_config.user.to_string() + # TODO: We should have a way to let clients differentiate between the states of: + # * no change in OTK count since the provided since token + # * the server has zero OTKs left for this device + # Spec issue: https://github.com/matrix-org/matrix-doc/issues/3298 + one_time_keys_count = await self.store.count_e2e_one_time_keys( + user_id, device_id + ) + unused_fallback_key_types = list( + await self.store.get_e2e_unused_fallback_key_types( + user_id, device_id + ) + ) + + cache_context.should_cache = False # Don't cache empty responses + return SyncResult.empty( + since_token, one_time_keys_count, unused_fallback_key_types + ) # If we've spent significant time waiting to catch up, take it off # the timeout. From b84e31375b0ca80970a6c56e3e0e86a5c63af025 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 10 Jun 2024 15:55:42 +0100 Subject: [PATCH 2/5] Update changelog --- changelog.d/17275.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17275.bugfix b/changelog.d/17275.bugfix index eb522bb99..04e8ab5ef 100644 --- a/changelog.d/17275.bugfix +++ b/changelog.d/17275.bugfix @@ -1 +1 @@ -Fix bug where OTKs were not always included in `/sync` response when using workers. +Fix bug where OTKs were not always included in `/sync` response when using workers. Introduced v1.109.0rc1. From 8c4937b216309eec62f9262f23162878fa3b772c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 10 Jun 2024 15:56:57 +0100 Subject: [PATCH 3/5] Fix bug where device lists would break sync (#17292) 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 https://github.com/element-hq/synapse/pull/17229 --- changelog.d/17292.bugfix | 1 + synapse/storage/databases/main/devices.py | 28 +++++++++++++------ .../05_add_instance_names_converted_pos.sql | 16 +++++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 changelog.d/17292.bugfix create mode 100644 synapse/storage/schema/main/delta/85/05_add_instance_names_converted_pos.sql diff --git a/changelog.d/17292.bugfix b/changelog.d/17292.bugfix new file mode 100644 index 000000000..c067a98ce --- /dev/null +++ b/changelog.d/17292.bugfix @@ -0,0 +1 @@ +Fix bug where `/sync` could get stuck due to edge case in device lists handling. Introduced in v1.109.0rc1. diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index 1c771e48f..40187496e 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -108,6 +108,11 @@ class DeviceWorkerStore(RoomMemberWorkerStore, EndToEndKeyWorkerStore): ("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"], @@ -2394,15 +2399,16 @@ class DeviceStore(DeviceWorkerStore, DeviceBackgroundUpdateStore): `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, @@ -2417,6 +2423,10 @@ class DeviceStore(DeviceWorkerStore, DeviceBackgroundUpdateStore): 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", ) diff --git a/synapse/storage/schema/main/delta/85/05_add_instance_names_converted_pos.sql b/synapse/storage/schema/main/delta/85/05_add_instance_names_converted_pos.sql new file mode 100644 index 000000000..c3f2b6a1d --- /dev/null +++ b/synapse/storage/schema/main/delta/85/05_add_instance_names_converted_pos.sql @@ -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: +-- . + +-- 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; From a8069e9739fdef8c7200d0d33a38b85100398136 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 11 Jun 2024 15:22:21 +0200 Subject: [PATCH 4/5] 1.109.0rc2 --- CHANGES.md | 10 ++++++++++ changelog.d/17275.bugfix | 1 - changelog.d/17292.bugfix | 1 - debian/changelog | 6 ++++++ pyproject.toml | 2 +- 5 files changed, 17 insertions(+), 3 deletions(-) delete mode 100644 changelog.d/17275.bugfix delete mode 100644 changelog.d/17292.bugfix diff --git a/CHANGES.md b/CHANGES.md index 092dbdbf2..3116e1b2a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,13 @@ +# Synapse 1.109.0rc2 (2024-06-11) + +### Bugfixes + +- Fix bug where OTKs were not always included in `/sync` response when using workers. Introduced in v1.109.0rc1. ([\#17275](https://github.com/element-hq/synapse/issues/17275)) +- Fix bug where `/sync` could get stuck due to edge case in device lists handling. Introduced in v1.109.0rc1. ([\#17292](https://github.com/element-hq/synapse/issues/17292)) + + + + # Synapse 1.109.0rc1 (2024-06-04) ### Features diff --git a/changelog.d/17275.bugfix b/changelog.d/17275.bugfix deleted file mode 100644 index 04e8ab5ef..000000000 --- a/changelog.d/17275.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix bug where OTKs were not always included in `/sync` response when using workers. Introduced v1.109.0rc1. diff --git a/changelog.d/17292.bugfix b/changelog.d/17292.bugfix deleted file mode 100644 index c067a98ce..000000000 --- a/changelog.d/17292.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix bug where `/sync` could get stuck due to edge case in device lists handling. Introduced in v1.109.0rc1. diff --git a/debian/changelog b/debian/changelog index 927248bda..ac2536749 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (1.109.0~rc2) stable; urgency=medium + + * New synapse release 1.109.0rc2. + + -- Synapse Packaging team Tue, 11 Jun 2024 13:20:17 +0000 + matrix-synapse-py3 (1.109.0~rc1) stable; urgency=medium * New Synapse release 1.109.0rc1. diff --git a/pyproject.toml b/pyproject.toml index 8cc99b8cb..f4f7f7060 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,7 +96,7 @@ module-name = "synapse.synapse_rust" [tool.poetry] name = "matrix-synapse" -version = "1.109.0rc1" +version = "1.109.0rc2" description = "Homeserver for the Matrix decentralised comms protocol" authors = ["Matrix.org Team and Contributors "] license = "AGPL-3.0-or-later" From e6816babf6ce37cca4fbd9d67e5b2a0b06f65d1a Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 11 Jun 2024 15:39:30 +0200 Subject: [PATCH 5/5] CHANGES.md: s/OTKs/one-time-keys/ --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 3116e1b2a..61c6170c6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,7 +2,7 @@ ### Bugfixes -- Fix bug where OTKs were not always included in `/sync` response when using workers. Introduced in v1.109.0rc1. ([\#17275](https://github.com/element-hq/synapse/issues/17275)) +- Fix bug where one-time-keys were not always included in `/sync` response when using workers. Introduced in v1.109.0rc1. ([\#17275](https://github.com/element-hq/synapse/issues/17275)) - Fix bug where `/sync` could get stuck due to edge case in device lists handling. Introduced in v1.109.0rc1. ([\#17292](https://github.com/element-hq/synapse/issues/17292))