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

Don't unnecessarily start bg process in replication sending loop. #8670

Merged
merged 3 commits into from
Oct 27, 2020
Merged

Don't unnecessarily start bg process in replication sending loop. #8670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c89c75
Don't unnecessarily start bg process in replication sending loop.
erikjohnston Oct 27, 2020
41bad69
Newsfile
erikjohnston Oct 27, 2020
9627934
Fix
erikjohnston Oct 27, 2020
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/8670.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce number of OpenTracing spans started.
10 changes: 10 additions & 0 deletions synapse/replication/tcp/resource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ def on_notifier_poke(self):
stream.discard_updates_and_advance()
return

# We check up front to see if anything has actually changed, as we get
# poked because of changes that happened on other instances.
if all(
stream.last_token == stream.current_token(self._instance_name)
for stream in self.streams
):
return
Comment on lines +122 to +126
Copy link
Member

Choose a reason for hiding this comment

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

I wondered if there was a clever way we could filter out the streams that have not changed and only operate on the ones that have changed, but I think that would require a much large refactoring.

For reference, this seems to be uplifting the check in the loop (

synapse/synapse/replication/tcp/resource.py

Lines 158 to 161 in 9627934

if stream.last_token == stream.current_token(
self._instance_name
):
continue
) to handle a case of skipping all entries.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I thought about doing something more clever then decided life was too short 🙂


# If there are updates then we need to set this even if we're already
# looping, as the loop needs to know that he might need to loop again.
self.pending_updates = True

if self.is_looping:
Expand Down