Apply review nits: hoist bg-jobs check, drop redundant intersection, omit total_rooms when unknown, conn_id caveat, drop stray .gitignore hunk

Also adds a test for the aging-lane fairness path.

.gitignore

@@ -81,4 +81,3 @@ book/
 
 # Don't include users' poetry configs
 /poetry.toml
-/localtest/

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(

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
 

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,
         )

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."""