From feb30ffe1e62bb794f970c22d0c4d68302f206ff Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 7 Dec 2022 11:39:45 -0500 Subject: [PATCH 1/5] Check the stream position before checking if the cache is empty. --- synapse/util/caches/stream_change_cache.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/util/caches/stream_change_cache.py b/synapse/util/caches/stream_change_cache.py index c8b17acb59e1..dde8bd8be86b 100644 --- a/synapse/util/caches/stream_change_cache.py +++ b/synapse/util/caches/stream_change_cache.py @@ -213,16 +213,17 @@ def has_any_entity_changed(self, stream_pos: int) -> bool: """ assert isinstance(stream_pos, int) - if not self._cache: - # If the cache is empty, nothing can have changed. - return False - # _cache is not valid at or before the earliest known stream position, so # return that an entity has changed. if stream_pos <= self._earliest_known_stream_pos: self.metrics.inc_misses() return True + # If the cache is empty, nothing can have changed. + if not self._cache: + # TODO Metrics? + return False + self.metrics.inc_hits() return stream_pos < self._cache.peekitem()[0] From df11922cf73e1eef5569b6c535d8616fd7fb0950 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 7 Dec 2022 11:41:17 -0500 Subject: [PATCH 2/5] Newsfragment --- changelog.d/14639.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14639.bugfix diff --git a/changelog.d/14639.bugfix b/changelog.d/14639.bugfix new file mode 100644 index 000000000000..149ee99dd716 --- /dev/null +++ b/changelog.d/14639.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where a device list update might not be sent to clients in certain circumstances. From 36c61df94bcdcc15207dc46eb356c9d8cb135c4f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 7 Dec 2022 11:42:38 -0500 Subject: [PATCH 3/5] Fix tests. --- tests/util/test_stream_change_cache.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/util/test_stream_change_cache.py b/tests/util/test_stream_change_cache.py index 0305741c9940..3df053493b3e 100644 --- a/tests/util/test_stream_change_cache.py +++ b/tests/util/test_stream_change_cache.py @@ -144,9 +144,10 @@ def test_has_any_entity_changed(self) -> None: """ cache = StreamChangeCache("#test", 1) - # With no entities, it returns False for the past, present, and future. - self.assertFalse(cache.has_any_entity_changed(0)) - self.assertFalse(cache.has_any_entity_changed(1)) + # With no entities, it returns True for the past, present, and False for + # the future. + self.assertTrue(cache.has_any_entity_changed(0)) + self.assertTrue(cache.has_any_entity_changed(1)) self.assertFalse(cache.has_any_entity_changed(2)) # We add an entity From 345d5badeb662aaada14630cb52ddb9e3cfb319c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 8 Dec 2022 09:55:45 -0500 Subject: [PATCH 4/5] Add metrics. --- synapse/util/caches/stream_change_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/util/caches/stream_change_cache.py b/synapse/util/caches/stream_change_cache.py index dde8bd8be86b..165745954990 100644 --- a/synapse/util/caches/stream_change_cache.py +++ b/synapse/util/caches/stream_change_cache.py @@ -221,7 +221,7 @@ def has_any_entity_changed(self, stream_pos: int) -> bool: # If the cache is empty, nothing can have changed. if not self._cache: - # TODO Metrics? + self.metrics.inc_misses() return False self.metrics.inc_hits() From a3530725ab1d63dbd42a144d6ec8e9af1247caf8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 8 Dec 2022 09:56:44 -0500 Subject: [PATCH 5/5] Update changelog. --- changelog.d/14639.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/14639.bugfix b/changelog.d/14639.bugfix index 149ee99dd716..8730b10afe7b 100644 --- a/changelog.d/14639.bugfix +++ b/changelog.d/14639.bugfix @@ -1 +1 @@ -Fix a long-standing bug where a device list update might not be sent to clients in certain circumstances. +Fix a long-standing bug where the user directory and room/user stats might be out of sync.