Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Consistently compare to the earliest known stream position in the stream change cache #14435

Merged
merged 16 commits into from
Dec 5, 2022
Merged

Consistently compare to the earliest known stream position in the stream change cache #14435

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
64dc954
Add (failing) tests.
clokep Nov 14, 2022
c969ab4
Use strict comparisons in stream change cache.
clokep Nov 14, 2022
3a46941
Newsfragment
clokep Nov 14, 2022
f0dbf31
Update asserts.
clokep Nov 15, 2022
7d0e07f
Clarify comments.
clokep Nov 15, 2022
9842054
Use peek instead of bisect.
clokep Nov 16, 2022
0c6c603
Clarify code & comments.
clokep Nov 16, 2022
e2e4f37
Improve docstrings.
clokep Nov 16, 2022
532f69a
Update tests for changes.
clokep Nov 16, 2022
dff4b5c
Merge branch 'develop' into clokep/stream-cache-fix
clokep Dec 2, 2022
6b09889
Bump min sortedcontainers version.
clokep Dec 2, 2022
f0634bf
Merge remote-tracking branch 'origin/develop' into clokep/stream-cach…
clokep Dec 2, 2022
d3513a3
Fix poetry lock.
clokep Dec 2, 2022
ae77f8b
Newsfragment
clokep Dec 2, 2022
53cd4cc
Add a comment.
clokep Dec 5, 2022
865792a
Merge remote-tracking branch 'origin/develop' into clokep/stream-cach…
clokep Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14435.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix.
2 changes: 1 addition & 1 deletion synapse/util/caches/stream_change_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def has_entity_changed(self, entity: EntityType, stream_pos: int) -> bool:
"""Returns True if the entity may have been updated since stream_pos"""
clokep marked this conversation as resolved.
Show resolved Hide resolved
assert isinstance(stream_pos, int)

if stream_pos < self._earliest_known_stream_pos:
if stream_pos <= self._earliest_known_stream_pos:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_entity_changed is called from the following places:

  • AccountDataWorkerStore.get_updated_account_data_for_user, which seems to be used to build account data in /sync responses.
  • DeviceInboxWorkerStore:
    • _get_device_messages: feeds into to-device updates in /sync and app services.
    • delete_messages_for_device: used to pause /sync in some situation? (Unclear...?)
    • get_new_device_msgs_for_remote: used when calculating EDUs to send in federation transactions.
  • DeviceWorkerStore:
    • get_device_updates_by_remote: used when calculating EDUs to send in federation transactions.
    • get_users_whose_signatures_changed: used in the device list entry for /sync.
  • EventPushActionsWorkerStore._get_notif_unread_count_for_user_room, which is used to generate notification counts in /sync and to update event_push_summary.
  • PushRulesWorkerStore.have_push_rules_changed_for_user, which is used to build push rules in /sync responses.
  • ReceiptsWorkerStore.get_linearized_receipts_for_room, which is used build receipts data in /initial_sync.
  • The StreamWorkerStore uses it in a few spots:
    • get_rooms_that_changed: feeds into the /keys/changes response.
    • get_room_events_stream_for_room: used to get the events that have changed in a room for /sync response.
    • get_membership_changes_for_user: feeds into the /keys/changes response. generating membership changes in /sync response, and whatever uses RoomEventSource.get_new_events.
  • TagsWorkerStore.get_updated_tags: used to generate data in a /sync response.

self.metrics.inc_misses()
return True

Expand Down
2 changes: 2 additions & 0 deletions tests/util/test_stream_change_cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ def test_has_entity_changed(self):
# return True, whether it's a known entity or not.
self.assertTrue(cache.has_entity_changed("user@foo.com", 0))
self.assertTrue(cache.has_entity_changed("not@here.website", 0))
self.assertTrue(cache.has_entity_changed("user@foo.com", 3))
self.assertTrue(cache.has_entity_changed("not@here.website", 3))

def test_entity_has_changed_pops_off_start(self):
"""
Expand Down