Skip to content

Move start_doing_background_updates() to SynapseHomeServer.start_background_tasks() #19036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 10, 2025
Merged

Move start_doing_background_updates() to SynapseHomeServer.start_background_tasks() #19036

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
327da0d
Move `start_doing_background_updates()` to `SynapseHomeServer.start_b…
MadLittleMods Oct 9, 2025
a00eef7
Add changelog
MadLittleMods Oct 9, 2025
b5fcd46
Merge branch 'develop' into madlittlemods/move-start-background-updates
MadLittleMods Oct 10, 2025
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/19036.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Move `start_doing_background_updates()` to `SynapseHomeServer.start_background_tasks()`.
10 changes: 5 additions & 5 deletions synapse/app/homeserver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,11 @@ def start_listening(self) -> None:
# during parsing
logger.warning("Unrecognized listener type: %s", listener.type)

def start_background_tasks(self) -> None:
super().start_background_tasks()

self.get_datastores().main.db_pool.updates.start_doing_background_updates()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The one difference here is that previously, start_doing_background_updates() only ran on the main Synapse instance.

But start_background_tasks() runs on the worker that supposed to run_background_tasks.

synapse/synapse/app/_base.py

Lines 683 to 684 in 47fb4b4

if hs.config.worker.run_background_tasks:
hs.start_background_tasks()

Is that okay? Nothing is immediately coming to mind on why this would be bad.

Copy link
Contributor

Choose a reason for hiding this comment

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

seems OK to me too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think there is a flaw with this setup.

SynapseHomeServer is for the main Synapse instance. We have SynapseHomeServer.start_background_tasks() setup to call start_doing_background_updates() but if the run_background_tasks_on is set to anything else besides main, then we will never run SynapseHomeServer.start_background_tasks() specifically.

What we actually need to do is just move this to HomeServer.start_background_tasks() instead. We want start_doing_background_updates() to run on whatever homeserver is configured to run_background_tasks_on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is being addressed in #19057

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deciding to revert this PR in the meantime -> #19059

While I figure out all of the intricacies in the test infrastructure from moving it to the correct spot. Will re-introduce things in #19057



def load_or_generate_config(argv_options: List[str]) -> HomeServerConfig:
"""
Expand Down Expand Up @@ -430,11 +435,6 @@ async def _start_when_reactor_running() -> None:

await _base.start(hs, freeze)

# TODO: This should be moved to `SynapseHomeServer.start_background_tasks` (not
# `HomeServer.start_background_tasks`) (this way it matches the behavior of only
# running on `main`)
hs.get_datastores().main.db_pool.updates.start_doing_background_updates()

# Register a callback to be invoked once the reactor is running
register_start(hs, _start_when_reactor_running)

Expand Down
Loading