diff --git a/.gitignore b/.gitignore index bb204dcfe1..1e2937230c 100644 --- a/.gitignore +++ b/.gitignore @@ -81,4 +81,3 @@ book/ # Don't include users' poetry configs /poetry.toml -/localtest/ diff --git a/synapse/handlers/sliding_sync/paginated.py b/synapse/handlers/sliding_sync/paginated.py index 176ec5ff4d..cb58c8e7eb 100644 --- a/synapse/handlers/sliding_sync/paginated.py +++ b/synapse/handlers/sliding_sync/paginated.py @@ -204,11 +204,15 @@ class PaginatedSyncHandler(SlidingSyncHandler): from_token = None previous_connection_state = PerConnectionState(last_used_ts=None) + # Whether the new sliding sync tables are usable (c.f. SlidingSyncBase); + # needed both here and for update detection below. + use_new_tables = await self.store.have_finished_sliding_sync_background_jobs() + # Reuse the sliding sync membership machinery wholesale: with no lists # and no subscriptions it assembles the full membership map (with # rewinds, newly-left add-back and state-reset handling) and the # newly-joined/newly-left/DM sets, without computing any list windows. - if await self.store.have_finished_sliding_sync_background_jobs(): + if use_new_tables: interested_rooms = ( await self.room_lists._compute_interested_rooms_new_tables( sync_config=sync_config, # type: ignore[arg-type] @@ -268,7 +272,7 @@ class PaginatedSyncHandler(SlidingSyncHandler): else: live_room_ids.append(room_id) - if await self.store.have_finished_sliding_sync_background_jobs(): + if use_new_tables: updated_room_ids = await ( self.store.get_rooms_that_have_updates_since_sliding_sync_table( room_ids=live_room_ids, @@ -361,7 +365,7 @@ class PaginatedSyncHandler(SlidingSyncHandler): return room_status.last_token.stream aged_room_ids = sorted( - previously_room_ids & candidates, key=last_sent_stream_pos + previously_room_ids, key=last_sent_stream_pos )[:aging_lane_size] sorted_room_infos = await self.room_lists.sort_rooms( diff --git a/synapse/rest/client/paginated_sync.py b/synapse/rest/client/paginated_sync.py index 5da0972067..1f403e36f2 100644 --- a/synapse/rest/client/paginated_sync.py +++ b/synapse/rest/client/paginated_sync.py @@ -102,9 +102,12 @@ class PaginatedSyncRestServlet(SlidingSyncRestServlet): sync_config = PaginatedSyncConfig( user=user, requester=requester, - # Namespace the connection ID so a paginated sync connection can - # never collide with a sliding sync connection from the same device - # in the shared per-connection tables. + # Namespace the connection ID so a paginated sync connection + # doesn't collide with a sliding sync connection from the same + # device in the shared per-connection tables. (A sliding sync + # client that literally sends `conn_id: "paginated:foo"` would + # still collide - self-inflicted and same-device-only, so we + # accept it.) conn_id=f"paginated:{body.conn_id or ''}", page_size=body.page_size, limit=body.limit, @@ -155,7 +158,8 @@ class PaginatedSyncRestServlet(SlidingSyncRestServlet): ) if result.pending: response["pending"] = result.pending - response["total_rooms"] = result.total_rooms + if result.total_rooms is not None: + response["total_rooms"] = result.total_rooms return response diff --git a/synapse/types/handlers/paginated_sync.py b/synapse/types/handlers/paginated_sync.py index 4118e1c4b3..95a712d826 100644 --- a/synapse/types/handlers/paginated_sync.py +++ b/synapse/types/handlers/paginated_sync.py @@ -64,14 +64,16 @@ class PaginatedSyncResult: not fit into `page_size`. While non-zero the client should sync again immediately to drain the backlog. total_rooms: The total number of rooms in the user's account, for - cold-start progress reporting. + cold-start progress reporting. `None` (omitted from the response) + when the room set wasn't computed, e.g. the worker-catch-up + timeout path. """ next_pos: SlidingSyncStreamToken rooms: dict[str, SlidingSyncResult.RoomResult] extensions: SlidingSyncResult.Extensions pending: int - total_rooms: int + total_rooms: int | None def __bool__(self) -> bool: """Whether there are any updates that should be returned immediately to @@ -90,5 +92,5 @@ class PaginatedSyncResult: rooms={}, extensions=SlidingSyncResult.Extensions(), pending=0, - total_rooms=0, + total_rooms=None, ) diff --git a/tests/rest/client/sliding_sync/test_paginated_sync.py b/tests/rest/client/sliding_sync/test_paginated_sync.py index d611f5a6c2..a7f6cfa3cf 100644 --- a/tests/rest/client/sliding_sync/test_paginated_sync.py +++ b/tests/rest/client/sliding_sync/test_paginated_sync.py @@ -208,6 +208,36 @@ class PaginatedSyncTestCase(unittest.HomeserverTestCase): room_id, ) + def test_aging_lane_prevents_starvation(self) -> None: + """When more rooms have updates than fit in the page, part of the page + is reserved for the longest-deferred rooms, so busier rooms can't + starve a quiet room's update indefinitely.""" + room_ids = self._create_rooms(4) + body = {"page_size": 10, "limit": 5, "history": 1} + response = self._sync(body) + pos = response["pos"] + + # Three rooms update; a page of 2 defers the least recently active. + for room_id in room_ids[:3]: + self.helper.send(room_id, body="update", tok=self.tok) + small_body = {"page_size": 2, "limit": 5, "history": 1} + response = self._sync(small_body, pos=pos) + self.assertEqual( + set(response["rooms"]), {room_ids[1], room_ids[2]}, response["rooms"] + ) + self.assertEqual(response["pending"], 1) + pos = response["pos"] + + # The busier rooms re-earn their place at the top; the deferred room + # (room 0) stays the least recently active, but the aging lane + # guarantees it a slot in the page anyway. + for room_id in room_ids[1:]: + self.helper.send(room_id, body="busy", tok=self.tok) + response = self._sync(small_body, pos=pos) + self.assertIn(room_ids[0], response["rooms"], response["rooms"].keys()) + self.assertEqual(len(response["rooms"]), 2) + self.assertEqual(response["pending"], 2) + def test_newly_joined_room_uses_history(self) -> None: """A room that appears mid-session (never sent on the connection) comes down `initial` with `history` events."""