From d201b45d512490cbe9768de37733e3808dde4f4e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 27 Apr 2026 10:26:52 +0100 Subject: [PATCH 01/11] Add note about using 'now_token' --- synapse/handlers/sliding_sync/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index a3443b300cc..16497a3bd4f 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -196,6 +196,12 @@ async def current_sync_callback( sync_config.user.to_string(), timeout_ms, current_sync_callback, + # We *wait* from `now_token` as we have already computed the sync + # response up to `now_token` above, so we want to wait for something + # new to arrive after `now_token`. + # + # We still generate the sync response using `from_token` in the + # callback above though. from_token=now_token, ) did_wait = True From b580c1c7f21640c6f5ffd298b99bb397d3b07dc9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 27 Apr 2026 10:31:01 +0100 Subject: [PATCH 02/11] Add test that required state is immediately returned --- .../sliding_sync/test_rooms_required_state.py | 66 +++++++++++++++++++ .../client/sliding_sync/test_sliding_sync.py | 35 ++++++++-- 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index 586b127f8a1..a9895987381 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -2245,3 +2245,69 @@ def test_lazy_loading_room_members_state_reset_non_limited_timeline(self) -> Non response_body["rooms"][room_id]["required_state"][0]["event_id"], first_event_id, ) + + def test_changing_required_state_returns_immediately(self) -> None: + """Test that if we change the required state, then we return immediately + with the new required state.""" + + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Make an initial sync request with no required state + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 1]], + "required_state": [], + "timeline_limit": 0, + } + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + + # We should see no required state + self.assertIsNone(response_body["rooms"][room_id1].get("required_state")) + + # Get the state_map before we change the state as this is the final state we + # expect to see when we update the required state. + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + + # There is no new data, and so making another sync request will block. + channel = self.make_sync_request( + sync_body, + since=from_token, + tok=user1_tok, + timeout_ms=10_000, + await_result=False, + ) + self.reactor.advance(0.1) # Allow the request to start processing + self.reactor.advance(9.5) + self.assertFalse(channel.is_finished()) + + # Advance past the timeout to make sure the request finishes. (We do this + # to ensure log contexts don't leak between tests). + self.reactor.advance(1) + self.assertTrue(channel.is_finished()) + + # Now update the sliding sync requests to include a required state + # event, and make another sync request. + sync_body["lists"]["foo-list"]["required_state"] = [ + [EventTypes.Create, ""], + ] + + response_body, _ = self.do_sync( + sync_body, since=from_token, tok=user1_tok, timeout_ms=10_000 + ) + + # We should see the new required state immediately without waiting. + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index ac8dfd37d8d..2b258ecb061 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -12,6 +12,7 @@ # . # import logging +import urllib.parse from typing import Any, Iterable, Literal from unittest.mock import AsyncMock @@ -81,7 +82,13 @@ def default_config(self) -> JsonDict: return config def make_sync_request( - self, sync_body: JsonDict, *, since: str | None = None, tok: str + self, + sync_body: JsonDict, + *, + since: str | None = None, + tok: str, + timeout_ms: int | None = None, + await_result: bool = True, ) -> FakeChannel: """Make a sliding sync request with given body. @@ -89,25 +96,40 @@ def make_sync_request( sync_body: The full request body to use since: Optional since token tok: Access token to use - + timeout_ms: Optional timeout in milliseconds to use for the request. + await_result: Whether to block and wait for the result before returning. Returns: A tuple of the response body and the `pos` field. """ sync_path = self.sync_endpoint + + query_params: dict[str, Any] = {} if since: - sync_path += f"?pos={since}" + query_params["pos"] = since + if timeout_ms is not None: + query_params["timeout"] = timeout_ms + + if query_params: + query_str = urllib.parse.urlencode(query_params) + sync_path += f"?{query_str}" channel = self.make_request( method="POST", path=sync_path, content=sync_body, access_token=tok, + await_result=await_result, ) return channel def do_sync( - self, sync_body: JsonDict, *, since: str | None = None, tok: str + self, + sync_body: JsonDict, + *, + since: str | None = None, + tok: str, + timeout_ms: int | None = None, ) -> tuple[JsonDict, str]: """Do a sliding sync request with given body. @@ -117,11 +139,14 @@ def do_sync( sync_body: The full request body to use since: Optional since token tok: Access token to use + timeout_ms: Optional timeout in milliseconds to use for the request. Returns: A tuple of the response body and the `pos` field. """ - channel = self.make_sync_request(sync_body, since=since, tok=tok) + channel = self.make_sync_request( + sync_body, since=since, tok=tok, timeout_ms=timeout_ms + ) self.assertEqual(channel.code, 200, channel.json_body) return channel.json_body, channel.json_body["pos"] From f86094cad46f8f0ee80a445bb77ad874a1731e03 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 27 Apr 2026 10:33:00 +0100 Subject: [PATCH 03/11] Newsfile --- changelog.d/19734.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19734.bugfix diff --git a/changelog.d/19734.bugfix b/changelog.d/19734.bugfix new file mode 100644 index 00000000000..6aba7b21a61 --- /dev/null +++ b/changelog.d/19734.bugfix @@ -0,0 +1 @@ +Have SSS return a new response immediately if a room subscription have changed and produced a new response. From 812fcbeb253413769bf5745deb559ef3314703fb Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Apr 2026 13:10:35 +0100 Subject: [PATCH 04/11] Apply suggestions from code review Co-authored-by: Eric Eastwood --- synapse/handlers/sliding_sync/__init__.py | 6 +++--- .../client/sliding_sync/test_rooms_required_state.py | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 16497a3bd4f..60670c37367 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -196,9 +196,9 @@ async def current_sync_callback( sync_config.user.to_string(), timeout_ms, current_sync_callback, - # We *wait* from `now_token` as we have already computed the sync - # response up to `now_token` above, so we want to wait for something - # new to arrive after `now_token`. + # We *wait* from `now_token` as we have already computed the sync response + # up to `now_token` above, so as a minor optimization, we can wait for + # something new to arrive after `now_token`. # # We still generate the sync response using `from_token` in the # callback above though. diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index a9895987381..b2bc097f968 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -2247,8 +2247,8 @@ def test_lazy_loading_room_members_state_reset_non_limited_timeline(self) -> Non ) def test_changing_required_state_returns_immediately(self) -> None: - """Test that if we change the required state, then we return immediately - with the new required state.""" + """Test that if we change the `required_state`, then we return immediately + with the new `required_state`.""" user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") @@ -2281,7 +2281,7 @@ def test_changing_required_state_returns_immediately(self) -> None: sync_body, since=from_token, tok=user1_tok, - timeout_ms=10_000, + timeout_ms=Duration(seconds=10).as_millis(), await_result=False, ) self.reactor.advance(0.1) # Allow the request to start processing @@ -2293,7 +2293,7 @@ def test_changing_required_state_returns_immediately(self) -> None: self.reactor.advance(1) self.assertTrue(channel.is_finished()) - # Now update the sliding sync requests to include a required state + # Now update the Sliding Sync requests to include a `required_state` # event, and make another sync request. sync_body["lists"]["foo-list"]["required_state"] = [ [EventTypes.Create, ""], @@ -2303,7 +2303,7 @@ def test_changing_required_state_returns_immediately(self) -> None: sync_body, since=from_token, tok=user1_tok, timeout_ms=10_000 ) - # We should see the new required state immediately without waiting. + # We should see the new `required_state` immediately without waiting. self._assertRequiredStateIncludes( response_body["rooms"][room_id1]["required_state"], { From c8e6e5da889a46b82a0e6105dde54887a8743e8b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Apr 2026 13:11:34 +0100 Subject: [PATCH 05/11] Update newsfile --- changelog.d/19714.bugfix | 2 +- changelog.d/19734.bugfix | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog.d/19714.bugfix b/changelog.d/19714.bugfix index 6aba7b21a61..01af7d9ab83 100644 --- a/changelog.d/19714.bugfix +++ b/changelog.d/19714.bugfix @@ -1 +1 @@ -Have SSS return a new response immediately if a room subscription have changed and produced a new response. +Update Sliding Sync to return a new response immediately if a room subscription have changed and produced a new response. diff --git a/changelog.d/19734.bugfix b/changelog.d/19734.bugfix index 6aba7b21a61..01af7d9ab83 100644 --- a/changelog.d/19734.bugfix +++ b/changelog.d/19734.bugfix @@ -1 +1 @@ -Have SSS return a new response immediately if a room subscription have changed and produced a new response. +Update Sliding Sync to return a new response immediately if a room subscription have changed and produced a new response. From 6d83c1f7b84c04d08f4f6b758e9dc933b10b7d0a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Apr 2026 13:16:55 +0100 Subject: [PATCH 06/11] Clarify why we need to use from_token --- synapse/handlers/sliding_sync/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 60670c37367..7878274ed21 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -196,12 +196,13 @@ async def current_sync_callback( sync_config.user.to_string(), timeout_ms, current_sync_callback, - # We *wait* from `now_token` as we have already computed the sync response - # up to `now_token` above, so as a minor optimization, we can wait for - # something new to arrive after `now_token`. + # We *wait* from `now_token` as we have already computed the sync + # response up to `now_token` above, so as a minor optimization, we + # can wait for something new to arrive after `now_token`. # # We still generate the sync response using `from_token` in the - # callback above though. + # callback above though, as to generate the correct response it + # needs to know the "real" `from_token`. from_token=now_token, ) did_wait = True From 39b0b68d431541e11ed8b1ac9eae696d0e51ba31 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Apr 2026 13:27:09 +0100 Subject: [PATCH 07/11] Use Duration in tests --- .../sliding_sync/test_rooms_required_state.py | 5 +++-- tests/rest/client/sliding_sync/test_sliding_sync.py | 13 +++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index b2bc097f968..b167d47d380 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -25,6 +25,7 @@ from synapse.server import HomeServer from synapse.storage.databases.main.events import DeltaState, SlidingSyncTableChanges from synapse.util.clock import Clock +from synapse.util.duration import Duration from tests.rest.client.sliding_sync.test_sliding_sync import SlidingSyncBase from tests.test_utils.event_injection import mark_event_as_partial_state @@ -2281,7 +2282,7 @@ def test_changing_required_state_returns_immediately(self) -> None: sync_body, since=from_token, tok=user1_tok, - timeout_ms=Duration(seconds=10).as_millis(), + timeout=Duration(seconds=10), await_result=False, ) self.reactor.advance(0.1) # Allow the request to start processing @@ -2300,7 +2301,7 @@ def test_changing_required_state_returns_immediately(self) -> None: ] response_body, _ = self.do_sync( - sync_body, since=from_token, tok=user1_tok, timeout_ms=10_000 + sync_body, since=from_token, tok=user1_tok, timeout=Duration(seconds=10) ) # We should see the new `required_state` immediately without waiting. diff --git a/tests/rest/client/sliding_sync/test_sliding_sync.py b/tests/rest/client/sliding_sync/test_sliding_sync.py index 2b258ecb061..e3056331a06 100644 --- a/tests/rest/client/sliding_sync/test_sliding_sync.py +++ b/tests/rest/client/sliding_sync/test_sliding_sync.py @@ -44,6 +44,7 @@ StreamToken, ) from synapse.util.clock import Clock +from synapse.util.duration import Duration from synapse.util.stringutils import random_string from tests import unittest @@ -87,7 +88,7 @@ def make_sync_request( *, since: str | None = None, tok: str, - timeout_ms: int | None = None, + timeout: Duration | None = None, await_result: bool = True, ) -> FakeChannel: """Make a sliding sync request with given body. @@ -107,8 +108,8 @@ def make_sync_request( query_params: dict[str, Any] = {} if since: query_params["pos"] = since - if timeout_ms is not None: - query_params["timeout"] = timeout_ms + if timeout is not None: + query_params["timeout"] = timeout.as_millis() if query_params: query_str = urllib.parse.urlencode(query_params) @@ -129,7 +130,7 @@ def do_sync( *, since: str | None = None, tok: str, - timeout_ms: int | None = None, + timeout: Duration | None = None, ) -> tuple[JsonDict, str]: """Do a sliding sync request with given body. @@ -139,13 +140,13 @@ def do_sync( sync_body: The full request body to use since: Optional since token tok: Access token to use - timeout_ms: Optional timeout in milliseconds to use for the request. + timeout: Optional timeout to use for the request. Returns: A tuple of the response body and the `pos` field. """ channel = self.make_sync_request( - sync_body, since=since, tok=tok, timeout_ms=timeout_ms + sync_body, since=since, tok=tok, timeout=timeout ) self.assertEqual(channel.code, 200, channel.json_body) From 9966496ce2a4ba48ed7f2f765efc8bae9a961a8a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 30 Apr 2026 13:12:51 +0100 Subject: [PATCH 08/11] Use await_result and assertRaises --- .../sliding_sync/test_rooms_required_state.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index b167d47d380..7f8fad0f6b6 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -28,6 +28,7 @@ from synapse.util.duration import Duration from tests.rest.client.sliding_sync.test_sliding_sync import SlidingSyncBase +from tests.server import TimedOutException from tests.test_utils.event_injection import mark_event_as_partial_state logger = logging.getLogger(__name__) @@ -2285,14 +2286,14 @@ def test_changing_required_state_returns_immediately(self) -> None: timeout=Duration(seconds=10), await_result=False, ) - self.reactor.advance(0.1) # Allow the request to start processing - self.reactor.advance(9.5) - self.assertFalse(channel.is_finished()) - # Advance past the timeout to make sure the request finishes. (We do this - # to ensure log contexts don't leak between tests). - self.reactor.advance(1) - self.assertTrue(channel.is_finished()) + # Request will block for 10 seconds as there no updates. + with self.assertRaises(TimedOutException): + channel.await_result(timeout_ms=9500) + + # Wait for the request to actually finish. (We do this to ensure log + # contexts don't leak between tests). + channel.await_result(timeout_ms=1000) # Now update the Sliding Sync requests to include a `required_state` # event, and make another sync request. From 972777926af784e28c1f3a0d29040890cb8b66ea Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 30 Apr 2026 13:42:20 +0100 Subject: [PATCH 09/11] Ensure we don't wait for the timeout --- .../sliding_sync/test_rooms_required_state.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index 7f8fad0f6b6..9b12790f1cd 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -2301,11 +2301,18 @@ def test_changing_required_state_returns_immediately(self) -> None: [EventTypes.Create, ""], ] - response_body, _ = self.do_sync( - sync_body, since=from_token, tok=user1_tok, timeout=Duration(seconds=10) + channel = self.make_sync_request( + sync_body, + since=from_token, + tok=user1_tok, + timeout=Duration(seconds=10), + await_result=False, ) - # We should see the new `required_state` immediately without waiting. + # We should see the new `required_state` immediately without waiting + # (for the full timeout, we may need to wait briefly). + channel.await_result(timeout_ms=100) + response_body = channel.json_body self._assertRequiredStateIncludes( response_body["rooms"][room_id1]["required_state"], { From 340fdd3b46fd028db0f4ae99aa2b06fd741bf29d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 Jun 2026 10:41:21 +0100 Subject: [PATCH 10/11] Point out new test --- .../rest/client/sliding_sync/test_rooms_required_state.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index 9b12790f1cd..d69d5b994a2 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -1926,7 +1926,12 @@ def test_rooms_required_state_expand(self) -> None: def test_rooms_required_state_expand_retract_expand(self) -> None: """Test that when expanding, retracting and then expanding the required - state, we get the changes that happened.""" + state, we get the changes that happened. + + Also see `test_changing_required_state_returns_immediately`, which tests + that the sync stream is woken up immediately when changing the required + state, and not just on the next change to the room. + """ user1_id = self.register_user("user1", "pass") user1_tok = self.login(user1_id, "pass") From 60a2e2d0a7bdeb44636cdd53e50d5044916199a5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 Jun 2026 10:38:32 +0100 Subject: [PATCH 11/11] We don't need a timeout in the test --- tests/rest/client/sliding_sync/test_rooms_required_state.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index d69d5b994a2..901f22a35d1 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -2315,8 +2315,7 @@ def test_changing_required_state_returns_immediately(self) -> None: ) # We should see the new `required_state` immediately without waiting - # (for the full timeout, we may need to wait briefly). - channel.await_result(timeout_ms=100) + channel.await_result(timeout_ms=0) response_body = channel.json_body self._assertRequiredStateIncludes( response_body["rooms"][room_id1]["required_state"],