Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()` #19057

MadLittleMods · 2025-10-13T18:50:44Z

Move start_doing_background_updates() to HomeServer.start_background_tasks()

Follow-up to #19036 (reverted in #19059) where I incorrectly had this in SynapseHomeServer.start_background_tasks(). This was flawed because if run_background_tasks_on is configured to something other than main, we will never call SynapseHomeServer.start_background_tasks() specifically (SynapseHomeServer is for the main Synapse instance), and instead only call HomeServer.start_background_tasks() (HomeServer is for main and worker Synapse instances).

synapse/synapse/app/_base.py

Lines 682 to 683 in ec7554b

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

Our goal is to run start_doing_background_updates() on whatever homeserver is configured to run_background_tasks_on.

Dev notes

From #18886

# This will cause all of the relevant storage classes to be instantiated and call
# `register_background_update_handler(...)`,
# `register_background_index_update(...)`,
# `register_background_validate_constraint(...)`, etc so they are available to use
# if we are asked to run those background updates.
hs.get_storage_controllers()

homeserver_to_use=GenericWorkerServer

synapse.storage.prepare_database.UpgradeDatabaseException: Uninitialised database: run the main synapse process to prepare the database schema before starting worker processes.

-> #15671 (matrix-org/synapse#15671)

SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.replication.test_federation_ack.FederationAckTestCase.test_federation_ack_sent

SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.app.test_openid_listener.FederationReaderOpenIDListenerTests

SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.storage.test_rollback_worker.WorkerSchemaTests

run_background_tasks_on

SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.handlers.test_presence.PresenceHandlerInitTestCase

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
Code style is correct (run the linters)

…nd_tasks()`

MadLittleMods · 2025-10-13T18:52:09Z

changelog.d/19036.misc

@@ -1 +1 @@
-Move `start_doing_background_updates()` to `SynapseHomeServer.start_background_tasks()`.
+Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()`.


Updated the changelog from #19036 to merge with this one.

synapse/server.py

MadLittleMods · 2025-10-13T18:55:05Z

synapse/server.py

        self.get_task_scheduler()
        self.get_common_usage_metrics_manager().setup()
        start_phone_stats_home(self)
+        self.get_datastores().main.db_pool.updates.start_doing_background_updates()


To call this out again; before #19036 (comment), start_doing_background_updates() only ran on the main Synapse instance.

With this change, it runs on whatever instance is configured to run_background_tasks_on.

Based on the previous discussion, we thought this was fine. I just didn't have this in the correct spot to match that description.

synapse/server.py

See #19057 (comment) Running any `HomeserverTestCase` results in: ``` $ SYNAPSE_TEST_LOG_LEVEL=DEBUG poetry run trial tests.rest.client.sliding_sync.test_sliding_sync.SlidingSyncTestCase_new.test_sync_list ... builtins.AssertionError: Expected `call_later` callback from the reactor to start with the sentinel logcontext but saw run_bg_updates. In other words, another task shouldn't have leaked their logcontext to us. ```

This already happens as part of `HomeServer.start_background_tasks()`

See #19057 (comment)

MadLittleMods · 2025-10-13T20:30:39Z

tests/unittest.py

-        async def run_bg_updates() -> None:
-            with LoggingContext(name="run_bg_updates", server_name=server_name):
-                self.get_success(stor.db_pool.updates.run_background_updates(False))
-
        hs = setup_test_homeserver(
            cleanup_func=self.addCleanup,
            server_name=server_name,
            config=config_obj,
            reactor=reactor,
            clock=clock,
            **extra_homeserver_attributes,
        )
-        stor = hs.get_datastores().main

-        # Run the database background updates, when running against "master".
-        if hs.__class__.__name__ == "TestHomeServer":
-            self.get_success(run_bg_updates())


We don't need to duplicate running the database background updates here now that we do it in HomeServer.start_background_tasks().

We've now updated to just wait for the database background updates to finish which ensures that the homeserver is starting and driving the background to completion as part of its normal process. (less snowflake behavior in tests)

MadLittleMods · 2025-10-13T20:35:43Z

tests/unittest.py


-        async def run_bg_updates() -> None:
-            with LoggingContext(name="run_bg_updates", server_name=server_name):
-                self.get_success(stor.db_pool.updates.run_background_updates(False))


One difference to note here is that we're running with run_background_updates(False) which sets argument sleep: False and determines:

synapse/synapse/storage/background_updates.py

Lines 526 to 527 in ec7554b

sleep: Whether to limit how quickly we run background updates or

not.

