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

Allow modules to check whether the current worker is configured to run background tasks. #15991

Merged
merged 2 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/15991.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow modules to check whether the current worker is configured to run background tasks.
12 changes: 12 additions & 0 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,18 @@ def looping_background_call(
f,
)

def should_run_background_tasks(self) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a fairly low-level thing to expose -- can you expand a bit for why we need this? Is this to be paired with #15993 inside a module? Would it make more sense to have the same run_on_all_instances that looping_background_call has?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to be paired with looping_background_call for registering an event handler that may be useful in conjunction with the background call.
I get it's a bit weird but the alternative is having every module need to accept its own configuration for which worker to use as the background worker (often we do this and default to the master process).

(Maybe I should just push the configuration and logic up to the module and use run_on_all_instances=True but only register the background call on one worker?)

Copy link
Member

Choose a reason for hiding this comment

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

(Maybe I should just push the configuration and logic up to the module and use run_on_all_instances=True but only register the background call on one worker?)

I'm confused -- why can't you just let it do run_on_all_instances=False? It sounds like the goal is to run it on a single worker?

Copy link
Contributor Author

@reivilibre reivilibre Jul 27, 2023

Choose a reason for hiding this comment

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

I need to register an event handler for new events (on_new_event) on the same worker as the one running the looping_background_call jobs, so either need to let the background worker be in charge, or alternatively ignore the logic for run_on_all_instances=False and control it entirely within the module.

"""
Return true if and only if the current worker is configured to run
background tasks.
There should only be one worker configured to run background tasks, so
this is helpful when you need to only run a task on one worker but don't
have any other good way to choose which one.

Added in Synapse v1.89.0.
"""
return self._hs.config.worker.run_background_tasks

async def sleep(self, seconds: float) -> None:
"""Sleeps for the given number of seconds.

Expand Down
Loading