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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a01201f
Add module API function to ask whether the current worker is configur…
reivilibre Jul 24, 2023
7a613b3
Newsfile
reivilibre Jul 24, 2023
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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