Versus now where we run this via start_doing_background_updates() and sleep is configured via hs.config.background_updates.sleep_enabled and default to True. This means that we're potentially running slower in tests now. Will post an update to compare before/after to see if it even matters.

synapse/synapse/storage/background_updates.py

Lines 400 to 409 in ec7554b

def start_doing_background_updates(self) -> None:

if self.enabled:

# if we start a new background update, not all updates are done.

self._all_done = False

sleep = self.sleep_enabled

self.hs.run_as_background_process(

"background_updates",

self.run_background_updates,

sleep,

)

MadLittleMods · 2025-10-13T20:38:36Z

tests/unittest.py

-        store = self.hs.get_datastores().main
+        self._wait_for_background_updates(self.hs)
+
+    def _wait_for_background_updates(self, hs: HomeServer) -> None:


We no longer use this variant but I think it's useful to reference in the comment down below.

``` builtins.AssertionError: Expected `looping_call` callback from the reactor to start with the sentinel logcontext but saw task-_resumable_task-0-IBzAmHUoepQfLnEA. In other words, another task shouldn't have leaked their logcontext to us. ```

MadLittleMods · 2025-10-13T21:47:43Z

tests/rest/admin/test_room.py

        # Mock PaginationHandler.purge_room to sleep for 100s, so we have time to do a second call
        # before the purge is over. Note that it doesn't purge anymore, but we don't care.
        async def purge_room(room_id: str, force: bool) -> None:
-            await deferLater(self.hs.get_reactor(), 100, lambda: None)


All of these deferLater changes have been split off to #19058 so we could merge that first in order to clean up the diff here.

They are changed here because we started seeing these failures in CI with this PR for some reason:

Example failure: https://github.com/element-hq/synapse/actions/runs/18477454390/job/52645183606?pr=19057

builtins.AssertionError: Expected `looping_call` callback from the reactor to start with the sentinel logcontext but saw task-_resumable_task-0-IBzAmHUoepQfLnEA. In other words, another task shouldn't have leaked their logcontext to us.

MadLittleMods · 2025-10-13T23:01:39Z

changelog.d/19036.misc

@@ -1 +1 @@
-Move `start_doing_background_updates()` to `SynapseHomeServer.start_background_tasks()`.
+Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()`.


PresenceHandlerInitTestCase failures ❌

PresenceHandlerInitTestCase purposely tries to disable background tasks to keep PresenceHandler uninitialized until the test is prepared. It tries to test that "presence state restored from the database [when PresenceHandler is initialized] should not persist forever".

This conflicts with the changes currently being made where we expect the background tasks to run because we want the database background tasks to complete before we start the tests.

synapse/tests/handlers/test_presence.py

Lines 811 to 817 in ec7554b

class PresenceHandlerInitTestCase(unittest.HomeserverTestCase):

def default_config(self) -> JsonDict:

config = super().default_config()

# Disable background tasks on this worker so that the PresenceHandler isn't

# loaded until we request it.

config["run_background_tasks_on"] = "other"

return config

Example failures:

SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.handlers.test_presence.PresenceHandlerInitTestCase The "poetry.dev-dependencies" section is deprecated and will be removed in a future version. Use "poetry.group.dev.dependencies" instead. tests.handlers.test_presence PresenceHandlerInitTestCase test_restored_presence_idles ... [ERROR] test_restored_presence_online_after_sync_0_org_matrix_msc3026_busy ... [ERROR] test_restored_presence_online_after_sync_1_online ... [ERROR] test_restored_presence_online_after_sync_2_unavailable ... [ERROR] test_restored_presence_online_after_sync_3_offline ... [ERROR] =============================================================================== [ERROR] Traceback (most recent call last): File "/home/eric/Documents/github/element/synapse/tests/unittest.py", line 138, in new return code(orig, *args, **kwargs) File "/home/eric/Documents/github/element/synapse/tests/unittest.py", line 232, in setUp return orig() File "/home/eric/Documents/github/element/synapse/tests/unittest.py", line 404, in setUp self.hs = self.make_homeserver(self.reactor, self.clock) File "/home/eric/Documents/github/element/synapse/tests/unittest.py", line 525, in make_homeserver hs = self.setup_test_homeserver(reactor=reactor, clock=clock) File "/home/eric/Documents/github/element/synapse/tests/unittest.py", line 733, in setup_test_homeserver raise AssertionError( builtins.AssertionError: Timed out waiting for background updates to complete (5s). Did you forget to `start_doing_background_updates()`? tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_idles tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_0_org_matrix_msc3026_busy tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_1_online tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_2_unavailable tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_3_offline ------------------------------------------------------------------------------- Ran 5 tests in 25.208s FAILED (errors=5)

MadLittleMods · 2025-10-13T23:17:30Z

changelog.d/19036.misc

@@ -1 +1 @@
-Move `start_doing_background_updates()` to `SynapseHomeServer.start_background_tasks()`.
+Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()`.


Database isn't setup for tests that use GenericWorkerServer ❌

We have a few tests that use homeserver_to_use=GenericWorkerServer and are failing with builtins.AssertionError: Timed out waiting for background updates to complete (5s). Did you forget to 'start_doing_background_updates()'? in CI (from our new assertion).

Error: Traceback (most recent call last): File "/home/runner/work/synapse/synapse/tests/unittest.py", line 138, in new return code(orig, *args, **kwargs) File "/home/runner/work/synapse/synapse/tests/unittest.py", line 232, in setUp return orig() File "/home/runner/work/synapse/synapse/tests/unittest.py", line 404, in setUp self.hs = self.make_homeserver(self.reactor, self.clock) File "/home/runner/work/synapse/synapse/tests/app/test_openid_listener.py", line 40, in make_homeserver hs = self.setup_test_homeserver(homeserver_to_use=GenericWorkerServer) File "/home/runner/work/synapse/synapse/tests/unittest.py", line 733, in setup_test_homeserver raise AssertionError( builtins.AssertionError: Timed out waiting for background updates to complete (5s). Did you forget to `start_doing_background_updates()`? tests.app.test_openid_listener.FederationReaderOpenIDListenerTests.test_openid_listener_0 tests.app.test_openid_listener.FederationReaderOpenIDListenerTests.test_openid_listener_1 tests.app.test_openid_listener.FederationReaderOpenIDListenerTests.test_openid_listener_2 tests.app.test_openid_listener.FederationReaderOpenIDListenerTests.test_openid_listener_3 ... tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_idles tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_0_org_matrix_msc3026_busy tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_1_online tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_2_unavailable tests.handlers.test_presence.PresenceHandlerInitTestCase.test_restored_presence_online_after_sync_3_offline ... tests.replication.test_federation_ack.FederationAckTestCase.test_federation_ack_sent ... tests.storage.test_rollback_worker.WorkerSchemaTests.test_not_upgraded_current_schema_version_with_outstanding_deltas tests.storage.test_rollback_worker.WorkerSchemaTests.test_not_upgraded_old_schema_version tests.storage.test_rollback_worker.WorkerSchemaTests.test_rolling_back

Previously, we didn't wait for background updates in these cases and only ran the database background updates for the TestHomeServer (the default in tests). Basically turning a blind eye to this kind of problem.

synapse/tests/unittest.py

Lines 680 to 696 in ec7554b

async def run_bg_updates() -> None:

with LoggingContext(name="run_bg_updates", server_name=server_name):

self.get_success(stor.db_pool.updates.run_background_updates(False))

hs = setup_test_homeserver(

cleanup_func=self.addCleanup,

server_name=server_name,

config=config_obj,

reactor=reactor,

clock=clock,

**extra_homeserver_attributes,

)

stor = hs.get_datastores().main

# Run the database background updates, when running against "master".

if hs.__class__.__name__ == "TestHomeServer":

self.get_success(run_bg_updates())

Unable to run test by itself

Compounding things is that we have to deal with #15671 (previously matrix-org/synapse#15671) which prevents us from running these tests on their own so it's even harder to debug what's going on.

synapse.storage.prepare_database.UpgradeDatabaseException: Uninitialised database: run the main synapse process to prepare the database schema before starting worker processes.

….start_background_tasks()` (#19036)" (#19059) ### Why See #19036 (comment) Revert while I figure out the tests in #19057

MadLittleMods added 2 commits October 13, 2025 13:46

Move start_doing_background_updates() to `HomeServer.start_backgrou…

4cf90e1

…nd_tasks()`

Add changelog

78fd66c

MadLittleMods commented Oct 13, 2025

View reviewed changes

synapse/server.py Outdated Show resolved Hide resolved

MadLittleMods commented Oct 13, 2025

View reviewed changes

MadLittleMods added the A-Database label Oct 13, 2025

MadLittleMods commented Oct 13, 2025

View reviewed changes

synapse/server.py Show resolved Hide resolved

MadLittleMods added 9 commits October 13, 2025 14:21

Remove duplicate database background update running

01343d6

This already happens as part of `HomeServer.start_background_tasks()`

Add future TODO

2fcd70d

Fix lints

74c5e64

Remove assumption

c1d89d8

See #19057 (comment)

Wait for homeserver to drive database background updates

064b30b

Better context for error

5e87f89

Comment about slight naivety

edf9ef0

Fix lints

e14e5d3

MadLittleMods commented Oct 13, 2025

View reviewed changes

MadLittleMods mentioned this pull request Oct 13, 2025

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

Merged

3 tasks

MadLittleMods commented Oct 13, 2025

View reviewed changes

MadLittleMods added 5 commits October 13, 2025 16:21

Better conversion

b4d1789

Even better adaption incomplete_d

6a11d64

Remove `deferLater from other tests

4f4bf80

Remove mentions of deferLater

53fd1b2

MadLittleMods mentioned this pull request Oct 13, 2025

Remove logcontext problems caused by awaiting raw deferLater(...) #19058

Open

3 tasks

MadLittleMods commented Oct 13, 2025

View reviewed changes

Fix lints

9e99547

MadLittleMods mentioned this pull request Oct 13, 2025

Revert "Move start_doing_background_updates() to SynapseHomeServer.start_background_tasks() (#19036)" #19059

Merged

3 tasks

MadLittleMods commented Oct 13, 2025

View reviewed changes

MadLittleMods added a commit that referenced this pull request Oct 17, 2025

Update comment with actual learnings from #19057

623a748

MadLittleMods added a commit that referenced this pull request Oct 20, 2025

Revert "Move start_doing_background_updates() to `SynapseHomeServer…

eac8626

….start_background_tasks()` (#19036)" (#19059) ### Why See #19036 (comment) Revert while I figure out the tests in #19057

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()` #19057

Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()` #19057

Uh oh!

MadLittleMods commented Oct 13, 2025 •

edited

Loading

Uh oh!

MadLittleMods Oct 13, 2025

Uh oh!

Uh oh!

MadLittleMods Oct 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

MadLittleMods Oct 13, 2025

Uh oh!

MadLittleMods Oct 13, 2025

Uh oh!

MadLittleMods Oct 13, 2025

Uh oh!

MadLittleMods Oct 13, 2025 •

edited

Loading

Uh oh!

MadLittleMods Oct 13, 2025

Uh oh!

MadLittleMods Oct 13, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

		@@ -1 +1 @@
		Move `start_doing_background_updates()` to `SynapseHomeServer.start_background_tasks()`.
		Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()`.

	sleep: Whether to limit how quickly we run background updates or
	not.

	def start_doing_background_updates(self) -> None:
	if self.enabled:
	# if we start a new background update, not all updates are done.
	self._all_done = False
	sleep = self.sleep_enabled
	self.hs.run_as_background_process(
	"background_updates",
	self.run_background_updates,
	sleep,
	)

	class PresenceHandlerInitTestCase(unittest.HomeserverTestCase):
	def default_config(self) -> JsonDict:
	config = super().default_config()
	# Disable background tasks on this worker so that the PresenceHandler isn't
	# loaded until we request it.
	config["run_background_tasks_on"] = "other"
	return config

	async def run_bg_updates() -> None:
	with LoggingContext(name="run_bg_updates", server_name=server_name):
	self.get_success(stor.db_pool.updates.run_background_updates(False))

	hs = setup_test_homeserver(
	cleanup_func=self.addCleanup,
	server_name=server_name,
	config=config_obj,
	reactor=reactor,
	clock=clock,
	**extra_homeserver_attributes,
	)
	stor = hs.get_datastores().main

	# Run the database background updates, when running against "master".
	if hs.__class__.__name__ == "TestHomeServer":
	self.get_success(run_bg_updates())

Move start_doing_background_updates() to HomeServer.start_background_tasks() #19057

Are you sure you want to change the base?

Move start_doing_background_updates() to HomeServer.start_background_tasks() #19057

Uh oh!

Conversation

MadLittleMods commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev notes

Pull Request Checklist

Uh oh!

MadLittleMods Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MadLittleMods Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MadLittleMods Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Oct 13, 2025

Choose a reason for hiding this comment

PresenceHandlerInitTestCase failures ❌

Uh oh!

MadLittleMods Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Database isn't setup for tests that use GenericWorkerServer ❌

Unable to run test by itself

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()` #19057

Move `start_doing_background_updates()` to `HomeServer.start_background_tasks()` #19057

MadLittleMods commented Oct 13, 2025 •

edited

Loading

MadLittleMods Oct 13, 2025 •

edited

Loading

MadLittleMods Oct 13, 2025 •

edited

Loading

`PresenceHandlerInitTestCase` failures ❌

MadLittleMods Oct 13, 2025 •

edited

Loading

Database isn't setup for tests that use `GenericWorkerServer` ❌